diff --git a/Cargo.lock b/Cargo.lock index cb1f0341fc..6dc6c96fde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2087,6 +2087,7 @@ dependencies = [ "pacquet-package-manifest", "pacquet-registry", "pacquet-reporter", + "pacquet-resolving-parse-wanted-dependency", "pacquet-store-dir", "pacquet-tarball", "pacquet-testing-utils", diff --git a/pacquet/crates/cli/Cargo.toml b/pacquet/crates/cli/Cargo.toml index 3eb6672e37..69ad404cc4 100644 --- a/pacquet/crates/cli/Cargo.toml +++ b/pacquet/crates/cli/Cargo.toml @@ -27,6 +27,7 @@ pacquet-package-manager = { workspace = true } pacquet-package-is-installable = { workspace = true } pacquet-registry = { workspace = true } pacquet-reporter = { workspace = true } +pacquet-resolving-parse-wanted-dependency = { workspace = true } pacquet-tarball = { workspace = true } pacquet-diagnostics = { workspace = true } diff --git a/pacquet/crates/cli/src/cli_args/dlx.rs b/pacquet/crates/cli/src/cli_args/dlx.rs index e377e12e25..32439d298c 100644 --- a/pacquet/crates/cli/src/cli_args/dlx.rs +++ b/pacquet/crates/cli/src/cli_args/dlx.rs @@ -8,6 +8,7 @@ use pacquet_crypto_hash::create_short_hash; use pacquet_fs::force_symlink_dir; use pacquet_package_manifest::DependencyGroup; use pacquet_reporter::Reporter; +use pacquet_resolving_parse_wanted_dependency::parse_wanted_dependency; use serde_json::{Value, json}; use std::{ collections::BTreeMap, @@ -163,7 +164,16 @@ impl DlxArgs { None => { let prepare_dir = get_prepare_dir(&dlx_command_cache_dir, SystemTime::now(), std::process::id()); - install_into_cache::(&prepare_dir, &pkgs, &allow_build, config).await?; + if let Err(error) = + install_into_cache::(&prepare_dir, &pkgs, &allow_build, config).await + { + // Don't leave a half-installed prepare dir behind to + // accumulate across failed runs. Mirrors pnpm's + // `fs.rm(cachedDir, { recursive: true, force: true })` + // on install failure (dlx.ts). Best-effort cleanup. + let _ = fs::remove_dir_all(&prepare_dir); + return Err(error); + } // Best-effort: a parallel dlx process may have raced // us to the link. Either link is equally fresh, so // ignore the failure and run from our own prepare dir. @@ -210,8 +220,22 @@ async fn install_into_cache( // The cache install is always fresh, so no lockfile is loaded from // the process working directory. config.lockfile = false; - // Allow requested packages to run build scripts during the install, - // mirroring pnpm's dlx `allowBuilds` handling. + // Build a *fresh* allow-list for the throwaway install — the dlx + // packages themselves plus the CLI `--allow-build` entries — rather + // than inheriting the caller project's `allow_builds` / + // `dangerously_allow_all_builds`. Mirrors pnpm's + // `allowBuilds = Object.fromEntries([...resolvedPkgAliases, ...allowBuild])` + // (dlx.ts:168). Inheriting the caller's policy would run build + // scripts the dlx invocation never opted into, and would also leave + // the cache key (which hashes only pkgs + CLI allow_build) unable to + // distinguish two callers with different policies. + config.dangerously_allow_all_builds = false; + config.allow_builds.clear(); + for spec in pkgs { + if let Some(alias) = parse_wanted_dependency(spec).alias { + config.allow_builds.insert(alias, true); + } + } for name in allow_build { config.allow_builds.insert(name.clone(), true); } @@ -359,8 +383,14 @@ fn get_bin_name(cached_dir: &Path) -> Result { [] => Err(DlxError::NoBin { package: pkg_name }), [bin] => Ok(bin.name.clone()), bins => { - let scopeless = scopeless(&pkg_name); - if let Some(bin) = bins.iter().find(|bin| bin.name == scopeless) { + // The default bin is the one named after the installed + // package's own `name` field (scopeless), not the dependency + // alias it was installed under. Mirrors `scopeless(manifest.name)` + // at dlx.ts:286 — they differ for aliased specs such as + // `foo@npm:@scope/realtool`. + let manifest_name = manifest.get("name").and_then(Value::as_str).unwrap_or(&pkg_name); + let scopeless_name = scopeless(manifest_name); + if let Some(bin) = bins.iter().find(|bin| bin.name == scopeless_name) { return Ok(bin.name.clone()); } let names = bins.iter().map(|bin| bin.name.as_str()).collect::>().join(", "); diff --git a/pacquet/crates/cli/src/cli_args/dlx/tests.rs b/pacquet/crates/cli/src/cli_args/dlx/tests.rs index 04b192f41f..eec58dd585 100644 --- a/pacquet/crates/cli/src/cli_args/dlx/tests.rs +++ b/pacquet/crates/cli/src/cli_args/dlx/tests.rs @@ -143,6 +143,22 @@ fn get_bin_name_picks_the_scopeless_match_among_many() { assert_eq!(get_bin_name(dir.path()).expect("bin name"), "tool"); } +#[test] +fn get_bin_name_uses_installed_manifest_name_not_alias() { + // The dependency key (`alias`) differs from the installed package's + // own `name`. The default bin among many is selected by the manifest + // name (`scopeless("@scope/realtool")` == "realtool"), not the alias. + let dir = cached_dir_with( + "alias", + serde_json::json!({ + "name": "@scope/realtool", + "version": "1.0.0", + "bin": { "realtool": "r.js", "other": "o.js" }, + }), + ); + assert_eq!(get_bin_name(dir.path()).expect("bin name"), "realtool"); +} + #[test] fn get_bin_name_errors_when_no_dependency() { let dir = tempdir().expect("temp dir"); diff --git a/pacquet/crates/cli/src/cli_args/run.rs b/pacquet/crates/cli/src/cli_args/run.rs index 822e9b6530..cd047219db 100644 --- a/pacquet/crates/cli/src/cli_args/run.rs +++ b/pacquet/crates/cli/src/cli_args/run.rs @@ -200,6 +200,13 @@ fn run_stage( if args.is_empty() && script == "npx only-allow pnpm" { return Ok(()); } + // An empty script body is a no-op under pnpm, which skips any stage + // whose (post-arg) command is falsy (runLifecycleHook.ts:100). pnpm + // also gates pre/post on the body being truthy (run.ts:403,411), so + // an empty `pre`/`post` never runs. + if script.is_empty() { + return Ok(()); + } let status = run_script(RunScript { manifest: ctx.manifest.value(), @@ -223,7 +230,14 @@ fn run_stage( if !status.success() { let code = status.code().unwrap_or(1); - eprintln!("[ELIFECYCLE] Command failed with exit code {code}."); + // pnpm's reportLifecycleError special-cases the `test` stage + // (reportError.ts:371-378): "Test failed. See above for more + // details." rather than the exit-code line. + if stage == "test" { + eprintln!("[ELIFECYCLE] Test failed. See above for more details."); + } else { + eprintln!("[ELIFECYCLE] Command failed with exit code {code}."); + } std::process::exit(code); } Ok(()) diff --git a/pacquet/crates/cli/tests/run.rs b/pacquet/crates/cli/tests/run.rs index 025b370587..493059fa76 100644 --- a/pacquet/crates/cli/tests/run.rs +++ b/pacquet/crates/cli/tests/run.rs @@ -228,6 +228,32 @@ fn run_propagates_failing_script_exit_code() { drop(root); } +/// A failing `test` script prints pnpm's stage-specific lifecycle error +/// (`Test failed. See above for more details.`) rather than the generic +/// exit-code line, matching reportLifecycleError's `test` special case. +#[cfg_attr(target_os = "windows", ignore = "uses a POSIX shell `exit` builtin")] +#[test] +fn run_failing_test_script_prints_test_failed_message() { + let CommandTempCwd { pacquet, root, workspace, .. } = CommandTempCwd::init(); + let manifest = json!({ + "name": "test", + "version": "0.0.0", + "scripts": { "test": "exit 1" }, + }) + .to_string(); + fs::write(workspace.join("package.json"), manifest).expect("write package.json"); + + let output = pacquet.with_arg("run").with_arg("test").output().expect("spawn pacquet run"); + assert_eq!(output.status.code(), Some(1), "the script's exit code must propagate"); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Test failed. See above for more details."), + "test-stage failure should print pnpm's test message:\n{stderr}", + ); + + drop(root); +} + /// A script that invokes a locally-installed binary resolves it through /// `node_modules/.bin`, which `pnpm run` prepends to `PATH`. #[cfg(unix)]