diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 466702903f1..0d18ba4670e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -316,7 +316,15 @@ impl<'gctx> InstallablePackage<'gctx> { fn install_one(mut self, dry_run: bool) -> CargoResult { self.gctx.shell().status("Installing", &self.pkg)?; + // Normalize to absolute path for consistency throughout. + // See: https://github.com/rust-lang/cargo/issues/16023 let dst = self.root.join("bin").into_path_unlocked(); + let cwd = self.gctx.cwd(); + let dst = if dst.is_absolute() { + paths::normalize_path(dst.as_path()) + } else { + paths::normalize_path(&cwd.join(&dst)) + }; let mut td_opt = None; let mut needs_cleanup = false; @@ -655,7 +663,15 @@ pub fn install( lockfile_path: Option<&Path>, ) -> CargoResult<()> { let root = resolve_root(root, gctx)?; + // Normalize to absolute path for consistency throughout. + // See: https://github.com/rust-lang/cargo/issues/16023 let dst = root.join("bin").into_path_unlocked(); + let cwd = gctx.cwd(); + let dst = if dst.is_absolute() { + paths::normalize_path(dst.as_path()) + } else { + paths::normalize_path(&cwd.join(&dst)) + }; let map = SourceConfigMap::new(gctx)?; let current_rust_version = if opts.honor_rust_version.unwrap_or(true) { diff --git a/src/cargo/ops/common_for_install_and_uninstall.rs b/src/cargo/ops/common_for_install_and_uninstall.rs index f39bfd4b15b..c073c127580 100644 --- a/src/cargo/ops/common_for_install_and_uninstall.rs +++ b/src/cargo/ops/common_for_install_and_uninstall.rs @@ -6,6 +6,7 @@ use std::path::{Path, PathBuf}; use std::rc::Rc; use std::task::Poll; +use annotate_snippets::Level; use anyhow::{Context as _, bail, format_err}; use cargo_util::paths; use cargo_util_schemas::core::PartialVersion; @@ -21,6 +22,7 @@ use crate::sources::source::QueryKind; use crate::sources::source::Source; use crate::util::GlobalContext; use crate::util::cache_lock::CacheLockMode; +use crate::util::context::{ConfigRelativePath, Definition}; use crate::util::errors::CargoResult; use crate::util::{FileLock, Filesystem}; @@ -545,11 +547,39 @@ impl InstallInfo { /// Determines the root directory where installation is done. pub fn resolve_root(flag: Option<&str>, gctx: &GlobalContext) -> CargoResult { - let config_root = gctx.get_path("install.root")?; + let config_root = match gctx.get::>("install.root")? { + Some(p) => { + let resolved = p.resolve_program(gctx); + if resolved.is_relative() { + let definition = p.value().definition.clone(); + if matches!(definition, Definition::Path(_)) { + let suggested = format!("{}/", resolved.display()); + let notes = [ + Level::NOTE.message("a future version of Cargo will treat it as relative to the configuration directory"), + Level::HELP.message(format!("add a trailing slash (`{}`) to adopt the correct behavior and silence this warning", suggested)), + Level::NOTE.message("see more at https://doc.rust-lang.org/cargo/reference/config.html#config-relative-paths"), + ]; + gctx.shell().print_report( + &[Level::WARNING + .secondary_title(format!( + "the `install.root` value `{}` defined in {} without a trailing slash is deprecated", + resolved.display(), + definition + )) + .elements(notes)], + false, + )?; + } + } + Some(resolved) + } + None => None, + }; + Ok(flag .map(PathBuf::from) .or_else(|| gctx.get_env_os("CARGO_INSTALL_ROOT").map(PathBuf::from)) - .or_else(move || config_root.map(|v| v.val)) + .or_else(|| config_root) .map(Filesystem::new) .unwrap_or_else(|| gctx.home().clone())) } diff --git a/src/cargo/util/context/mod.rs b/src/cargo/util/context/mod.rs index c90549bf371..33002ecdefd 100644 --- a/src/cargo/util/context/mod.rs +++ b/src/cargo/util/context/mod.rs @@ -981,20 +981,6 @@ impl GlobalContext { self.get::>(key) } - /// Get a config value that is expected to be a path. - /// - /// This returns a relative path if the value does not contain any - /// directory separators. See `ConfigRelativePath::resolve_program` for - /// more details. - pub fn get_path(&self, key: &str) -> CargoResult> { - self.get::>(key).map(|v| { - v.map(|v| Value { - val: v.val.resolve_program(self), - definition: v.definition, - }) - }) - } - fn string_to_path(&self, value: &str, definition: &Definition) -> PathBuf { let is_path = value.contains('/') || (cfg!(windows) && value.contains('\\')); if is_path { diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs index ae1a2d89d1e..2cb55a428e9 100644 --- a/tests/testsuite/install.rs +++ b/tests/testsuite/install.rs @@ -500,6 +500,157 @@ fn install_location_precedence() { assert_has_installed_exe(&t4, "foo"); } +#[cargo_test] +fn relative_install_location_without_trailing_slash() { + let p = project().file("src/main.rs", "fn main() {}").build(); + + let root = paths::root(); + let root_t1 = root.join("t1"); + let p_path = p.root().to_path_buf(); + let project_t1 = p_path.join("t1"); + + fs::create_dir(root.join(".cargo")).unwrap(); + fs::write( + root.join(".cargo/config.toml"), + r#" + [install] + root = "t1" + "#, + ) + .unwrap(); + + let mut cmd = cargo_process("install --path ."); + cmd.cwd(p.root()); + cmd.with_stderr_data(str![[r#" +[WARNING] the `install.root` value `t1` defined in [ROOT]/.cargo/config.toml without a trailing slash is deprecated + | + = [NOTE] a future version of Cargo will treat it as relative to the configuration directory + = [HELP] add a trailing slash (`t1/`) to adopt the correct behavior and silence this warning + = [NOTE] see more at https://doc.rust-lang.org/cargo/reference/config.html#config-relative-paths +[INSTALLING] foo v0.0.1 ([ROOT]/foo) +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s +[INSTALLING] [ROOT]/foo/t1/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1 ([ROOT]/foo)` (executable `foo[EXE]`) +[WARNING] be sure to add `[ROOT]/foo/t1/bin` to your PATH to be able to run the installed binaries + +"#]]) + .run(); + + // NOTE: the install location is relative to the CWD, not the config file + assert_has_not_installed_exe(&root_t1, "foo"); + assert_has_installed_exe(&project_t1, "foo"); +} + +#[cargo_test] +fn cli_root_argument_without_deprecation_warning() { + // Verify that using the --root CLI argument does not produce the deprecation warning. + let p = project().file("src/main.rs", "fn main() {}").build(); + + let root = paths::root(); + let root_t1 = root.join("t1"); + let p_path = p.root().to_path_buf(); + let project_t1 = p_path.join("t1"); + + cargo_process("install --path . --root") + .arg("t1") + .cwd(p.root()) + .with_stderr_data(str![[r#" +[INSTALLING] foo v0.0.1 ([ROOT]/foo) +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s +[INSTALLING] [ROOT]/foo/t1/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1 ([ROOT]/foo)` (executable `foo[EXE]`) +[WARNING] be sure to add `[ROOT]/foo/t1/bin` to your PATH to be able to run the installed binaries + +"#]]) + .run(); + assert_has_not_installed_exe(&root_t1, "foo"); + assert_has_installed_exe(&project_t1, "foo"); +} + +#[cargo_test] +fn relative_install_location_with_trailing_slash() { + let p = project().file("src/main.rs", "fn main() {}").build(); + + let root = paths::root(); + let root_t1 = root.join("t1"); + let p_path = p.root().to_path_buf(); + let project_t1 = p_path.join("t1"); + + fs::create_dir(root.join(".cargo")).unwrap(); + fs::write( + root.join(".cargo/config.toml"), + r#" + [install] + root = "t1/" + "#, + ) + .unwrap(); + + let mut cmd = cargo_process("install --path ."); + cmd.cwd(p.root()); + cmd.with_stderr_data(str![[r#" +[INSTALLING] foo v0.0.1 ([ROOT]/foo) +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s +[INSTALLING] [ROOT]/t1/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1 ([ROOT]/foo)` (executable `foo[EXE]`) +[WARNING] be sure to add `[ROOT]/t1/bin` to your PATH to be able to run the installed binaries + +"#]]) + .run(); + + assert_has_installed_exe(&root_t1, "foo"); + assert_has_not_installed_exe(&project_t1, "foo"); +} + +#[cargo_test] +fn relative_install_location_with_path_set() { + // Test that when the absolute install path is in PATH, no warning is shown + let p = project().file("src/main.rs", "fn main() {}").build(); + + let root = paths::root(); + let p_path = p.root().to_path_buf(); + let project_t1 = p_path.join("t1"); + + fs::create_dir(root.join(".cargo")).unwrap(); + fs::write( + root.join(".cargo/config.toml"), + r#" + [install] + root = "t1" + "#, + ) + .unwrap(); + + // Add the absolute path to PATH environment variable + let install_bin_path = project_t1.join("bin"); + let mut path = path(); + path.push(install_bin_path); + let new_path = env::join_paths(path).unwrap(); + + let mut cmd = cargo_process("install --path ."); + cmd.cwd(p.root()); + cmd.env("PATH", new_path); + cmd.with_stderr_data(str![[r#" +[WARNING] the `install.root` value `t1` defined in [ROOT]/.cargo/config.toml without a trailing slash is deprecated + | + = [NOTE] a future version of Cargo will treat it as relative to the configuration directory + = [HELP] add a trailing slash (`t1/`) to adopt the correct behavior and silence this warning + = [NOTE] see more at https://doc.rust-lang.org/cargo/reference/config.html#config-relative-paths +[INSTALLING] foo v0.0.1 ([ROOT]/foo) +[COMPILING] foo v0.0.1 ([ROOT]/foo) +[FINISHED] `release` profile [optimized] target(s) in [ELAPSED]s +[INSTALLING] [ROOT]/foo/t1/bin/foo[EXE] +[INSTALLED] package `foo v0.0.1 ([ROOT]/foo)` (executable `foo[EXE]`) + +"#]]) + .run(); + + assert_has_installed_exe(&project_t1, "foo"); +} + #[cargo_test] fn install_path() { let p = project().file("src/main.rs", "fn main() {}").build();