Skip to content

Commit 84a38b9

Browse files
authored
Don't canonicalize executable path in cargo_exe (#15355)
### What does this PR try to resolve? Follow up to #9991. The issue explains why but in short canonicalising executable paths causes issues for symlinks. It seems this was missed in #9991. ### How should we test and review this PR? Note that the comment I deleted is wrong since #9991. Running existing test should ensure this doesn't break anything.
2 parents a6c604d + d82b596 commit 84a38b9

File tree

4 files changed

+9
-26
lines changed

4 files changed

+9
-26
lines changed

src/cargo/util/context/mod.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ use crate::sources::CRATES_IO_REGISTRY;
7777
use crate::util::errors::CargoResult;
7878
use crate::util::network::http::configure_http_handle;
7979
use crate::util::network::http::http_handle;
80-
use crate::util::try_canonicalize;
8180
use crate::util::{internal, CanonicalUrl};
8281
use crate::util::{Filesystem, IntoUrl, IntoUrlWithBase, Rustc};
8382
use anyhow::{anyhow, bail, format_err, Context as _};
@@ -457,11 +456,10 @@ impl GlobalContext {
457456
// commands that use Cargo as a library to inherit (via `cargo <subcommand>`)
458457
// or set (by setting `$CARGO`) a correct path to `cargo` when the current exe
459458
// is not actually cargo (e.g., `cargo-*` binaries, Valgrind, `ld.so`, etc.).
460-
let exe = try_canonicalize(
461-
self.get_env_os(crate::CARGO_ENV)
462-
.map(PathBuf::from)
463-
.ok_or_else(|| anyhow!("$CARGO not set"))?,
464-
)?;
459+
let exe = self
460+
.get_env_os(crate::CARGO_ENV)
461+
.map(PathBuf::from)
462+
.ok_or_else(|| anyhow!("$CARGO not set"))?;
465463
Ok(exe)
466464
};
467465

@@ -470,7 +468,7 @@ impl GlobalContext {
470468
// The method varies per operating system and might fail; in particular,
471469
// it depends on `/proc` being mounted on Linux, and some environments
472470
// (like containers or chroots) may not have that available.
473-
let exe = try_canonicalize(env::current_exe()?)?;
471+
let exe = env::current_exe()?;
474472
Ok(exe)
475473
}
476474

@@ -481,8 +479,6 @@ impl GlobalContext {
481479
// Otherwise, it has multiple components and is either:
482480
// - a relative path (e.g., `./cargo`, `target/debug/cargo`), or
483481
// - an absolute path (e.g., `/usr/local/bin/cargo`).
484-
// In either case, `Path::canonicalize` will return the full absolute path
485-
// to the target if it exists.
486482
let argv0 = env::args_os()
487483
.map(PathBuf::from)
488484
.next()

tests/testsuite/build_script.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,9 @@ fn custom_build_env_vars() {
181181
)
182182
.file("bar/src/lib.rs", "pub fn hello() {}");
183183

184-
let cargo = cargo_exe().canonicalize().unwrap();
184+
let cargo = cargo_exe();
185185
let cargo = cargo.to_str().unwrap();
186-
let rustc = paths::resolve_executable("rustc".as_ref())
187-
.unwrap()
188-
.canonicalize()
189-
.unwrap();
186+
let rustc = paths::resolve_executable("rustc".as_ref()).unwrap();
190187
let rustc = rustc.to_str().unwrap();
191188
let file_content = format!(
192189
r##"

tests/testsuite/cargo_command.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ fn cargo_subcommand_env() {
375375
p.cargo("build").run();
376376
assert!(p.bin("cargo-envtest").is_file());
377377

378-
let cargo = cargo_exe().canonicalize().unwrap();
378+
let cargo = cargo_exe();
379379
let mut path = path();
380380
path.push(target_dir.clone());
381381
let path = env::join_paths(path.iter()).unwrap();
@@ -388,9 +388,7 @@ fn cargo_subcommand_env() {
388388
// Check that subcommands inherit an overridden $CARGO
389389
let envtest_bin = target_dir
390390
.join("cargo-envtest")
391-
.with_extension(std::env::consts::EXE_EXTENSION)
392-
.canonicalize()
393-
.unwrap();
391+
.with_extension(std::env::consts::EXE_EXTENSION);
394392
let envtest_bin = envtest_bin.to_str().unwrap();
395393
// Previously, `$CARGO` would be left at `envtest_bin`. However, with the
396394
// fix for #15099, `$CARGO` is now overwritten with the path to the current
@@ -623,8 +621,6 @@ fn overwrite_cargo_environment_variable() {
623621
let stderr_cargo = format!(
624622
"{}[EXE]\n",
625623
cargo_exe
626-
.canonicalize()
627-
.unwrap()
628624
.with_extension("")
629625
.to_str()
630626
.unwrap()
@@ -648,8 +644,6 @@ fn overwrite_cargo_environment_variable() {
648644
let stderr_other_cargo = format!(
649645
"{}[EXE]\n",
650646
other_cargo_path
651-
.canonicalize()
652-
.unwrap()
653647
.with_extension("")
654648
.to_str()
655649
.unwrap()

tests/testsuite/test.rs

-4
Original file line numberDiff line numberDiff line change
@@ -3890,8 +3890,6 @@ fn cargo_test_env() {
38903890
let cargo = format!(
38913891
"{}[EXE]",
38923892
cargo_exe()
3893-
.canonicalize()
3894-
.unwrap()
38953893
.with_extension("")
38963894
.to_str()
38973895
.unwrap()
@@ -3913,8 +3911,6 @@ test env_test ... ok
39133911
let stderr_other_cargo = format!(
39143912
"{}[EXE]",
39153913
other_cargo_path
3916-
.canonicalize()
3917-
.unwrap()
39183914
.with_extension("")
39193915
.to_str()
39203916
.unwrap()

0 commit comments

Comments
 (0)