Skip to content

Commit

Permalink
Auto merge of #14464 - linyihai:issue-14194, r=weihanglo
Browse files Browse the repository at this point in the history
fix: avoid inserting duplicate `dylib_path_envvar` when calling `cargo run` recursively

### What does this PR try to resolve?

If the current program started by `cargo run` recursively call into `cargo run`, the second `cargo run` will insert  `search_path`  into `dylib_path_envvar` again.

Fixes #14194

### How should we test and review this PR?

The first commit adds the test to reflect the issue. The first call to `cargo run` stores the dylib search path env var to a file. Subsequent calls verify that env var remains the same.

The second commit fixes the behavior by checking if env vars in `search_path` are a prefix of the slice of env vars in `dylib_path_envvar`.
  • Loading branch information
bors committed Oct 8, 2024
2 parents 2e309bd + 54dbc2b commit fbcd9bb
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,11 @@ impl<'gctx> Compilation<'gctx> {

let dylib_path = paths::dylib_path();
let dylib_path_is_empty = dylib_path.is_empty();
search_path.extend(dylib_path.into_iter());
if dylib_path.starts_with(&search_path) {
search_path = dylib_path;
} else {
search_path.extend(dylib_path.into_iter());
}
if cfg!(target_os = "macos") && dylib_path_is_empty {
// These are the defaults when DYLD_FALLBACK_LIBRARY_PATH isn't
// set or set to an empty string. Since Cargo is explicitly setting
Expand Down
60 changes: 60 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2154,6 +2154,66 @@ fn crate_library_path_env_var() {
setenv_for_removing_empty_component(p.cargo("run")).run();
}

// See https://github.com/rust-lang/cargo/issues/14194
#[cargo_test]
fn issue_14194_deduplicate_library_path_env_var() {
let p = project()
.file(
"src/main.rs",
&format!(
r#"
use std::process::Command;
fn main() {{
let level: i32 = std::env::args().nth(1).unwrap().parse().unwrap();
let txt = "var.txt";
let lib_path = std::env::var("{}").unwrap();
// Make sure we really have something in dylib search path.
let count = std::env::split_paths(&lib_path).count();
assert!(count > 0);
if level >= 3 {{
std::fs::write(txt, &lib_path).unwrap();
}} else {{
let prev_lib_path = std::fs::read_to_string(txt).unwrap();
// Ensure no duplicate insertion to dylib search paths
// when calling `cargo run` recursively.
assert_eq!(lib_path, prev_lib_path);
}}
if level == 0 {{
return;
}}
let _ = Command::new(std::env!("CARGO"))
.arg("run")
.arg("--")
.arg((level - 1).to_string())
.status()
.unwrap();
}}
"#,
dylib_path_envvar(),
),
)
.build();

setenv_for_removing_empty_component(p.cargo("run -- 3"))
.with_stderr_data(str![[r#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] `target/debug/foo[EXE] 3`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] `target/debug/foo[EXE] 2`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] `target/debug/foo[EXE] 1`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] `target/debug/foo[EXE] 0`
"#]])
.run();
}

// Regression test for #4277
#[cargo_test]
fn build_with_fake_libc_not_loading() {
Expand Down

0 comments on commit fbcd9bb

Please sign in to comment.