From e05b54232c5eeecf1de430d7fe2fc4fe8a549399 Mon Sep 17 00:00:00 2001 From: Chuck Pierce Date: Fri, 19 Feb 2021 17:56:54 -0800 Subject: [PATCH 1/2] Add extra directory when linking scoped package --- crates/volta-core/src/run/executor.rs | 17 ++++++++ crates/volta-core/src/run/npm.rs | 2 +- crates/volta-core/src/run/parser.rs | 9 +++-- crates/volta-core/src/tool/package/mod.rs | 48 +++++++++++++++++------ 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/crates/volta-core/src/run/executor.rs b/crates/volta-core/src/run/executor.rs index 2854a3a91..22bad773b 100644 --- a/crates/volta-core/src/run/executor.rs +++ b/crates/volta-core/src/run/executor.rs @@ -237,6 +237,23 @@ impl PackageInstallCommand { }) } + pub fn for_npm_link(args: A, platform: Platform, name: String) -> Fallible + where + A: IntoIterator, + S: AsRef, + { + let installer = DirectInstall::with_name(PackageManager::Npm, name)?; + + let mut command = create_command("npm"); + command.args(args); + + Ok(PackageInstallCommand { + command, + installer, + platform, + }) + } + /// Adds or updates environment variables that the command will use pub fn envs(&mut self, envs: E) where diff --git a/crates/volta-core/src/run/npm.rs b/crates/volta-core/src/run/npm.rs index 2aee72215..6322ba512 100644 --- a/crates/volta-core/src/run/npm.rs +++ b/crates/volta-core/src/run/npm.rs @@ -35,7 +35,7 @@ pub(super) fn command(args: &[OsString], session: &mut Session) -> Fallible { // For link commands, only intercept if a platform exists if let Some(platform) = Platform::current(session)? { - return link.executor(platform); + return link.executor(platform, current_project_name(session)); } } CommandArg::Intercepted(InterceptedCommand::Unlink) => { diff --git a/crates/volta-core/src/run/parser.rs b/crates/volta-core/src/run/parser.rs index 5a7eeac37..8cc697d48 100644 --- a/crates/volta-core/src/run/parser.rs +++ b/crates/volta-core/src/run/parser.rs @@ -345,12 +345,15 @@ pub struct LinkArgs<'a> { } impl<'a> LinkArgs<'a> { - pub fn executor(self, platform: Platform) -> Fallible { + pub fn executor(self, platform: Platform, project_name: Option) -> Fallible { if self.tools.is_empty() { // If no tools are specified, then this is a bare link command, linking the current // project as a global package. We treat this exactly like a global install - PackageInstallCommand::new(self.common_args, platform, PackageManager::Npm) - .map(Into::into) + match project_name { + Some(name) => PackageInstallCommand::for_npm_link(self.common_args, platform, name), + None => PackageInstallCommand::new(self.common_args, platform, PackageManager::Npm), + } + .map(Into::into) } else { // If there are tools specified, then this represents a command to link a global // package into the current project. We handle each tool separately to support Volta's diff --git a/crates/volta-core/src/tool/package/mod.rs b/crates/volta-core/src/tool/package/mod.rs index bef517e5c..47b120449 100644 --- a/crates/volta-core/src/tool/package/mod.rs +++ b/crates/volta-core/src/tool/package/mod.rs @@ -35,7 +35,7 @@ pub struct Package { impl Package { pub fn new(name: String, version: VersionSpec) -> Fallible { - let staging = setup_staging_directory(PackageManager::Npm)?; + let staging = setup_staging_directory(PackageManager::Npm, false /* needs_scope */)?; Ok(Package { name, @@ -126,13 +126,28 @@ impl Display for Package { pub struct DirectInstall { staging: TempDir, manager: PackageManager, + name: Option, } impl DirectInstall { pub fn new(manager: PackageManager) -> Fallible { - let staging = setup_staging_directory(manager)?; + let staging = setup_staging_directory(manager, false /* needs_scope */)?; - Ok(DirectInstall { staging, manager }) + Ok(DirectInstall { + staging, + manager, + name: None, + }) + } + + pub fn with_name(manager: PackageManager, name: String) -> Fallible { + let staging = setup_staging_directory(manager, name.contains('/'))?; + + Ok(DirectInstall { + staging, + manager, + name: Some(name), + }) } pub fn setup_command(&self, command: &mut Command) { @@ -141,16 +156,20 @@ impl DirectInstall { } pub fn complete_install(self, image: &Image) -> Fallible<()> { - let name = self - .manager - .get_installed_package(self.staging.path().to_owned()) + let DirectInstall { + staging, + name, + manager, + } = self; + + let name = name + .or_else(|| manager.get_installed_package(staging.path().to_owned())) .ok_or(ErrorKind::InstalledPackageNameError)?; - let manifest = - configure::parse_manifest(&name, self.staging.path().to_owned(), self.manager)?; + let manifest = configure::parse_manifest(&name, staging.path().to_owned(), manager)?; - persist_install(&name, &manifest.version, self.staging.path())?; - link_package_to_shared_dir(&name, self.manager)?; - configure::write_config_and_shims(&name, &manifest, image, self.manager) + persist_install(&name, &manifest.version, staging.path())?; + link_package_to_shared_dir(&name, manager)?; + configure::write_config_and_shims(&name, &manifest, image, manager) } } @@ -211,7 +230,7 @@ impl InPlaceUpgrade { /// Create the temporary staging directory we will use to install and ensure expected /// subdirectories exist within it -fn setup_staging_directory(manager: PackageManager) -> Fallible { +fn setup_staging_directory(manager: PackageManager, needs_scope: bool) -> Fallible { // Workaround to ensure relative symlinks continue to work. // The final installed location of packages is: // $VOLTA_HOME/tools/image/packages/{name}/ @@ -219,9 +238,14 @@ fn setup_staging_directory(manager: PackageManager) -> Fallible { // $VOLTA_HOME/tmp/image/packages/{tempdir}/ // This way any relative symlinks will have the same amount of nesting and will remain valid // even when the directory is persisted. + // We also need to handle the case when the linked package has a scope, which requires another + // level of nesting let mut staging_root = volta_home()?.tmp_dir().to_owned(); staging_root.push("image"); staging_root.push("packages"); + if needs_scope { + staging_root.push("scope"); + } create_dir_all(&staging_root).with_context(|| ErrorKind::ContainingDirError { path: staging_root.clone(), })?; From 9787385457cbeec5a55ff8ae9ce1016a744b4e82 Mon Sep 17 00:00:00 2001 From: Charles Pierce Date: Sun, 21 Feb 2021 19:37:21 -0800 Subject: [PATCH 2/2] Clean up implementation based on PR comments --- crates/volta-core/src/run/parser.rs | 3 ++- crates/volta-core/src/tool/package/mod.rs | 29 +++++++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/crates/volta-core/src/run/parser.rs b/crates/volta-core/src/run/parser.rs index 8cc697d48..45df0819c 100644 --- a/crates/volta-core/src/run/parser.rs +++ b/crates/volta-core/src/run/parser.rs @@ -348,7 +348,8 @@ impl<'a> LinkArgs<'a> { pub fn executor(self, platform: Platform, project_name: Option) -> Fallible { if self.tools.is_empty() { // If no tools are specified, then this is a bare link command, linking the current - // project as a global package. We treat this exactly like a global install + // project as a global package. We treat this like a global install except we look up + // the name from the current directory first. match project_name { Some(name) => PackageInstallCommand::for_npm_link(self.common_args, platform, name), None => PackageInstallCommand::new(self.common_args, platform, PackageManager::Npm), diff --git a/crates/volta-core/src/tool/package/mod.rs b/crates/volta-core/src/tool/package/mod.rs index 47b120449..c8ae39fc5 100644 --- a/crates/volta-core/src/tool/package/mod.rs +++ b/crates/volta-core/src/tool/package/mod.rs @@ -35,7 +35,7 @@ pub struct Package { impl Package { pub fn new(name: String, version: VersionSpec) -> Fallible { - let staging = setup_staging_directory(PackageManager::Npm, false /* needs_scope */)?; + let staging = setup_staging_directory(PackageManager::Npm, NeedsScope::No)?; Ok(Package { name, @@ -123,6 +123,9 @@ impl Display for Package { /// /// Provides methods to simplify installing into a staging directory and then moving that install /// into the proper location after it is complete. +/// +/// Note: We don't always know the name of the package up-front, as the install could be from a +/// tarball or a git coordinate. If we do know ahead of time, then we can skip looking it up pub struct DirectInstall { staging: TempDir, manager: PackageManager, @@ -131,7 +134,7 @@ pub struct DirectInstall { impl DirectInstall { pub fn new(manager: PackageManager) -> Fallible { - let staging = setup_staging_directory(manager, false /* needs_scope */)?; + let staging = setup_staging_directory(manager, NeedsScope::No)?; Ok(DirectInstall { staging, @@ -141,7 +144,7 @@ impl DirectInstall { } pub fn with_name(manager: PackageManager, name: String) -> Fallible { - let staging = setup_staging_directory(manager, name.contains('/'))?; + let staging = setup_staging_directory(manager, name.contains('/').into())?; Ok(DirectInstall { staging, @@ -228,9 +231,25 @@ impl InPlaceUpgrade { } } +#[derive(Clone, Copy, PartialEq)] +enum NeedsScope { + Yes, + No, +} + +impl From for NeedsScope { + fn from(value: bool) -> Self { + if value { + NeedsScope::Yes + } else { + NeedsScope::No + } + } +} + /// Create the temporary staging directory we will use to install and ensure expected /// subdirectories exist within it -fn setup_staging_directory(manager: PackageManager, needs_scope: bool) -> Fallible { +fn setup_staging_directory(manager: PackageManager, needs_scope: NeedsScope) -> Fallible { // Workaround to ensure relative symlinks continue to work. // The final installed location of packages is: // $VOLTA_HOME/tools/image/packages/{name}/ @@ -243,7 +262,7 @@ fn setup_staging_directory(manager: PackageManager, needs_scope: bool) -> Fallib let mut staging_root = volta_home()?.tmp_dir().to_owned(); staging_root.push("image"); staging_root.push("packages"); - if needs_scope { + if needs_scope == NeedsScope::Yes { staging_root.push("scope"); } create_dir_all(&staging_root).with_context(|| ErrorKind::ContainingDirError {