Feature/fix no symlink (rust-lang#432)

* Refactor `BinFile::from_product`: Simplify logic flow. * Fix `--no-symlink` behavior Signed-off-by: Jiahao XU <[email protected]>
EFanZh · Sep 28, 2022 · c159036 · c159036
1 parent e3cf3e9
commit c159036
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 46 deletions.
diff --git a/crates/binstalk/src/bins.rs b/crates/binstalk/src/bins.rs
@@ -66,14 +66,15 @@ pub struct BinFile {
     pub base_name: CompactString,
     pub source: PathBuf,
     pub dest: PathBuf,
-    pub link: PathBuf,
+    pub link: Option<PathBuf>,
 }
 
 impl BinFile {
     pub fn from_product(
         data: &Data<'_>,
         product: &Product,
         bin_dir: &str,
+        no_symlinks: bool,
     ) -> Result<Self, BinstallError> {
         let base_name = product.name.as_deref().unwrap();
 
@@ -93,42 +94,48 @@ impl BinFile {
             binary_ext,
         };
 
-        // Generate install paths
-        // Source path is the download dir + the generated binary path
-        let path = ctx.render(bin_dir)?;
+        let source = if data.meta.pkg_fmt == Some(PkgFmt::Bin) {
+            data.bin_path.clone()
+        } else {
+            // Generate install paths
+            // Source path is the download dir + the generated binary path
+            let path = ctx.render(bin_dir)?;
 
-        let path_normalized = Path::new(&path).normalize();
+            let path_normalized = Path::new(&path).normalize();
 
-        if path_normalized.components().next().is_none() {
-            return Err(BinstallError::EmptySourceFilePath);
-        }
+            if path_normalized.components().next().is_none() {
+                return Err(BinstallError::EmptySourceFilePath);
+            }
 
-        if !is_valid_path(&path_normalized) {
-            return Err(BinstallError::InvalidSourceFilePath {
-                path: path_normalized.into_owned(),
-            });
-        }
+            if !is_valid_path(&path_normalized) {
+                return Err(BinstallError::InvalidSourceFilePath {
+                    path: path_normalized.into_owned(),
+                });
+            }
 
-        let source_file_path = match path_normalized {
-            Cow::Borrowed(..) => path,
-            Cow::Owned(path) => path.to_string_lossy().into_owned(),
-        };
+            let source_file_path = match path_normalized {
+                Cow::Borrowed(..) => path,
+                Cow::Owned(path) => path.to_string_lossy().into_owned(),
+            };
 
-        let source = if data.meta.pkg_fmt == Some(PkgFmt::Bin) {
-            data.bin_path.clone()
-        } else {
             data.bin_path.join(&source_file_path)
         };
 
-        // Destination path is the install dir + base-name-version{.extension}
-        let dest_file_path = ctx.render("{ bin }-v{ version }{ binary-ext }")?;
-        let dest = data.install_path.join(dest_file_path);
-
-        // Link at install dir + base-name{.extension}
-        let link = data
+        // Destination at install dir + base-name{.extension}
+        let dest = data
             .install_path
             .join(&ctx.render("{ bin }{ binary-ext }")?);
 
+        let (dest, link) = if no_symlinks {
+            (dest, None)
+        } else {
+            // Destination path is the install dir + base-name-version{.extension}
+            let dest_file_path_with_ver = ctx.render("{ bin }-v{ version }{ binary-ext }")?;
+            let dest_with_ver = data.install_path.join(dest_file_path_with_ver);
+
+            (dest_with_ver, Some(dest))
+        };
+
         Ok(Self {
             base_name: CompactString::from(base_name),
             source,
@@ -147,12 +154,16 @@ impl BinFile {
     }
 
     pub fn preview_link(&self) -> String {
-        format!(
-            "{} ({} -> {})",
-            self.base_name,
-            self.link.display(),
-            self.link_dest().display()
-        )
+        if let Some(link) = &self.link {
+            format!(
+                "{} ({} -> {})",
+                self.base_name,
+                link.display(),
+                self.link_dest().display()
+            )
+        } else {
+            String::new()
+        }
     }
 
     /// Return `Ok` if the source exists, otherwise `Err`.
@@ -178,21 +189,23 @@ impl BinFile {
     }
 
     pub fn install_link(&self) -> Result<(), BinstallError> {
-        // Remove existing symlink
-        // TODO: check if existing symlink is correct
-        if self.link.exists() {
-            debug!("Remove link '{}'", self.link.display());
-            std::fs::remove_file(&self.link)?;
+        if let Some(link) = &self.link {
+            // Remove existing symlink
+            // TODO: check if existing symlink is correct
+            if link.exists() {
+                debug!("Remove link '{}'", link.display());
+                std::fs::remove_file(link)?;
+            }
+
+            let dest = self.link_dest();
+            debug!(
+                "Create link '{}' pointing to '{}'",
+                link.display(),
+                dest.display()
+            );
+            atomic_symlink_file(dest, link)?;
         }
 
-        let dest = self.link_dest();
-        debug!(
-            "Create link '{}' pointing to '{}'",
-            self.link.display(),
-            dest.display()
-        );
-        atomic_symlink_file(dest, &self.link)?;
-
         Ok(())
     }
 

diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs
@@ -238,6 +238,7 @@ async fn resolve_inner(
                     &package,
                     &install_path,
                     &binaries,
+                    opts.no_symlinks,
                 )
                 .await
                 {
@@ -281,6 +282,7 @@ async fn download_extract_and_verify(
     package: &Package<Meta>,
     install_path: &Path,
     binaries: &[Product],
+    no_symlinks: bool,
 ) -> Result<Vec<bins::BinFile>, BinstallError> {
     // Build final metadata
     let meta = fetcher.target_meta();
@@ -322,6 +324,7 @@ async fn download_extract_and_verify(
             binaries,
             bin_path.to_path_buf(),
             install_path.to_path_buf(),
+            no_symlinks,
         )?;
 
         for bin_file in bin_files.iter() {
@@ -340,6 +343,7 @@ fn collect_bin_files(
     binaries: &[Product],
     bin_path: PathBuf,
     install_path: PathBuf,
+    no_symlinks: bool,
 ) -> Result<Vec<bins::BinFile>, BinstallError> {
     // List files to be installed
     // based on those found via Cargo.toml
@@ -363,7 +367,7 @@ fn collect_bin_files(
     // Create bin_files
     let bin_files = binaries
         .iter()
-        .map(|p| bins::BinFile::from_product(&bin_data, p, &bin_dir))
+        .map(|p| bins::BinFile::from_product(&bin_data, p, &bin_dir, no_symlinks))
         .collect::<Result<Vec<_>, BinstallError>>()?;
 
     let mut source_set = BTreeSet::new();