From 3beb3227caf0a46c48b417df8461e9144e3b07e9 Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Sun, 15 Aug 2021 00:02:01 -0700 Subject: [PATCH 1/3] Refactor install_one into two parts --- src/cargo/ops/cargo_install.rs | 67 ++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 19 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index e612fc4b9..281461fc4 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -49,7 +49,7 @@ pub fn install( let map = SourceConfigMap::new(config)?; let (installed_anything, scheduled_error) = if krates.len() <= 1 { - install_one( + let pkg = determine_package( config, &root, &map, @@ -59,9 +59,13 @@ pub fn install( vers, opts, force, - no_track, true, )?; + if let Some(pkg) = pkg { + install_one( + config, &root, source_id, from_cwd, vers, opts, force, no_track, pkg, + )?; + } (true, false) } else { let mut succeeded = vec![]; @@ -72,7 +76,7 @@ pub fn install( for krate in krates { let root = root.clone(); let map = map.clone(); - match install_one( + let pkg = match determine_package( config, &root, &map, @@ -82,18 +86,34 @@ pub fn install( vers, opts, force, - no_track, !did_update, ) { - Ok(still_needs_update) => { + Ok(Some(pkg)) => { + did_update = true; + pkg + } + Ok(None) => { + // Already installed succeeded.push(krate); - did_update |= !still_needs_update; + continue; } Err(e) => { crate::display_error(&e, &mut config.shell()); failed.push(krate); // We assume an update was performed if we got an error. did_update = true; + continue; + } + }; + match install_one( + config, &root, source_id, from_cwd, vers, opts, force, no_track, pkg, + ) { + Ok(()) => { + succeeded.push(krate); + } + Err(e) => { + crate::display_error(&e, &mut config.shell()); + failed.push(krate); } } } @@ -138,12 +158,8 @@ pub fn install( Ok(()) } -// Returns whether a subsequent call should attempt to update again. -// The `needs_update_if_source_is_index` parameter indicates whether or not the source index should -// be updated. This is used ensure it is only updated once when installing multiple crates. -// The return value here is used so that the caller knows what to pass to the -// `needs_update_if_source_is_index` parameter when `install_one` is called again. -fn install_one( +// Returns pkg to install. None if pkg is already installed +fn determine_package( config: &Config, root: &Filesystem, map: &SourceConfigMap<'_>, @@ -153,9 +169,8 @@ fn install_one( vers: Option<&str>, opts: &ops::CompileOptions, force: bool, - no_track: bool, needs_update_if_source_is_index: bool, -) -> CargoResult { +) -> CargoResult> { if let Some(name) = krate { if name == "." { bail!( @@ -167,7 +182,6 @@ fn install_one( } let dst = root.join("bin").into_path_unlocked(); - let pkg = { let dep = { if let Some(krate) = krate { @@ -241,7 +255,7 @@ fn install_one( pkg ); config.shell().status("Ignored", &msg)?; - return Ok(true); + return Ok(None); } select_dep_pkg(&mut source, dep, config, needs_update_if_source_is_index)? } else { @@ -252,6 +266,21 @@ fn install_one( ) } }; + Ok(Some(pkg)) +} + +fn install_one( + config: &Config, + root: &Filesystem, + source_id: SourceId, + from_cwd: bool, + vers: Option<&str>, + opts: &ops::CompileOptions, + force: bool, + no_track: bool, + pkg: Package, +) -> CargoResult<()> { + let dst = root.join("bin").into_path_unlocked(); let (mut ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; // If we're installing in --locked mode and there's no `Cargo.lock` published @@ -345,7 +374,7 @@ fn install_one( pkg ); config.shell().status("Ignored", &msg)?; - return Ok(false); + return Ok(()); } config.shell().status("Installing", &pkg)?; @@ -500,7 +529,7 @@ fn install_one( "Installed", format!("package `{}` {}", pkg, executables(successful_bins.iter())), )?; - Ok(false) + Ok(()) } else { if !to_install.is_empty() { config.shell().status( @@ -525,7 +554,7 @@ fn install_one( ), )?; } - Ok(false) + Ok(()) } } From f7e89ea021da7ce800aca57b576c85255a5a8d14 Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Mon, 16 Aug 2021 13:46:11 -0700 Subject: [PATCH 2/3] Refactor additional validation steps to determine_package --- src/cargo/ops/cargo_install.rs | 123 +++++++++++++++++---------------- 1 file changed, 64 insertions(+), 59 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 281461fc4..38ff5073e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -59,12 +59,11 @@ pub fn install( vers, opts, force, + no_track, true, )?; if let Some(pkg) = pkg { - install_one( - config, &root, source_id, from_cwd, vers, opts, force, no_track, pkg, - )?; + install_one(config, &root, source_id, vers, opts, force, no_track, pkg)?; } (true, false) } else { @@ -86,6 +85,7 @@ pub fn install( vers, opts, force, + no_track, !did_update, ) { Ok(Some(pkg)) => { @@ -105,9 +105,7 @@ pub fn install( continue; } }; - match install_one( - config, &root, source_id, from_cwd, vers, opts, force, no_track, pkg, - ) { + match install_one(config, &root, source_id, vers, opts, force, no_track, pkg) { Ok(()) => { succeeded.push(krate); } @@ -169,6 +167,7 @@ fn determine_package( vers: Option<&str>, opts: &ops::CompileOptions, force: bool, + no_track: bool, needs_update_if_source_is_index: bool, ) -> CargoResult> { if let Some(name) = krate { @@ -266,23 +265,8 @@ fn determine_package( ) } }; - Ok(Some(pkg)) -} -fn install_one( - config: &Config, - root: &Filesystem, - source_id: SourceId, - from_cwd: bool, - vers: Option<&str>, - opts: &ops::CompileOptions, - force: bool, - no_track: bool, - pkg: Package, -) -> CargoResult<()> { - let dst = root.join("bin").into_path_unlocked(); - - let (mut ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; + let (ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; // If we're installing in --locked mode and there's no `Cargo.lock` published // ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026 if config.locked() && !ws.root().join("Cargo.lock").exists() { @@ -299,22 +283,6 @@ fn install_one( ws.current()?.clone() }; - let mut td_opt = None; - let mut needs_cleanup = false; - if !source_id.is_path() { - let target_dir = if let Some(dir) = config.target_dir()? { - dir - } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { - let p = td.path().to_owned(); - td_opt = Some(td); - Filesystem::new(p) - } else { - needs_cleanup = true; - Filesystem::new(config.cwd().join("target-install")) - }; - ws.set_target_dir(target_dir); - } - if from_cwd { if pkg.manifest().edition() == Edition::Edition2015 { config.shell().warn( @@ -345,40 +313,77 @@ fn install_one( ); } - // Helper for --no-track flag to make sure it doesn't overwrite anything. - let no_track_duplicates = || -> CargoResult>> { - let duplicates: BTreeMap> = exe_names(&pkg, &opts.filter) - .into_iter() - .filter(|name| dst.join(name).exists()) - .map(|name| (name, None)) - .collect(); - if !force && !duplicates.is_empty() { - let mut msg: Vec = duplicates - .iter() - .map(|(name, _)| format!("binary `{}` already exists in destination", name)) - .collect(); - msg.push("Add --force to overwrite".to_string()); - bail!("{}", msg.join("\n")); - } - Ok(duplicates) - }; - // WARNING: no_track does not perform locking, so there is no protection // of concurrent installs. if no_track { // Check for conflicts. - no_track_duplicates()?; + no_track_duplicates(&pkg, opts, &dst, force)?; } else if is_installed(&pkg, config, opts, &rustc, &target, root, &dst, force)? { let msg = format!( "package `{}` is already installed, use --force to override", pkg ); config.shell().status("Ignored", &msg)?; - return Ok(()); + return Ok(None); } + Ok(Some(pkg)) +} + +fn no_track_duplicates( + pkg: &Package, + opts: &ops::CompileOptions, + dst: &Path, + force: bool, +) -> CargoResult>> { + // Helper for --no-track flag to make sure it doesn't overwrite anything. + let duplicates: BTreeMap> = exe_names(&pkg, &opts.filter) + .into_iter() + .filter(|name| dst.join(name).exists()) + .map(|name| (name, None)) + .collect(); + if !force && !duplicates.is_empty() { + let mut msg: Vec = duplicates + .iter() + .map(|(name, _)| format!("binary `{}` already exists in destination", name)) + .collect(); + msg.push("Add --force to overwrite".to_string()); + bail!("{}", msg.join("\n")); + } + Ok(duplicates) +} + +fn install_one<'cfg>( + config: &Config, + root: &Filesystem, + source_id: SourceId, + vers: Option<&str>, + opts: &ops::CompileOptions, + force: bool, + no_track: bool, + pkg: Package, +) -> CargoResult<()> { config.shell().status("Installing", &pkg)?; + let dst = root.join("bin").into_path_unlocked(); + let (mut ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; + + let mut td_opt = None; + let mut needs_cleanup = false; + if !source_id.is_path() { + let target_dir = if let Some(dir) = config.target_dir()? { + dir + } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { + let p = td.path().to_owned(); + td_opt = Some(td); + Filesystem::new(p) + } else { + needs_cleanup = true; + Filesystem::new(config.cwd().join("target-install")) + }; + ws.set_target_dir(target_dir); + } + check_yanked_install(&ws)?; let exec: Arc = Arc::new(DefaultExecutor); @@ -414,7 +419,7 @@ fn install_one( binaries.sort_unstable(); let (tracker, duplicates) = if no_track { - (None, no_track_duplicates()?) + (None, no_track_duplicates(&pkg, opts, &dst, force)?) } else { let tracker = InstallTracker::load(config, root)?; let (_freshness, duplicates) = From f9c2d04592b1c4f48e9b4c30cba23f2136101cfc Mon Sep 17 00:00:00 2001 From: Nipunn Koorapati Date: Wed, 18 Aug 2021 11:34:33 -0700 Subject: [PATCH 3/3] Factor into struct InstallablePackage --- src/cargo/ops/cargo_install.rs | 1003 +++++++++++++++------------- tests/testsuite/install.rs | 12 +- tests/testsuite/install_upgrade.rs | 12 +- 3 files changed, 539 insertions(+), 488 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 38ff5073e..ad62de96d 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -34,6 +34,482 @@ impl Drop for Transaction { } } +struct InstallablePackage<'cfg, 'a> { + config: &'cfg Config, + opts: &'a ops::CompileOptions, + root: Filesystem, + source_id: SourceId, + vers: Option<&'a str>, + force: bool, + no_track: bool, + + pkg: Package, + ws: Workspace<'cfg>, + rustc: Rustc, + target: String, +} + +impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { + // Returns pkg to install. None if pkg is already installed + pub fn new( + config: &'cfg Config, + root: Filesystem, + map: SourceConfigMap<'_>, + krate: Option<&str>, + source_id: SourceId, + from_cwd: bool, + vers: Option<&'a str>, + opts: &'a ops::CompileOptions, + force: bool, + no_track: bool, + needs_update_if_source_is_index: bool, + ) -> CargoResult>> { + if let Some(name) = krate { + if name == "." { + bail!( + "To install the binaries for the package in current working \ + directory use `cargo install --path .`. \ + Use `cargo build` if you want to simply build the package." + ) + } + } + + let dst = root.join("bin").into_path_unlocked(); + let pkg = { + let dep = { + if let Some(krate) = krate { + let vers = if let Some(vers_flag) = vers { + Some(parse_semver_flag(vers_flag)?.to_string()) + } else if source_id.is_registry() { + // Avoid pre-release versions from crate.io + // unless explicitly asked for + Some(String::from("*")) + } else { + None + }; + Some(Dependency::parse(krate, vers.as_deref(), source_id)?) + } else { + None + } + }; + + if source_id.is_git() { + let mut source = GitSource::new(source_id, config)?; + select_pkg( + &mut source, + dep, + |git: &mut GitSource<'_>| git.read_packages(), + config, + )? + } else if source_id.is_path() { + let mut src = path_source(source_id, config)?; + if !src.path().is_dir() { + bail!( + "`{}` is not a directory. \ + --path must point to a directory containing a Cargo.toml file.", + src.path().display() + ) + } + if !src.path().join("Cargo.toml").exists() { + if from_cwd { + bail!( + "`{}` is not a crate root; specify a crate to \ + install from crates.io, or use --path or --git to \ + specify an alternate source", + src.path().display() + ); + } else if src.path().join("cargo.toml").exists() { + bail!( + "`{}` does not contain a Cargo.toml file, but found cargo.toml please try to rename it to Cargo.toml. \ + --path must point to a directory containing a Cargo.toml file.", + src.path().display() + ) + } else { + bail!( + "`{}` does not contain a Cargo.toml file. \ + --path must point to a directory containing a Cargo.toml file.", + src.path().display() + ) + } + } + select_pkg( + &mut src, + dep, + |path: &mut PathSource<'_>| path.read_packages(), + config, + )? + } else if let Some(dep) = dep { + let mut source = map.load(source_id, &HashSet::new())?; + if let Ok(Some(pkg)) = installed_exact_package( + dep.clone(), + &mut source, + config, + opts, + &root, + &dst, + force, + ) { + let msg = format!( + "package `{}` is already installed, use --force to override", + pkg + ); + config.shell().status("Ignored", &msg)?; + return Ok(None); + } + select_dep_pkg(&mut source, dep, config, needs_update_if_source_is_index)? + } else { + bail!( + "must specify a crate to install from \ + crates.io, or use --path or --git to \ + specify alternate source" + ) + } + }; + + let (ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; + // If we're installing in --locked mode and there's no `Cargo.lock` published + // ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026 + if config.locked() && !ws.root().join("Cargo.lock").exists() { + config.shell().warn(format!( + "no Cargo.lock file published in {}", + pkg.to_string() + ))?; + } + let pkg = if source_id.is_git() { + // Don't use ws.current() in order to keep the package source as a git source so that + // install tracking uses the correct source. + pkg + } else { + ws.current()?.clone() + }; + + if from_cwd { + if pkg.manifest().edition() == Edition::Edition2015 { + config.shell().warn( + "Using `cargo install` to install the binaries for the \ + package in current working directory is deprecated, \ + use `cargo install --path .` instead. \ + Use `cargo build` if you want to simply build the package.", + )? + } else { + bail!( + "Using `cargo install` to install the binaries for the \ + package in current working directory is no longer supported, \ + use `cargo install --path .` instead. \ + Use `cargo build` if you want to simply build the package." + ) + } + }; + + // For bare `cargo install` (no `--bin` or `--example`), check if there is + // *something* to install. Explicit `--bin` or `--example` flags will be + // checked at the start of `compile_ws`. + if !opts.filter.is_specific() && !pkg.targets().iter().any(|t| t.is_bin()) { + bail!( + "there is nothing to install in `{}`, because it has no binaries\n\ + `cargo install` is only for installing programs, and can't be used with libraries.\n\ + To use a library crate, add it as a dependency in a Cargo project instead.", + pkg + ); + } + + let ip = InstallablePackage { + config, + opts, + root, + source_id, + vers, + force, + no_track, + + pkg, + ws, + rustc, + target, + }; + + // WARNING: no_track does not perform locking, so there is no protection + // of concurrent installs. + if no_track { + // Check for conflicts. + ip.no_track_duplicates(&dst)?; + } else if is_installed( + &ip.pkg, config, opts, &ip.rustc, &ip.target, &ip.root, &dst, force, + )? { + let msg = format!( + "package `{}` is already installed, use --force to override", + ip.pkg + ); + config.shell().status("Ignored", &msg)?; + return Ok(None); + } + + Ok(Some(ip)) + } + + fn no_track_duplicates(&self, dst: &Path) -> CargoResult>> { + // Helper for --no-track flag to make sure it doesn't overwrite anything. + let duplicates: BTreeMap> = + exe_names(&self.pkg, &self.opts.filter) + .into_iter() + .filter(|name| dst.join(name).exists()) + .map(|name| (name, None)) + .collect(); + if !self.force && !duplicates.is_empty() { + let mut msg: Vec = duplicates + .iter() + .map(|(name, _)| format!("binary `{}` already exists in destination", name)) + .collect(); + msg.push("Add --force to overwrite".to_string()); + bail!("{}", msg.join("\n")); + } + Ok(duplicates) + } + + fn install_one(mut self) -> CargoResult<()> { + self.config.shell().status("Installing", &self.pkg)?; + + let dst = self.root.join("bin").into_path_unlocked(); + + let mut td_opt = None; + let mut needs_cleanup = false; + if !self.source_id.is_path() { + let target_dir = if let Some(dir) = self.config.target_dir()? { + dir + } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { + let p = td.path().to_owned(); + td_opt = Some(td); + Filesystem::new(p) + } else { + needs_cleanup = true; + Filesystem::new(self.config.cwd().join("target-install")) + }; + self.ws.set_target_dir(target_dir); + } + + self.check_yanked_install()?; + + let exec: Arc = Arc::new(DefaultExecutor); + let compile = ops::compile_ws(&self.ws, self.opts, &exec).with_context(|| { + if let Some(td) = td_opt.take() { + // preserve the temporary directory, so the user can inspect it + td.into_path(); + } + + format!( + "failed to compile `{}`, intermediate artifacts can be \ + found at `{}`", + self.pkg, + self.ws.target_dir().display() + ) + })?; + let mut binaries: Vec<(&str, &Path)> = compile + .binaries + .iter() + .map(|UnitOutput { path, .. }| { + let name = path.file_name().unwrap(); + if let Some(s) = name.to_str() { + Ok((s, path.as_ref())) + } else { + bail!("Binary `{:?}` name can't be serialized into string", name) + } + }) + .collect::>()?; + if binaries.is_empty() { + bail!("no binaries are available for install using the selected features"); + } + // This is primarily to make testing easier. + binaries.sort_unstable(); + + let (tracker, duplicates) = if self.no_track { + (None, self.no_track_duplicates(&dst)?) + } else { + let tracker = InstallTracker::load(self.config, &self.root)?; + let (_freshness, duplicates) = tracker.check_upgrade( + &dst, + &self.pkg, + self.force, + self.opts, + &self.target, + &self.rustc.verbose_version, + )?; + (Some(tracker), duplicates) + }; + + paths::create_dir_all(&dst)?; + + // Copy all binaries to a temporary directory under `dst` first, catching + // some failure modes (e.g., out of space) before touching the existing + // binaries. This directory will get cleaned up via RAII. + let staging_dir = TempFileBuilder::new() + .prefix("cargo-install") + .tempdir_in(&dst)?; + for &(bin, src) in binaries.iter() { + let dst = staging_dir.path().join(bin); + // Try to move if `target_dir` is transient. + if !self.source_id.is_path() && fs::rename(src, &dst).is_ok() { + continue; + } + paths::copy(src, &dst)?; + } + + let (to_replace, to_install): (Vec<&str>, Vec<&str>) = binaries + .iter() + .map(|&(bin, _)| bin) + .partition(|&bin| duplicates.contains_key(bin)); + + let mut installed = Transaction { bins: Vec::new() }; + let mut successful_bins = BTreeSet::new(); + + // Move the temporary copies into `dst` starting with new binaries. + for bin in to_install.iter() { + let src = staging_dir.path().join(bin); + let dst = dst.join(bin); + self.config.shell().status("Installing", dst.display())?; + fs::rename(&src, &dst).with_context(|| { + format!("failed to move `{}` to `{}`", src.display(), dst.display()) + })?; + installed.bins.push(dst); + successful_bins.insert(bin.to_string()); + } + + // Repeat for binaries which replace existing ones but don't pop the error + // up until after updating metadata. + let replace_result = { + let mut try_install = || -> CargoResult<()> { + for &bin in to_replace.iter() { + let src = staging_dir.path().join(bin); + let dst = dst.join(bin); + self.config.shell().status("Replacing", dst.display())?; + fs::rename(&src, &dst).with_context(|| { + format!("failed to move `{}` to `{}`", src.display(), dst.display()) + })?; + successful_bins.insert(bin.to_string()); + } + Ok(()) + }; + try_install() + }; + + if let Some(mut tracker) = tracker { + tracker.mark_installed( + &self.pkg, + &successful_bins, + self.vers.map(|s| s.to_string()), + self.opts, + &self.target, + &self.rustc.verbose_version, + ); + + if let Err(e) = + remove_orphaned_bins(&self.ws, &mut tracker, &duplicates, &self.pkg, &dst) + { + // Don't hard error on remove. + self.config + .shell() + .warn(format!("failed to remove orphan: {:?}", e))?; + } + + match tracker.save() { + Err(err) => replace_result.with_context(|| err)?, + Ok(_) => replace_result?, + } + } + + // Reaching here means all actions have succeeded. Clean up. + installed.success(); + if needs_cleanup { + // Don't bother grabbing a lock as we're going to blow it all away + // anyway. + let target_dir = self.ws.target_dir().into_path_unlocked(); + paths::remove_dir_all(&target_dir)?; + } + + // Helper for creating status messages. + fn executables>(mut names: impl Iterator + Clone) -> String { + if names.clone().count() == 1 { + format!("(executable `{}`)", names.next().unwrap().as_ref()) + } else { + format!( + "(executables {})", + names + .map(|b| format!("`{}`", b.as_ref())) + .collect::>() + .join(", ") + ) + } + } + + if duplicates.is_empty() { + self.config.shell().status( + "Installed", + format!( + "package `{}` {}", + self.pkg, + executables(successful_bins.iter()) + ), + )?; + Ok(()) + } else { + if !to_install.is_empty() { + self.config.shell().status( + "Installed", + format!("package `{}` {}", self.pkg, executables(to_install.iter())), + )?; + } + // Invert the duplicate map. + let mut pkg_map = BTreeMap::new(); + for (bin_name, opt_pkg_id) in &duplicates { + let key = + opt_pkg_id.map_or_else(|| "unknown".to_string(), |pkg_id| pkg_id.to_string()); + pkg_map.entry(key).or_insert_with(Vec::new).push(bin_name); + } + for (pkg_descr, bin_names) in &pkg_map { + self.config.shell().status( + "Replaced", + format!( + "package `{}` with `{}` {}", + pkg_descr, + self.pkg, + executables(bin_names.iter()) + ), + )?; + } + Ok(()) + } + } + + fn check_yanked_install(&self) -> CargoResult<()> { + if self.ws.ignore_lock() || !self.ws.root().join("Cargo.lock").exists() { + return Ok(()); + } + // It would be best if `source` could be passed in here to avoid a + // duplicate "Updating", but since `source` is taken by value, then it + // wouldn't be available for `compile_ws`. + let (pkg_set, resolve) = ops::resolve_ws(&self.ws)?; + let mut sources = pkg_set.sources_mut(); + + // Checking the yanked status involves taking a look at the registry and + // maybe updating files, so be sure to lock it here. + let _lock = self.ws.config().acquire_package_cache_lock()?; + + for pkg_id in resolve.iter() { + if let Some(source) = sources.get_mut(pkg_id.source_id()) { + if source.is_yanked(pkg_id)? { + self.ws.config().shell().warn(format!( + "package `{}` in Cargo.lock is yanked in registry `{}`, \ + consider running without --locked", + pkg_id, + pkg_id.source_id().display_registry_name() + ))?; + } + } + } + + Ok(()) + } +} + pub fn install( config: &Config, root: Option<&str>, @@ -46,13 +522,14 @@ pub fn install( no_track: bool, ) -> CargoResult<()> { let root = resolve_root(root, config)?; + let dst = root.join("bin").into_path_unlocked(); let map = SourceConfigMap::new(config)?; let (installed_anything, scheduled_error) = if krates.len() <= 1 { - let pkg = determine_package( + let installable_pkg = InstallablePackage::new( config, - &root, - &map, + root, + map, krates.into_iter().next(), source_id, from_cwd, @@ -62,8 +539,8 @@ pub fn install( no_track, true, )?; - if let Some(pkg) = pkg { - install_one(config, &root, source_id, vers, opts, force, no_track, pkg)?; + if let Some(installable_pkg) = installable_pkg { + installable_pkg.install_one()?; } (true, false) } else { @@ -72,40 +549,52 @@ pub fn install( // "Tracks whether or not the source (such as a registry or git repo) has been updated. // This is used to avoid updating it multiple times when installing multiple crates. let mut did_update = false; - for krate in krates { - let root = root.clone(); - let map = map.clone(); - let pkg = match determine_package( - config, - &root, - &map, - Some(krate), - source_id, - from_cwd, - vers, - opts, - force, - no_track, - !did_update, - ) { - Ok(Some(pkg)) => { - did_update = true; - pkg + + let pkgs_to_install: Vec<_> = krates + .into_iter() + .filter_map(|krate| { + let root = root.clone(); + let map = map.clone(); + match InstallablePackage::new( + config, + root, + map, + Some(krate), + source_id, + from_cwd, + vers, + opts, + force, + no_track, + !did_update, + ) { + Ok(Some(installable_pkg)) => { + did_update = true; + Some((krate, installable_pkg)) + } + Ok(None) => { + // Already installed + succeeded.push(krate); + None + } + Err(e) => { + crate::display_error(&e, &mut config.shell()); + failed.push(krate); + // We assume an update was performed if we got an error. + did_update = true; + None + } } - Ok(None) => { - // Already installed - succeeded.push(krate); - continue; - } - Err(e) => { - crate::display_error(&e, &mut config.shell()); - failed.push(krate); - // We assume an update was performed if we got an error. - did_update = true; - continue; - } - }; - match install_one(config, &root, source_id, vers, opts, force, no_track, pkg) { + }) + .collect(); + + let install_results: Vec<_> = pkgs_to_install + .into_iter() + .map(|(krate, installable_pkg)| (krate, installable_pkg.install_one())) + .collect(); + + for (krate, result) in install_results { + match result { Ok(()) => { succeeded.push(krate); } @@ -136,7 +625,6 @@ pub fn install( if installed_anything { // Print a warning that if this directory isn't in PATH that they won't be // able to run these commands. - let dst = root.join("bin").into_path_unlocked(); let path = env::var_os("PATH").unwrap_or_default(); let dst_in_path = env::split_paths(&path).any(|path| path == dst); @@ -156,413 +644,6 @@ pub fn install( Ok(()) } -// Returns pkg to install. None if pkg is already installed -fn determine_package( - config: &Config, - root: &Filesystem, - map: &SourceConfigMap<'_>, - krate: Option<&str>, - source_id: SourceId, - from_cwd: bool, - vers: Option<&str>, - opts: &ops::CompileOptions, - force: bool, - no_track: bool, - needs_update_if_source_is_index: bool, -) -> CargoResult> { - if let Some(name) = krate { - if name == "." { - bail!( - "To install the binaries for the package in current working \ - directory use `cargo install --path .`. \ - Use `cargo build` if you want to simply build the package." - ) - } - } - - let dst = root.join("bin").into_path_unlocked(); - let pkg = { - let dep = { - if let Some(krate) = krate { - let vers = if let Some(vers_flag) = vers { - Some(parse_semver_flag(vers_flag)?.to_string()) - } else if source_id.is_registry() { - // Avoid pre-release versions from crate.io - // unless explicitly asked for - Some(String::from("*")) - } else { - None - }; - Some(Dependency::parse(krate, vers.as_deref(), source_id)?) - } else { - None - } - }; - - if source_id.is_git() { - let mut source = GitSource::new(source_id, config)?; - select_pkg( - &mut source, - dep, - |git: &mut GitSource<'_>| git.read_packages(), - config, - )? - } else if source_id.is_path() { - let mut src = path_source(source_id, config)?; - if !src.path().is_dir() { - bail!( - "`{}` is not a directory. \ - --path must point to a directory containing a Cargo.toml file.", - src.path().display() - ) - } - if !src.path().join("Cargo.toml").exists() { - if from_cwd { - bail!( - "`{}` is not a crate root; specify a crate to \ - install from crates.io, or use --path or --git to \ - specify an alternate source", - src.path().display() - ); - } else if src.path().join("cargo.toml").exists() { - bail!( - "`{}` does not contain a Cargo.toml file, but found cargo.toml please try to rename it to Cargo.toml. \ - --path must point to a directory containing a Cargo.toml file.", - src.path().display() - ) - } else { - bail!( - "`{}` does not contain a Cargo.toml file. \ - --path must point to a directory containing a Cargo.toml file.", - src.path().display() - ) - } - } - select_pkg( - &mut src, - dep, - |path: &mut PathSource<'_>| path.read_packages(), - config, - )? - } else if let Some(dep) = dep { - let mut source = map.load(source_id, &HashSet::new())?; - if let Ok(Some(pkg)) = - installed_exact_package(dep.clone(), &mut source, config, opts, root, &dst, force) - { - let msg = format!( - "package `{}` is already installed, use --force to override", - pkg - ); - config.shell().status("Ignored", &msg)?; - return Ok(None); - } - select_dep_pkg(&mut source, dep, config, needs_update_if_source_is_index)? - } else { - bail!( - "must specify a crate to install from \ - crates.io, or use --path or --git to \ - specify alternate source" - ) - } - }; - - let (ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; - // If we're installing in --locked mode and there's no `Cargo.lock` published - // ie. the bin was published before https://github.com/rust-lang/cargo/pull/7026 - if config.locked() && !ws.root().join("Cargo.lock").exists() { - config.shell().warn(format!( - "no Cargo.lock file published in {}", - pkg.to_string() - ))?; - } - let pkg = if source_id.is_git() { - // Don't use ws.current() in order to keep the package source as a git source so that - // install tracking uses the correct source. - pkg - } else { - ws.current()?.clone() - }; - - if from_cwd { - if pkg.manifest().edition() == Edition::Edition2015 { - config.shell().warn( - "Using `cargo install` to install the binaries for the \ - package in current working directory is deprecated, \ - use `cargo install --path .` instead. \ - Use `cargo build` if you want to simply build the package.", - )? - } else { - bail!( - "Using `cargo install` to install the binaries for the \ - package in current working directory is no longer supported, \ - use `cargo install --path .` instead. \ - Use `cargo build` if you want to simply build the package." - ) - } - }; - - // For bare `cargo install` (no `--bin` or `--example`), check if there is - // *something* to install. Explicit `--bin` or `--example` flags will be - // checked at the start of `compile_ws`. - if !opts.filter.is_specific() && !pkg.targets().iter().any(|t| t.is_bin()) { - bail!( - "there is nothing to install in `{}`, because it has no binaries\n\ - `cargo install` is only for installing programs, and can't be used with libraries.\n\ - To use a library crate, add it as a dependency in a Cargo project instead.", - pkg - ); - } - - // WARNING: no_track does not perform locking, so there is no protection - // of concurrent installs. - if no_track { - // Check for conflicts. - no_track_duplicates(&pkg, opts, &dst, force)?; - } else if is_installed(&pkg, config, opts, &rustc, &target, root, &dst, force)? { - let msg = format!( - "package `{}` is already installed, use --force to override", - pkg - ); - config.shell().status("Ignored", &msg)?; - return Ok(None); - } - - Ok(Some(pkg)) -} - -fn no_track_duplicates( - pkg: &Package, - opts: &ops::CompileOptions, - dst: &Path, - force: bool, -) -> CargoResult>> { - // Helper for --no-track flag to make sure it doesn't overwrite anything. - let duplicates: BTreeMap> = exe_names(&pkg, &opts.filter) - .into_iter() - .filter(|name| dst.join(name).exists()) - .map(|name| (name, None)) - .collect(); - if !force && !duplicates.is_empty() { - let mut msg: Vec = duplicates - .iter() - .map(|(name, _)| format!("binary `{}` already exists in destination", name)) - .collect(); - msg.push("Add --force to overwrite".to_string()); - bail!("{}", msg.join("\n")); - } - Ok(duplicates) -} - -fn install_one<'cfg>( - config: &Config, - root: &Filesystem, - source_id: SourceId, - vers: Option<&str>, - opts: &ops::CompileOptions, - force: bool, - no_track: bool, - pkg: Package, -) -> CargoResult<()> { - config.shell().status("Installing", &pkg)?; - - let dst = root.join("bin").into_path_unlocked(); - let (mut ws, rustc, target) = make_ws_rustc_target(config, opts, &source_id, pkg.clone())?; - - let mut td_opt = None; - let mut needs_cleanup = false; - if !source_id.is_path() { - let target_dir = if let Some(dir) = config.target_dir()? { - dir - } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() { - let p = td.path().to_owned(); - td_opt = Some(td); - Filesystem::new(p) - } else { - needs_cleanup = true; - Filesystem::new(config.cwd().join("target-install")) - }; - ws.set_target_dir(target_dir); - } - - check_yanked_install(&ws)?; - - let exec: Arc = Arc::new(DefaultExecutor); - let compile = ops::compile_ws(&ws, opts, &exec).with_context(|| { - if let Some(td) = td_opt.take() { - // preserve the temporary directory, so the user can inspect it - td.into_path(); - } - - format!( - "failed to compile `{}`, intermediate artifacts can be \ - found at `{}`", - pkg, - ws.target_dir().display() - ) - })?; - let mut binaries: Vec<(&str, &Path)> = compile - .binaries - .iter() - .map(|UnitOutput { path, .. }| { - let name = path.file_name().unwrap(); - if let Some(s) = name.to_str() { - Ok((s, path.as_ref())) - } else { - bail!("Binary `{:?}` name can't be serialized into string", name) - } - }) - .collect::>()?; - if binaries.is_empty() { - bail!("no binaries are available for install using the selected features"); - } - // This is primarily to make testing easier. - binaries.sort_unstable(); - - let (tracker, duplicates) = if no_track { - (None, no_track_duplicates(&pkg, opts, &dst, force)?) - } else { - let tracker = InstallTracker::load(config, root)?; - let (_freshness, duplicates) = - tracker.check_upgrade(&dst, &pkg, force, opts, &target, &rustc.verbose_version)?; - (Some(tracker), duplicates) - }; - - paths::create_dir_all(&dst)?; - - // Copy all binaries to a temporary directory under `dst` first, catching - // some failure modes (e.g., out of space) before touching the existing - // binaries. This directory will get cleaned up via RAII. - let staging_dir = TempFileBuilder::new() - .prefix("cargo-install") - .tempdir_in(&dst)?; - for &(bin, src) in binaries.iter() { - let dst = staging_dir.path().join(bin); - // Try to move if `target_dir` is transient. - if !source_id.is_path() && fs::rename(src, &dst).is_ok() { - continue; - } - paths::copy(src, &dst)?; - } - - let (to_replace, to_install): (Vec<&str>, Vec<&str>) = binaries - .iter() - .map(|&(bin, _)| bin) - .partition(|&bin| duplicates.contains_key(bin)); - - let mut installed = Transaction { bins: Vec::new() }; - let mut successful_bins = BTreeSet::new(); - - // Move the temporary copies into `dst` starting with new binaries. - for bin in to_install.iter() { - let src = staging_dir.path().join(bin); - let dst = dst.join(bin); - config.shell().status("Installing", dst.display())?; - fs::rename(&src, &dst).with_context(|| { - format!("failed to move `{}` to `{}`", src.display(), dst.display()) - })?; - installed.bins.push(dst); - successful_bins.insert(bin.to_string()); - } - - // Repeat for binaries which replace existing ones but don't pop the error - // up until after updating metadata. - let replace_result = { - let mut try_install = || -> CargoResult<()> { - for &bin in to_replace.iter() { - let src = staging_dir.path().join(bin); - let dst = dst.join(bin); - config.shell().status("Replacing", dst.display())?; - fs::rename(&src, &dst).with_context(|| { - format!("failed to move `{}` to `{}`", src.display(), dst.display()) - })?; - successful_bins.insert(bin.to_string()); - } - Ok(()) - }; - try_install() - }; - - if let Some(mut tracker) = tracker { - tracker.mark_installed( - &pkg, - &successful_bins, - vers.map(|s| s.to_string()), - opts, - &target, - &rustc.verbose_version, - ); - - if let Err(e) = remove_orphaned_bins(&ws, &mut tracker, &duplicates, &pkg, &dst) { - // Don't hard error on remove. - config - .shell() - .warn(format!("failed to remove orphan: {:?}", e))?; - } - - match tracker.save() { - Err(err) => replace_result.with_context(|| err)?, - Ok(_) => replace_result?, - } - } - - // Reaching here means all actions have succeeded. Clean up. - installed.success(); - if needs_cleanup { - // Don't bother grabbing a lock as we're going to blow it all away - // anyway. - let target_dir = ws.target_dir().into_path_unlocked(); - paths::remove_dir_all(&target_dir)?; - } - - // Helper for creating status messages. - fn executables>(mut names: impl Iterator + Clone) -> String { - if names.clone().count() == 1 { - format!("(executable `{}`)", names.next().unwrap().as_ref()) - } else { - format!( - "(executables {})", - names - .map(|b| format!("`{}`", b.as_ref())) - .collect::>() - .join(", ") - ) - } - } - - if duplicates.is_empty() { - config.shell().status( - "Installed", - format!("package `{}` {}", pkg, executables(successful_bins.iter())), - )?; - Ok(()) - } else { - if !to_install.is_empty() { - config.shell().status( - "Installed", - format!("package `{}` {}", pkg, executables(to_install.iter())), - )?; - } - // Invert the duplicate map. - let mut pkg_map = BTreeMap::new(); - for (bin_name, opt_pkg_id) in &duplicates { - let key = opt_pkg_id.map_or_else(|| "unknown".to_string(), |pkg_id| pkg_id.to_string()); - pkg_map.entry(key).or_insert_with(Vec::new).push(bin_name); - } - for (pkg_descr, bin_names) in &pkg_map { - config.shell().status( - "Replaced", - format!( - "package `{}` with `{}` {}", - pkg_descr, - pkg, - executables(bin_names.iter()) - ), - )?; - } - Ok(()) - } -} - fn is_installed( pkg: &Package, config: &Config, @@ -684,36 +765,6 @@ fn parse_semver_flag(v: &str) -> CargoResult { } } -fn check_yanked_install(ws: &Workspace<'_>) -> CargoResult<()> { - if ws.ignore_lock() || !ws.root().join("Cargo.lock").exists() { - return Ok(()); - } - // It would be best if `source` could be passed in here to avoid a - // duplicate "Updating", but since `source` is taken by value, then it - // wouldn't be available for `compile_ws`. - let (pkg_set, resolve) = ops::resolve_ws(ws)?; - let mut sources = pkg_set.sources_mut(); - - // Checking the yanked status involves taking a look at the registry and - // maybe updating files, so be sure to lock it here. - let _lock = ws.config().acquire_package_cache_lock()?; - - for pkg_id in resolve.iter() { - if let Some(source) = sources.get_mut(pkg_id.source_id()) { - if source.is_yanked(pkg_id)? { - ws.config().shell().warn(format!( - "package `{}` in Cargo.lock is yanked in registry `{}`, \ - consider running without --locked", - pkg_id, - pkg_id.source_id().display_registry_name() - ))?; - } - } - } - - Ok(()) -} - /// Display a list of installed binaries. pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> { let root = resolve_root(dst, config)?; diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index 112974765..4e8ca4bcb 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -95,19 +95,19 @@ fn multiple_pkgs() { [UPDATING] `[..]` index [DOWNLOADING] crates ... [DOWNLOADED] foo v0.0.1 (registry `dummy-registry`) +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.2 (registry `dummy-registry`) +[ERROR] could not find `baz` in registry `[..]` with version `*` [INSTALLING] foo v0.0.1 [COMPILING] foo v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [CWD]/home/.cargo/bin/foo[EXE] [INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) -[DOWNLOADING] crates ... -[DOWNLOADED] bar v0.0.2 (registry `dummy-registry`) [INSTALLING] bar v0.0.2 [COMPILING] bar v0.0.2 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [CWD]/home/.cargo/bin/bar[EXE] [INSTALLED] package `bar v0.0.2` (executable `bar[EXE]`) -[ERROR] could not find `baz` in registry `[..]` with version `*` [SUMMARY] Successfully installed foo, bar! Failed to install baz (see error(s) above). [WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries [ERROR] some crates failed to install @@ -155,19 +155,19 @@ fn multiple_pkgs_path_set() { [UPDATING] `[..]` index [DOWNLOADING] crates ... [DOWNLOADED] foo v0.0.1 (registry `dummy-registry`) +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.2 (registry `dummy-registry`) +[ERROR] could not find `baz` in registry `[..]` with version `*` [INSTALLING] foo v0.0.1 [COMPILING] foo v0.0.1 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [CWD]/home/.cargo/bin/foo[EXE] [INSTALLED] package `foo v0.0.1` (executable `foo[EXE]`) -[DOWNLOADING] crates ... -[DOWNLOADED] bar v0.0.2 (registry `dummy-registry`) [INSTALLING] bar v0.0.2 [COMPILING] bar v0.0.2 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [CWD]/home/.cargo/bin/bar[EXE] [INSTALLED] package `bar v0.0.2` (executable `bar[EXE]`) -[ERROR] could not find `baz` in registry `[..]` with version `*` [SUMMARY] Successfully installed foo, bar! Failed to install baz (see error(s) above). [ERROR] some crates failed to install ", diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index 23681a2fa..296d21c4f 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -595,20 +595,20 @@ fn multiple_report() { [UPDATING] `[..]` index [DOWNLOADING] crates ... [DOWNLOADED] one v1.0.0 (registry `[..]`) +[DOWNLOADING] crates ... +[DOWNLOADED] two v1.0.0 (registry `[..]`) +[DOWNLOADING] crates ... +[DOWNLOADED] three v1.0.0 (registry `[..]`) [INSTALLING] one v1.0.0 [COMPILING] one v1.0.0 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [..]/.cargo/bin/one[EXE] [INSTALLED] package `one v1.0.0` (executable `one[EXE]`) -[DOWNLOADING] crates ... -[DOWNLOADED] two v1.0.0 (registry `[..]`) [INSTALLING] two v1.0.0 [COMPILING] two v1.0.0 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [..]/.cargo/bin/two[EXE] [INSTALLED] package `two v1.0.0` (executable `two[EXE]`) -[DOWNLOADING] crates ... -[DOWNLOADED] three v1.0.0 (registry `[..]`) [INSTALLING] three v1.0.0 [COMPILING] three v1.0.0 [FINISHED] release [optimized] target(s) in [..] @@ -842,13 +842,13 @@ fn partially_already_installed_does_one_update() { [UPDATING] `[..]` index [DOWNLOADING] crates ... [DOWNLOADED] bar v1.0.0 (registry [..]) +[DOWNLOADING] crates ... +[DOWNLOADED] baz v1.0.0 (registry [..]) [INSTALLING] bar v1.0.0 [COMPILING] bar v1.0.0 [FINISHED] release [optimized] target(s) in [..] [INSTALLING] [CWD]/home/.cargo/bin/bar[EXE] [INSTALLED] package `bar v1.0.0` (executable `bar[EXE]`) -[DOWNLOADING] crates ... -[DOWNLOADED] baz v1.0.0 (registry [..]) [INSTALLING] baz v1.0.0 [COMPILING] baz v1.0.0 [FINISHED] release [optimized] target(s) in [..]