From c15903684f8b9cdf1105b788e86c8fea490a7091 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 28 Sep 2022 12:32:22 +1000 Subject: [PATCH] Feature/fix no symlink (#432) * Refactor `BinFile::from_product`: Simplify logic flow. * Fix `--no-symlink` behavior Signed-off-by: Jiahao XU --- crates/binstalk/src/bins.rs | 103 ++++++++++++++++------------- crates/binstalk/src/ops/resolve.rs | 6 +- 2 files changed, 63 insertions(+), 46 deletions(-) diff --git a/crates/binstalk/src/bins.rs b/crates/binstalk/src/bins.rs index 9fd74d408..181be24d4 100644 --- a/crates/binstalk/src/bins.rs +++ b/crates/binstalk/src/bins.rs @@ -66,7 +66,7 @@ pub struct BinFile { pub base_name: CompactString, pub source: PathBuf, pub dest: PathBuf, - pub link: PathBuf, + pub link: Option, } impl BinFile { @@ -74,6 +74,7 @@ impl BinFile { data: &Data<'_>, product: &Product, bin_dir: &str, + no_symlinks: bool, ) -> Result { 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 index d5a16b992..330ac7ab3 100644 --- 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, install_path: &Path, binaries: &[Product], + no_symlinks: bool, ) -> Result, 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, 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::, BinstallError>>()?; let mut source_set = BTreeSet::new();