diff --git a/Cargo.lock b/Cargo.lock index e1402e365..3f634b69e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "adler" @@ -1657,7 +1657,7 @@ dependencies = [ "volta-core", "volta-migrate", "which", - "winreg", + "winreg 0.53.0", ] [[package]] @@ -1701,7 +1701,7 @@ dependencies = [ "volta-layout", "walkdir", "which", - "winreg", + "winreg 0.55.0", ] [[package]] @@ -2015,6 +2015,16 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "winreg" +version = "0.55.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb5a765337c50e9ec252c2069be9bf91c7df47afb103b642ba3a53bf8101be97" +dependencies = [ + "cfg-if", + "windows-sys 0.59.0", +] + [[package]] name = "winsafe" version = "0.0.19" diff --git a/crates/volta-core/src/tool/package/mod.rs b/crates/volta-core/src/tool/package/mod.rs index c8ae39fc5..181d1359e 100644 --- a/crates/volta-core/src/tool/package/mod.rs +++ b/crates/volta-core/src/tool/package/mod.rs @@ -1,5 +1,9 @@ use std::fmt::{self, Display}; +#[cfg(unix)] +use std::fs; use std::fs::create_dir_all; +#[cfg(unix)] +use std::io; use std::path::{Path, PathBuf}; use std::process::Command; @@ -299,9 +303,90 @@ where rename(staging_dir, &package_dir).with_context(|| ErrorKind::SetupToolImageError { tool: package_name.into(), version: package_version.to_string(), - dir: package_dir, + dir: package_dir.clone(), })?; + repair_moved_bin_symlinks(staging_dir, &package_dir, package_name, package_version)?; + + Ok(()) +} + +#[cfg(unix)] +fn repair_moved_bin_symlinks( + staging_dir: &Path, + package_dir: &Path, + package_name: &str, + package_version: V, +) -> Fallible<()> +where + V: Display, +{ + let bin_dir = package_dir.join("bin"); + let entries = match fs::read_dir(&bin_dir) { + Ok(entries) => entries, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(()), + Err(error) => { + return Err(error).with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + }) + } + }; + + for entry in entries { + let entry = entry.with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + })?; + let metadata = + fs::symlink_metadata(entry.path()).with_context(|| ErrorKind::ReadDirError { + dir: bin_dir.clone(), + })?; + + if !metadata.file_type().is_symlink() { + continue; + } + + let target = + fs::read_link(entry.path()).with_context(|| ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + })?; + + if let Ok(relative_target) = target.strip_prefix(staging_dir) { + let repaired_target = package_dir.join(relative_target); + if !repaired_target.exists() { + return Err(ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + } + .into()); + } + + crate::fs::remove_file_if_exists(entry.path())?; + crate::fs::symlink_file(repaired_target, entry.path()).with_context(|| { + ErrorKind::SetupToolImageError { + tool: package_name.into(), + version: package_version.to_string(), + dir: package_dir.to_owned(), + } + })?; + } + } + + Ok(()) +} + +#[cfg(windows)] +fn repair_moved_bin_symlinks( + _staging_dir: &Path, + _package_dir: &Path, + _package_name: &str, + _package_version: V, +) -> Fallible<()> +where + V: Display, +{ Ok(()) } @@ -323,3 +408,135 @@ fn link_package_to_shared_dir(package_name: &str, manager: PackageManager) -> Fa name: package_name.into(), }) } + +#[cfg(all(test, unix))] +mod tests { + use super::repair_moved_bin_symlinks; + use crate::fs::symlink_file; + use std::fs; + use tempfile::tempdir; + + #[test] + fn repairs_bin_symlink_that_points_into_staging_dir() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let staging_target = staging_dir.join("lib/node_modules/volta-test/bin/native"); + let final_target = package_dir.join("lib/node_modules/volta-test/bin/native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(staging_target.parent().unwrap()).expect("create staging target dir"); + fs::create_dir_all(final_target.parent().unwrap()).expect("create final target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&staging_target, "#!/usr/bin/env node\n").expect("write staging target"); + fs::write(&final_target, "#!/usr/bin/env node\n").expect("write final target"); + symlink_file(&staging_target, &bin_link).expect("create staging symlink"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_link(&bin_link).expect("read repaired link"), + final_target + ); + } + + #[test] + fn leaves_non_symlink_bin_entries_unchanged() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let bin_file = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(bin_file.parent().unwrap()).expect("create package bin dir"); + fs::write(&bin_file, "#!/usr/bin/env node\n").expect("write bin file"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_to_string(&bin_file).expect("read bin file"), + "#!/usr/bin/env node\n" + ); + } + + #[test] + fn leaves_symlink_targets_outside_staging_dir_unchanged() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let external_target = temp.path().join("external").join("native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(external_target.parent().unwrap()).expect("create external target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&external_target, "#!/usr/bin/env node\n").expect("write external target"); + symlink_file(&external_target, &bin_link).expect("create external symlink"); + + repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0") + .expect("repair moved symlink"); + + assert_eq!( + fs::read_link(&bin_link).expect("read unchanged link"), + external_target + ); + } + + #[test] + fn errors_when_repaired_symlink_target_does_not_exist() { + let temp = tempdir().expect("create temp dir"); + let staging_dir = temp + .path() + .join("tmp") + .join("image") + .join("packages") + .join("staging"); + let package_dir = temp + .path() + .join("tools") + .join("image") + .join("packages") + .join("volta-test"); + let staging_target = staging_dir.join("lib/node_modules/volta-test/bin/native"); + let bin_link = package_dir.join("bin").join("volta-test"); + + fs::create_dir_all(staging_target.parent().unwrap()).expect("create staging target dir"); + fs::create_dir_all(bin_link.parent().unwrap()).expect("create package bin dir"); + fs::write(&staging_target, "#!/usr/bin/env node\n").expect("write staging target"); + symlink_file(&staging_target, &bin_link).expect("create staging symlink"); + + let result = repair_moved_bin_symlinks(&staging_dir, &package_dir, "volta-test", "1.0.0"); + + assert!(result.is_err()); + } +} diff --git a/tests/acceptance/support/sandbox.rs b/tests/acceptance/support/sandbox.rs index 25e6ab176..2f3cc5ee9 100644 --- a/tests/acceptance/support/sandbox.rs +++ b/tests/acceptance/support/sandbox.rs @@ -857,6 +857,7 @@ impl Sandbox { .env("PATH", &self.path) .env("VOLTA_POSTSCRIPT", volta_postscript()) .env_remove("VOLTA_SHELL") + .env_remove("_VOLTA_TOOL_RECURSION") .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows // overrides for env vars diff --git a/tests/smoke/direct_install.rs b/tests/smoke/direct_install.rs index 9b712e471..b06c09782 100644 --- a/tests/smoke/direct_install.rs +++ b/tests/smoke/direct_install.rs @@ -3,6 +3,43 @@ use hamcrest2::assert_that; use hamcrest2::prelude::*; use test_support::matchers::execs; +const ABSOLUTE_BIN_PACKAGE_JSON: &str = r#"{ + "name": "volta-absolute-bin", + "version": "1.0.0", + "bin": { + "volta-absolute-bin": "bin/wrapper.js" + }, + "scripts": { + "postinstall": "node scripts/postinstall.js" + } +}"#; + +const ABSOLUTE_BIN_WRAPPER: &str = r#"#!/usr/bin/env node +console.log("wrapper bin"); +"#; + +const ABSOLUTE_BIN_NATIVE: &str = r#"#!/usr/bin/env node +console.log("absolute bin successful"); +"#; + +const ABSOLUTE_BIN_POSTINSTALL: &str = r#"const fs = require("fs"); +const path = require("path"); + +const bin = path.join(process.env.npm_config_prefix, "bin", "volta-absolute-bin"); +const target = path.join(__dirname, "..", "bin", "native.js"); + +try { + fs.unlinkSync(bin); +} catch (error) { + if (error.code !== "ENOENT") { + throw error; + } +} + +fs.chmodSync(target, 0o755); +fs.symlinkSync(target, bin); +"#; + #[test] fn npm_global_install() { let p = temp_project().build(); @@ -42,6 +79,43 @@ fn npm_global_install() { ); } +#[test] +fn npm_global_install_repairs_absolute_bin_symlink() { + let p = temp_project() + .project_file("absolute-bin/package.json", ABSOLUTE_BIN_PACKAGE_JSON) + .project_file("absolute-bin/bin/wrapper.js", ABSOLUTE_BIN_WRAPPER) + .project_file("absolute-bin/bin/native.js", ABSOLUTE_BIN_NATIVE) + .project_file( + "absolute-bin/scripts/postinstall.js", + ABSOLUTE_BIN_POSTINSTALL, + ) + .build(); + + // Have to install node to ensure npm is available + assert_that!(p.volta("install node@14.3.0"), execs().with_status(0)); + + let package_path = p.root().join("absolute-bin"); + assert_that!( + p.npm(&format!("pack {}", package_path.display())), + execs().with_status(0) + ); + + let tarball_path = p.root().join("volta-absolute-bin-1.0.0.tgz"); + assert_that!( + p.npm(&format!("install --global {}", tarball_path.display())), + execs().with_status(0) + ); + + assert!(p.shim_exists("volta-absolute-bin")); + assert!(p.package_is_installed("volta-absolute-bin")); + assert_that!( + p.exec_shim("volta-absolute-bin", ""), + execs() + .with_status(0) + .with_stdout_contains("absolute bin successful") + ); +} + #[test] fn yarn_global_add() { let p = temp_project().build(); diff --git a/tests/smoke/support/temp_project.rs b/tests/smoke/support/temp_project.rs index 4be937647..ec6aad4b1 100644 --- a/tests/smoke/support/temp_project.rs +++ b/tests/smoke/support/temp_project.rs @@ -289,6 +289,7 @@ impl TempProject { .env("VOLTA_HOME", volta_home(self.root())) .env("VOLTA_INSTALL_DIR", cargo_dir()) .env_remove("VOLTA_NODE_VERSION") + .env_remove("_VOLTA_TOOL_RECURSION") .env_remove("MSYSTEM"); // assume cmd.exe everywhere on windows // overrides for env vars