Clean up implementation based on PR comments

volta-cli · Feb 22, 2021 · 9787385 · 9787385
1 parent e05b542
commit 9787385
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 6 deletions.
diff --git 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<String>) -> Fallible<Executor> {
         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
@@ -35,7 +35,7 @@ pub struct Package {
 
 impl Package {
     pub fn new(name: String, version: VersionSpec) -> Fallible<Self> {
-        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<Self> {
-        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<Self> {
-        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<bool> 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<TempDir> {
+fn setup_staging_directory(manager: PackageManager, needs_scope: NeedsScope) -> Fallible<TempDir> {
     // 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 {