fix(pacquet): honor --no-runtime on the fresh-lockfile install path (#13225)
## Summary Third parity gap surfaced by the [pnpm/setup](https://github.com/pnpm/setup) action (after #13194): `pnpm install --no-runtime` without `--frozen-lockfile` was refused with `ERR_PNPM_PACKAGE_MANAGER_UNSUPPORTED_FRESH_INSTALL_SKIP_RUNTIMES`. The setup action passes `--no-runtime` when its `runtime` input is set, so any workspace without a committed `pnpm-lock.yaml` failed its install step. The TypeScript CLI honors the flag in every install mode: runtime dependencies are resolved and recorded in the lockfile, but their archives are not fetched and their bins are not linked (`pnpm11/installing/deps-installer/src/install/index.ts`, the `skipRuntimes` block after `resolution_done`). ## Implementation - The frozen path's direct-runtime filter (previously inline in `install_frozen_lockfile.rs`) is extracted into a shared `add_direct_runtime_skips` helper in `installability.rs`, next to the `SkippedSnapshots` type it mutates. - The fresh path now runs the same filter between its `--no-optional` exclusion and the dependency-closure extension, so runtime snapshots never reach the materialization fetch, the hoist, the symlink pass, or bin linking. The skip reuses the transient `optional_excluded` bucket, so nothing is persisted to `.modules.yaml.skipped` and the resolved entries stay in the written lockfile — same as the frozen path. - `InstallWithFreshLockfile` grows a `skip_runtimes` field, threaded from `Install`; the dispatch-time guard and its `UnsupportedFreshInstallSkipRuntimes` error variant are removed. ## Parity notes - Rust-only change; the TypeScript CLI already behaves this way. Changeset targets `pacquet` (patch). - One deliberate behavior pin: after `install --no-runtime`, a follow-up plain `install` reports the modules state up to date and does **not** restore the skipped runtime. I verified empirically that TypeScript pnpm 11.13 behaves identically (`Already up to date`), so the integration test pins the current parity rather than a hypothetical restore. If we ever want the restore behavior, it needs to change in both stacks together. ## Tests - `fresh_install_with_no_runtime_resolves_but_does_not_fetch_the_runtime` (integration, mocked node release): fresh `install --no-runtime` succeeds, the runtime stays in the written lockfile, `node_modules/node` and its bin links are absent, and the archive endpoint records zero hits — including across the follow-up plain install. - The old guard unit test is repurposed as `fresh_install_honors_skip_runtimes` (dispatch must not refuse the flag). - Full `pacquet-package-manager` + `pacquet-cli` suites: 2858/2858 pass; fmt / clippy / doc clean (pre-push gate).
This commit is contained in:
Zoltan Kochan authored and GitHub committed
1 parent
10382e0d46
commit
aacdcee266
7 files changed
+113
-84
No files matched your search
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"pacquet": patch
|
||||
---
|
||||
|
||||
`pnpm install --no-runtime` now works without `--frozen-lockfile`: on a fresh install, runtime dependencies are resolved and recorded in the lockfile, but their archives are not downloaded and their bins are not linked.
|
||||
@@ -132,6 +132,45 @@ fn installs_node_runtime_from_the_rc_channel() {
|
||||
assert!(fs::read_to_string(workspace.join("pnpm-lock.yaml")).unwrap().contains(version));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn fresh_install_with_no_runtime_resolves_but_does_not_fetch_the_runtime() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let mut server = mockito::Server::new();
|
||||
let version = "24.0.0-rc.4";
|
||||
let [_index, _shasums, archive] = mock_node_release(&mut server, version);
|
||||
let workspace = prepare_workspace(
|
||||
&root,
|
||||
format!("nodeDownloadMirrors:\n rc: '{}/'\n", server.url()).as_str(),
|
||||
);
|
||||
fs::write(
|
||||
workspace.join("package.json"),
|
||||
json!({ "dependencies": { "node": format!("runtime:{version}") } }).to_string(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// No pnpm-lock.yaml, so the install takes the fresh-resolve path.
|
||||
command(&workspace).with_args(["install", "--no-runtime"]).assert().success();
|
||||
|
||||
let lockfile = fs::read_to_string(workspace.join("pnpm-lock.yaml")).unwrap();
|
||||
assert!(
|
||||
lockfile.contains(format!("node@runtime:{version}").as_str()),
|
||||
"the resolved runtime stays in the lockfile:\n{lockfile}",
|
||||
);
|
||||
assert!(
|
||||
!workspace.join("node_modules/node").exists(),
|
||||
"the runtime must not be materialized under --no-runtime",
|
||||
);
|
||||
let bin_dir = workspace.join("node_modules/.bin");
|
||||
for bin in ["node", "node.exe", "node.cmd"] {
|
||||
assert!(!bin_dir.join(bin).exists(), "runtime bin {bin} must not be linked");
|
||||
}
|
||||
// A follow-up plain install treats the modules state as up to date
|
||||
// and does not restore the runtime — same as the TypeScript CLI.
|
||||
command(&workspace).with_arg("install").assert().success();
|
||||
assert!(!workspace.join("node_modules/node").exists());
|
||||
assert!(!archive.matched(), "the runtime archive must never be downloaded");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn installs_node_runtime_declared_by_a_dependency_engine() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -384,18 +384,6 @@ pub enum InstallError {
|
||||
#[diagnostic(code(ERR_PNPM_PNPMFILE_FAIL))]
|
||||
CustomResolverForceResolve(#[error(not(source))] pacquet_hooks::HookError),
|
||||
|
||||
/// `--no-runtime` (or `config.skip_runtimes`) is honored only on
|
||||
/// the frozen-lockfile path today, where the runtime filter runs
|
||||
/// against the loaded lockfile's `packages:` map. A non-frozen
|
||||
/// install would still fetch + materialize runtime archives
|
||||
/// despite the opt-out, so refuse the install instead of
|
||||
/// silently ignoring the flag.
|
||||
#[display(
|
||||
"--no-runtime / skipRuntimes is not supported without --frozen-lockfile yet. Re-run with --frozen-lockfile against an existing pnpm-lock.yaml, or drop the flag."
|
||||
)]
|
||||
#[diagnostic(code(ERR_PNPM_PACKAGE_MANAGER_UNSUPPORTED_FRESH_INSTALL_SKIP_RUNTIMES))]
|
||||
UnsupportedFreshInstallSkipRuntimes,
|
||||
|
||||
#[diagnostic(transparent)]
|
||||
FrozenLockfile(#[error(source)] InstallFrozenLockfileError),
|
||||
|
||||
@@ -1889,24 +1877,6 @@ where
|
||||
.await?;
|
||||
}
|
||||
|
||||
// Flag combinations the fresh-lockfile path doesn't honor
|
||||
// yet are validated here, after the dispatch decision so an
|
||||
// auto-frozen install (state 2 of [`Install::run`]) doesn't
|
||||
// get rejected up front:
|
||||
//
|
||||
// - `skip_runtimes` (CLI `--no-runtime`) on the fresh path
|
||||
// would need a runtime-filter at the materialization step
|
||||
// matching the frozen path's runtime filter. Without it,
|
||||
// runtime archives get fetched + materialized despite the
|
||||
// opt-out.
|
||||
//
|
||||
// Bypassed under `--lockfile-only`: that path writes only
|
||||
// `pnpm-lock.yaml` and never materializes, so the runtime
|
||||
// filter is irrelevant to its output.
|
||||
if !resolve_only && skip_runtimes {
|
||||
return Err(InstallError::UnsupportedFreshInstallSkipRuntimes);
|
||||
}
|
||||
|
||||
// The fresh-lockfile path has no installability check
|
||||
// (no `packages:` metadata to evaluate constraints
|
||||
// against), so its skip set is empty by construction.
|
||||
@@ -1968,6 +1938,7 @@ where
|
||||
node_linker,
|
||||
supported_architectures: supported_architectures.as_ref(),
|
||||
lockfile_only: resolve_only,
|
||||
skip_runtimes,
|
||||
dry_run,
|
||||
can_prompt,
|
||||
is_full_install,
|
||||
|
||||
@@ -6317,12 +6317,11 @@ async fn fresh_install_hoisted_node_linker_records_modules_yaml() {
|
||||
drop(dir);
|
||||
}
|
||||
|
||||
/// `--no-runtime` (`config.skip_runtimes = true`) on the fresh path
|
||||
/// is refused for the same reason: pacquet's runtime filter runs only
|
||||
/// inside the frozen-lockfile path, so honoring the flag on a fresh
|
||||
/// install would need a runtime-snapshot filter there too.
|
||||
/// A fresh install must not be refused for carrying `--no-runtime`
|
||||
/// (`config.skip_runtimes = true`); the runtime-skipping behavior
|
||||
/// itself is covered by the `install_runtimes` integration tests.
|
||||
#[tokio::test]
|
||||
async fn fresh_install_refuses_skip_runtimes_before_writing_state() {
|
||||
async fn fresh_install_honors_skip_runtimes() {
|
||||
let dir = tempdir().unwrap();
|
||||
let store_dir = dir.path().join("pacquet-store");
|
||||
let project_root = dir.path().join("project");
|
||||
@@ -6373,10 +6372,9 @@ async fn fresh_install_refuses_skip_runtimes_before_writing_state() {
|
||||
.run::<SilentReporter>()
|
||||
.await;
|
||||
|
||||
assert!(matches!(result, Err(InstallError::UnsupportedFreshInstallSkipRuntimes)));
|
||||
assert!(!dir.path().join(Lockfile::FILE_NAME).exists(), "no wanted lockfile written");
|
||||
assert!(!virtual_store_dir.join(Lockfile::CURRENT_FILE_NAME).exists(), "no current lockfile");
|
||||
assert!(!modules_dir.join(".modules.yaml").exists(), "no modules manifest");
|
||||
result.expect("fresh install with skip_runtimes should succeed");
|
||||
let _ = virtual_store_dir;
|
||||
assert!(modules_dir.join(".modules.yaml").exists(), "modules manifest written");
|
||||
|
||||
drop(dir);
|
||||
}
|
||||
|
||||
@@ -810,51 +810,8 @@ where
|
||||
}
|
||||
}
|
||||
|
||||
// `--no-runtime` (or `config.skip_runtimes`): exclude
|
||||
// every project-direct runtime dependency — iterate each
|
||||
// importer's direct deps and add the runtime ones to the
|
||||
// skip set; transitive runtime entries (which would be
|
||||
// unusual but possible) stay in the install. The
|
||||
// discriminator is a `@runtime:` substring check on the
|
||||
// resolved depPath; pacquet's lockfile preserves the
|
||||
// `@runtime:` substring in the snapshot key, so the
|
||||
// string-test works here.
|
||||
//
|
||||
// Re-using `add_optional_excluded` keeps the bucket count
|
||||
// (and `.modules.yaml.skipped` semantics) unchanged: like
|
||||
// `--no-optional`, this is a transient user-driven
|
||||
// exclusion that should *not* be persisted into
|
||||
// `.modules.yaml.skipped` — a future install without the
|
||||
// flag must bring the runtime back.
|
||||
if skip_runtimes && let Some(pkgs) = packages {
|
||||
for importer in importers.values() {
|
||||
for dep_map in [
|
||||
importer.dependencies.as_ref(),
|
||||
importer.dev_dependencies.as_ref(),
|
||||
importer.optional_dependencies.as_ref(),
|
||||
] {
|
||||
let Some(dep_map) = dep_map else { continue };
|
||||
for (alias, spec) in dep_map {
|
||||
// Build the candidate snapshot key. For
|
||||
// non-aliased deps this is `(alias, version)`;
|
||||
// for aliased deps it's the alias's own
|
||||
// (name, suffix). `link:` deps are skipped.
|
||||
let Some(key) = spec.version.resolved_key(alias) else { continue };
|
||||
if !key.to_string().contains("@runtime:") {
|
||||
continue;
|
||||
}
|
||||
if let Some(meta) = pkgs.get(&key)
|
||||
&& matches!(
|
||||
&meta.resolution,
|
||||
pacquet_lockfile::LockfileResolution::Binary(_)
|
||||
| pacquet_lockfile::LockfileResolution::Variations(_),
|
||||
)
|
||||
{
|
||||
skipped.add_optional_excluded(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
crate::add_direct_runtime_skips(&mut skipped, importers, pkgs);
|
||||
}
|
||||
|
||||
// The recorded skip set must be the reachability closure of the
|
||||
|
||||
@@ -168,6 +168,9 @@ pub struct InstallWithFreshLockfile<'a, DependencyGroupList> {
|
||||
/// stays untouched (no tarball is fetched) — a dry-run resolve pass.
|
||||
/// See [`crate::Install::lockfile_only`].
|
||||
pub lockfile_only: bool,
|
||||
/// `config.skip_runtimes || --no-runtime`; see
|
||||
/// [`crate::add_direct_runtime_skips`].
|
||||
pub skip_runtimes: bool,
|
||||
/// `--dry-run`: build the would-be lockfile but do not write it to
|
||||
/// disk. Implies [`Self::lockfile_only`] (nothing is materialized);
|
||||
/// the caller diffs the returned [`InstallWithFreshLockfileResult::wanted_lockfile`]
|
||||
@@ -623,6 +626,7 @@ impl<DependencyGroupList> InstallWithFreshLockfile<'_, DependencyGroupList> {
|
||||
node_linker,
|
||||
supported_architectures,
|
||||
lockfile_only,
|
||||
skip_runtimes,
|
||||
dry_run,
|
||||
can_prompt,
|
||||
is_full_install,
|
||||
@@ -1943,6 +1947,15 @@ impl<DependencyGroupList> InstallWithFreshLockfile<'_, DependencyGroupList> {
|
||||
}
|
||||
}
|
||||
|
||||
if skip_runtimes && let Some(packages) = initial_materialization_lockfile.packages.as_ref()
|
||||
{
|
||||
crate::add_direct_runtime_skips(
|
||||
&mut skipped,
|
||||
&initial_materialization_lockfile.importers,
|
||||
packages,
|
||||
);
|
||||
}
|
||||
|
||||
// The recorded skip set must be the reachability closure of the
|
||||
// direct skips (see
|
||||
// [`crate::extend_skipped_with_dependency_closure`]); extend it
|
||||
|
||||
@@ -18,7 +18,9 @@ use std::{
|
||||
collections::{HashMap, HashSet, VecDeque},
|
||||
};
|
||||
|
||||
use pacquet_lockfile::{PackageKey, PackageMetadata, ProjectSnapshot, SnapshotEntry};
|
||||
use pacquet_lockfile::{
|
||||
LockfileResolution, PackageKey, PackageMetadata, ProjectSnapshot, SnapshotEntry,
|
||||
};
|
||||
use pacquet_package_is_installable::{
|
||||
InstallabilityError, InstallabilityOptions, PackageInstallabilityManifest, SkipReason,
|
||||
SupportedArchitectures, WantedEngine, WantedPlatformRef, check_package, inferred_platform,
|
||||
@@ -563,6 +565,50 @@ pub fn compute_skipped_snapshots<Reporter: self::Reporter>(
|
||||
Ok(skipped)
|
||||
}
|
||||
|
||||
/// `--no-runtime` (or `config.skip_runtimes`): add every project-direct
|
||||
/// runtime dependency (a `@runtime:` snapshot key with a binary
|
||||
/// resolution) to the skip set, keeping its archive unfetched and its
|
||||
/// bins unlinked while the resolved entry stays in the lockfile.
|
||||
/// Shared by the frozen- and fresh-lockfile install paths, which run it
|
||||
/// right before the dependency-closure extension.
|
||||
///
|
||||
/// The skips reuse the transient bucket of
|
||||
/// [`SkippedSnapshots::add_optional_excluded`], so — like
|
||||
/// `--no-optional` — the exclusion is never persisted into
|
||||
/// `.modules.yaml.skipped`.
|
||||
pub fn add_direct_runtime_skips(
|
||||
skipped: &mut SkippedSnapshots,
|
||||
importers: &HashMap<String, ProjectSnapshot>,
|
||||
packages: &HashMap<PackageKey, PackageMetadata>,
|
||||
) {
|
||||
for importer in importers.values() {
|
||||
for dep_map in [
|
||||
importer.dependencies.as_ref(),
|
||||
importer.dev_dependencies.as_ref(),
|
||||
importer.optional_dependencies.as_ref(),
|
||||
] {
|
||||
let Some(dep_map) = dep_map else { continue };
|
||||
for (alias, spec) in dep_map {
|
||||
// Build the candidate snapshot key. For non-aliased deps
|
||||
// this is `(alias, version)`; for aliased deps it's the
|
||||
// alias's own (name, suffix). `link:` deps are skipped.
|
||||
let Some(key) = spec.version.resolved_key(alias) else { continue };
|
||||
if !key.to_string().contains("@runtime:") {
|
||||
continue;
|
||||
}
|
||||
if let Some(meta) = packages.get(&key)
|
||||
&& matches!(
|
||||
&meta.resolution,
|
||||
LockfileResolution::Binary(_) | LockfileResolution::Variations(_),
|
||||
)
|
||||
{
|
||||
skipped.add_optional_excluded(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `None` = compatible. `Some(err)` = incompatible, with the
|
||||
/// diagnostic the caller would surface (the skip's `details` payload
|
||||
/// or the warn / engine-strict error).
|
||||
|
||||
Reference in new issue
Block a user