feat: Add new cmdline option --no-track (#1111)

Same as `cargo-install`'s `--no-track`.
It is also passed to `cargo-install` if it is invoked.

Also fixed `fs::atomic_symlink_file` which on Windows could fallback to
non-atomic install if symlinking failed.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
This commit is contained in:
Jiahao XU 2023-06-03 19:15:18 +10:00 committed by GitHub
parent a849db3ef4
commit 1432093dcc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 194 additions and 43 deletions

View file

@ -172,6 +172,21 @@ pub struct Args {
#[clap(help_heading = "Options", long)] #[clap(help_heading = "Options", long)]
pub no_cleanup: bool, pub no_cleanup: bool,
/// By default, binstall keeps track of the installed packages with metadata files
/// stored in the installation root directory.
///
/// This flag tells binstall not to use or create that file.
///
/// With this flag, binstall will refuse to overwrite any existing files unless the
/// `--force` flag is used.
///
/// This also disables binstalls ability to protect against multiple concurrent
/// invocations of binstall installing at the same time.
///
/// This flag will also be passed to `cargo-install` if it is invoked.
#[clap(help_heading = "Options", long)]
pub no_track: bool,
/// Install binaries in a custom location. /// Install binaries in a custom location.
/// ///
/// By default, binaries are installed to the global location `$CARGO_HOME/bin`, and global /// By default, binaries are installed to the global location `$CARGO_HOME/bin`, and global

View file

@ -59,7 +59,7 @@ pub fn install_crates(
// Compute paths // Compute paths
let cargo_root = args.root; let cargo_root = args.root;
let (install_path, mut manifests, temp_dir) = let (install_path, mut manifests, temp_dir) =
compute_paths_and_load_manifests(cargo_root.clone(), args.install_path)?; compute_paths_and_load_manifests(cargo_root.clone(), args.install_path, args.no_track)?;
// Remove installed crates // Remove installed crates
let mut crate_names = let mut crate_names =
@ -101,6 +101,7 @@ pub fn install_crates(
force: args.force, force: args.force,
quiet: args.log_level == Some(LevelFilter::Off), quiet: args.log_level == Some(LevelFilter::Off),
locked: args.locked, locked: args.locked,
no_track: args.no_track,
version_req: args.version_req, version_req: args.version_req,
manifest_path: args.manifest_path, manifest_path: args.manifest_path,
@ -234,6 +235,7 @@ fn read_root_certs(root_certificate_paths: Vec<PathBuf>) -> impl Iterator<Item =
fn compute_paths_and_load_manifests( fn compute_paths_and_load_manifests(
roots: Option<PathBuf>, roots: Option<PathBuf>,
install_path: Option<PathBuf>, install_path: Option<PathBuf>,
no_track: bool,
) -> Result<(PathBuf, Option<Manifests>, tempfile::TempDir)> { ) -> Result<(PathBuf, Option<Manifests>, tempfile::TempDir)> {
// Compute cargo_roots // Compute cargo_roots
let cargo_roots = install_path::get_cargo_roots_path(roots).ok_or_else(|| { let cargo_roots = install_path::get_cargo_roots_path(roots).ok_or_else(|| {
@ -251,8 +253,10 @@ fn compute_paths_and_load_manifests(
fs::create_dir_all(&install_path).map_err(BinstallError::Io)?; fs::create_dir_all(&install_path).map_err(BinstallError::Io)?;
debug!("Using install path: {}", install_path.display()); debug!("Using install path: {}", install_path.display());
let no_manifests = no_track || custom_install_path;
// Load manifests // Load manifests
let manifests = if !custom_install_path { let manifests = if !no_manifests {
Some(Manifests::open_exclusive(&cargo_roots)?) Some(Manifests::open_exclusive(&cargo_roots)?)
} else { } else {
None None

View file

@ -11,7 +11,10 @@ use tracing::debug;
use crate::{ use crate::{
errors::BinstallError, errors::BinstallError,
fs::{atomic_install, atomic_symlink_file}, fs::{
atomic_install, atomic_install_noclobber, atomic_symlink_file,
atomic_symlink_file_noclobber,
},
helpers::download::ExtractedFiles, helpers::download::ExtractedFiles,
manifests::cargo_toml_binstall::{PkgFmt, PkgMeta}, manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
}; };
@ -180,28 +183,48 @@ impl BinFile {
} }
} }
pub fn install_bin(&self) -> Result<(), BinstallError> { fn pre_install_bin(&self) -> Result<(), BinstallError> {
if !self.source.try_exists()? { if !self.source.try_exists()? {
return Err(BinstallError::BinFileNotFound(self.source.clone())); return Err(BinstallError::BinFileNotFound(self.source.clone()));
} }
debug!(
"Atomically install file from '{}' to '{}'",
self.source.display(),
self.dest.display()
);
#[cfg(unix)] #[cfg(unix)]
std::fs::set_permissions( std::fs::set_permissions(
&self.source, &self.source,
std::os::unix::fs::PermissionsExt::from_mode(0o755), std::os::unix::fs::PermissionsExt::from_mode(0o755),
)?; )?;
Ok(())
}
pub fn install_bin(&self) -> Result<(), BinstallError> {
self.pre_install_bin()?;
debug!(
"Atomically install file from '{}' to '{}'",
self.source.display(),
self.dest.display()
);
atomic_install(&self.source, &self.dest)?; atomic_install(&self.source, &self.dest)?;
Ok(()) Ok(())
} }
pub fn install_bin_noclobber(&self) -> Result<(), BinstallError> {
self.pre_install_bin()?;
debug!(
"Installing file from '{}' to '{}' only if dst not exists",
self.source.display(),
self.dest.display()
);
atomic_install_noclobber(&self.source, &self.dest)?;
Ok(())
}
pub fn install_link(&self) -> Result<(), BinstallError> { pub fn install_link(&self) -> Result<(), BinstallError> {
if let Some(link) = &self.link { if let Some(link) = &self.link {
let dest = self.link_dest(); let dest = self.link_dest();
@ -216,6 +239,20 @@ impl BinFile {
Ok(()) Ok(())
} }
pub fn install_link_noclobber(&self) -> Result<(), BinstallError> {
if let Some(link) = &self.link {
let dest = self.link_dest();
debug!(
"Create link '{}' pointing to '{}' only if dst not exists",
link.display(),
dest.display()
);
atomic_symlink_file_noclobber(dest, link)?;
}
Ok(())
}
fn link_dest(&self) -> &Path { fn link_dest(&self) -> &Path {
if cfg!(target_family = "unix") { if cfg!(target_family = "unix") {
Path::new(self.dest.file_name().unwrap()) Path::new(self.dest.file_name().unwrap())

View file

@ -3,6 +3,54 @@ use std::{fs, io, path::Path};
use tempfile::{NamedTempFile, TempPath}; use tempfile::{NamedTempFile, TempPath};
use tracing::{debug, warn}; use tracing::{debug, warn};
fn copy_to_tempfile(src: &Path, dst: &Path) -> io::Result<NamedTempFile> {
let mut src_file = fs::File::open(src)?;
let parent = dst.parent().unwrap();
debug!("Creating named tempfile at '{}'", parent.display());
let mut tempfile = NamedTempFile::new_in(parent)?;
debug!(
"Copying from '{}' to '{}'",
src.display(),
tempfile.path().display()
);
io::copy(&mut src_file, tempfile.as_file_mut())?;
debug!("Retrieving permissions of '{}'", src.display());
let permissions = src_file.metadata()?.permissions();
debug!(
"Setting permissions of '{}' to '{permissions:#?}'",
tempfile.path().display()
);
tempfile.as_file().set_permissions(permissions)?;
Ok(tempfile)
}
/// Install a file.
///
/// This is a blocking function, must be called in `block_in_place` mode.
pub fn atomic_install_noclobber(src: &Path, dst: &Path) -> io::Result<()> {
debug!(
"Attempting to rename from '{}' to '{}'.",
src.display(),
dst.display()
);
let tempfile = copy_to_tempfile(src, dst)?;
debug!(
"Persisting '{}' to '{}', fail if dst already exists",
tempfile.path().display(),
dst.display()
);
tempfile.persist_noclobber(dst)?;
Ok(())
}
/// Atomically install a file. /// Atomically install a file.
/// ///
/// This is a blocking function, must be called in `block_in_place` mode. /// This is a blocking function, must be called in `block_in_place` mode.
@ -33,29 +81,7 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> {
// Fallback to creating NamedTempFile on the parent dir of // Fallback to creating NamedTempFile on the parent dir of
// dst. // dst.
let mut src_file = fs::File::open(src)?; persist(copy_to_tempfile(src, dst)?.into_temp_path(), dst)?;
let parent = dst.parent().unwrap();
debug!("Creating named tempfile at '{}'", parent.display());
let mut tempfile = NamedTempFile::new_in(parent)?;
debug!(
"Copying from '{}' to '{}'",
src.display(),
tempfile.path().display()
);
io::copy(&mut src_file, tempfile.as_file_mut())?;
debug!("Retrieving permissions of '{}'", src.display());
let permissions = src_file.metadata()?.permissions();
debug!(
"Setting permissions of '{}' to '{permissions:#?}'",
tempfile.path().display()
);
tempfile.as_file().set_permissions(permissions)?;
persist(tempfile.into_temp_path(), dst)?;
} else { } else {
debug!("Attempting at atomically succeeded."); debug!("Attempting at atomically succeeded.");
} }
@ -63,17 +89,30 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> {
Ok(()) Ok(())
} }
fn symlink_file(original: &Path, link: &Path) -> io::Result<()> { fn symlink_file_inner(dest: &Path, link: &Path) -> io::Result<()> {
#[cfg(target_family = "unix")] #[cfg(target_family = "unix")]
std::os::unix::fs::symlink(original, link)?; std::os::unix::fs::symlink(dest, link)?;
// Symlinks on Windows are disabled in some editions, so creating one is unreliable.
#[cfg(target_family = "windows")] #[cfg(target_family = "windows")]
std::os::windows::fs::symlink_file(original, link) std::os::windows::fs::symlink_file(dest, link)?;
.or_else(|_| std::fs::copy(original, link).map(drop))?;
Ok(()) Ok(())
} }
pub fn atomic_symlink_file_noclobber(dest: &Path, link: &Path) -> io::Result<()> {
match symlink_file_inner(dest, link) {
Ok(_) => Ok(()),
#[cfg(target_family = "windows")]
// Symlinks on Windows are disabled in some editions, so creating one is unreliable.
// Fallback to copy if it fails.
Err(_) => atomic_install_noclobber(dest, link),
#[cfg(not(target_family = "windows"))]
Err(err) => Err(err),
}
}
/// Atomically install symlink "link" to a file "dst". /// Atomically install symlink "link" to a file "dst".
/// ///
/// This is a blocking function, must be called in `block_in_place` mode. /// This is a blocking function, must be called in `block_in_place` mode.
@ -82,6 +121,8 @@ pub fn atomic_symlink_file(dest: &Path, link: &Path) -> io::Result<()> {
debug!("Creating tempPath at '{}'", parent.display()); debug!("Creating tempPath at '{}'", parent.display());
let temp_path = NamedTempFile::new_in(parent)?.into_temp_path(); let temp_path = NamedTempFile::new_in(parent)?.into_temp_path();
// Remove this file so that we can create a symlink
// with the name.
fs::remove_file(&temp_path)?; fs::remove_file(&temp_path)?;
debug!( debug!(
@ -89,9 +130,18 @@ pub fn atomic_symlink_file(dest: &Path, link: &Path) -> io::Result<()> {
temp_path.display(), temp_path.display(),
dest.display() dest.display()
); );
symlink_file(dest, &temp_path)?;
persist(temp_path, link) match symlink_file_inner(dest, &temp_path) {
Ok(_) => persist(temp_path, link),
#[cfg(target_family = "windows")]
// Symlinks on Windows are disabled in some editions, so creating one is unreliable.
// Fallback to copy if it fails.
Err(_) => atomic_install(dest, link),
#[cfg(not(target_family = "windows"))]
Err(err) => Err(err),
}
} }
fn persist(temp_path: TempPath, to: &Path) -> io::Result<()> { fn persist(temp_path: TempPath, to: &Path) -> io::Result<()> {

View file

@ -25,6 +25,7 @@ pub struct Options {
pub force: bool, pub force: bool,
pub quiet: bool, pub quiet: bool,
pub locked: bool, pub locked: bool,
pub no_track: bool,
pub version_req: Option<VersionReq>, pub version_req: Option<VersionReq>,
pub manifest_path: Option<PathBuf>, pub manifest_path: Option<PathBuf>,

View file

@ -51,15 +51,26 @@ impl Resolution {
impl ResolutionFetch { impl ResolutionFetch {
pub fn install(self, opts: &Options) -> Result<CrateInfo, BinstallError> { pub fn install(self, opts: &Options) -> Result<CrateInfo, BinstallError> {
type InstallFp = fn(&bins::BinFile) -> Result<(), BinstallError>;
let (install_bin, install_link): (InstallFp, InstallFp) = match (opts.no_track, opts.force)
{
(true, true) | (false, _) => (bins::BinFile::install_bin, bins::BinFile::install_link),
(true, false) => (
bins::BinFile::install_bin_noclobber,
bins::BinFile::install_link_noclobber,
),
};
info!("Installing binaries..."); info!("Installing binaries...");
for file in &self.bin_files { for file in &self.bin_files {
file.install_bin()?; install_bin(file)?;
} }
// Generate symlinks // Generate symlinks
if !opts.no_symlinks { if !opts.no_symlinks {
for file in &self.bin_files { for file in &self.bin_files {
file.install_link()?; install_link(file)?;
} }
} }
@ -158,6 +169,10 @@ impl ResolutionSource {
cmd.arg("--root").arg(cargo_root); cmd.arg("--root").arg(cargo_root);
} }
if opts.no_track {
cmd.arg("--no-track");
}
if !opts.dry_run { if !opts.dry_run {
let mut child = opts let mut child = opts
.jobserver_client .jobserver_client

28
e2e-tests/no-track.sh Normal file
View file

@ -0,0 +1,28 @@
#!/bin/bash
set -euxo pipefail
unset CARGO_INSTALL_ROOT
CARGO_HOME=$(mktemp -d 2>/dev/null || mktemp -d -t 'cargo-home')
export CARGO_HOME
export PATH="$CARGO_HOME/bin:$PATH"
"./$1" binstall -y cargo-binstall@0.20.1
cargo-binstall --help >/dev/null
set +e
"./$1" binstall -y --no-track cargo-binstall@0.20.1
exit_code="$?"
set -e
if [ "$exit_code" != 74 ]; then
echo "Expected exit code 74 Io Error, but actual exit code $exit_code"
exit 1
fi
"./$1" binstall -y --no-track --force cargo-binstall@0.20.1
cargo-binstall --help >/dev/null

View file

@ -202,6 +202,7 @@ e2e-test-version-syntax: (e2e-test "version-syntax")
e2e-test-upgrade: (e2e-test "upgrade") e2e-test-upgrade: (e2e-test "upgrade")
e2e-test-self-upgrade-no-symlink: (e2e-test "self-upgrade-no-symlink") e2e-test-self-upgrade-no-symlink: (e2e-test "self-upgrade-no-symlink")
e2e-test-uninstall: (e2e-test "uninstall") e2e-test-uninstall: (e2e-test "uninstall")
e2e-test-no-track: (e2e-test "no-track")
# WinTLS (Windows in CI) does not have TLS 1.3 support # WinTLS (Windows in CI) does not have TLS 1.3 support
[windows] [windows]
@ -210,7 +211,7 @@ e2e-test-tls: (e2e-test "tls" "1.2")
[macos] [macos]
e2e-test-tls: (e2e-test "tls" "1.2") (e2e-test "tls" "1.3") e2e-test-tls: (e2e-test "tls" "1.2") (e2e-test "tls" "1.3")
e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-tests: e2e-test-live e2e-test-manifest-path e2e-test-other-repos e2e-test-strategies e2e-test-version-syntax e2e-test-upgrade e2e-test-tls e2e-test-self-upgrade-no-symlink e2e-test-uninstall e2e-test-subcrate e2e-test-no-track
unit-tests: print-env unit-tests: print-env
{{cargo-bin}} test {{cargo-build-args}} {{cargo-bin}} test {{cargo-build-args}}