fix: display absolute path in the missing in PATH warning (#16125)

### What does this PR try to resolve?

close #16023

Improve the warning message for realtive install location.
1. Did a small refactoring to inline GlobalContext::get_path to prevent
potential misuse in the future.
2. Made Cargo always display the absolute install path in warnings.
3. Added a warning indicating that `install.root` paths without a
trailing backslash will be deprecated in the future.
### How to test and review this PR?

Please check the unit test.

r?@ghost

Author: weihanglo

src/cargo/ops/cargo_install.rs

@@ -316,7 +316,15 @@ impl<'gctx> InstallablePackage<'gctx> {
     fn install_one(mut self, dry_run: bool) -> CargoResult<bool> {
         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) {

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<Filesystem> {
-    let config_root = gctx.get_path("install.root")?;
+    let config_root = match gctx.get::<Option<ConfigRelativePath>>("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()))
 }

src/cargo/util/context/mod.rs

@@ -989,20 +989,6 @@ impl GlobalContext {
         self.get::<OptValue<String>>(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<OptValue<PathBuf>> {
-        self.get::<OptValue<ConfigRelativePath>>(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 {

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();