Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default DYLD_FALLBACK_LIBRARY_PATH on MacOS. #6625

Merged
merged 1 commit into from
Feb 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,17 @@ impl<'cfg> Compilation<'cfg> {
};

search_path.extend(util::dylib_path().into_iter());
if cfg!(target_os = "macos") {
// These are the defaults when DYLD_FALLBACK_LIBRARY_PATH isn't
// set. Since Cargo is explicitly setting the value, make sure the
// defaults still work.
if let Ok(home) = env::var("HOME") {
search_path.push(PathBuf::from(home).join("lib"));
}
search_path.push(PathBuf::from("/usr/local/lib"));
search_path.push(PathBuf::from("/lib"));
search_path.push(PathBuf::from("/usr/lib"));
}
let search_path = join_paths(&search_path, util::dylib_path_envvar())?;

cmd.env(util::dylib_path_envvar(), &search_path);
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/util/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ pub fn dylib_path_envvar() -> &'static str {
// find the library in the install path.
// Setting DYLD_LIBRARY_PATH can easily have unintended
// consequences.
//
// Also, DYLD_LIBRARY_PATH appears to have significant performance
// penalty starting in 10.13. Cargo's testsuite ran more than twice as
// slow with it on CI.
"DYLD_FALLBACK_LIBRARY_PATH"
} else {
"LD_LIBRARY_PATH"
Expand Down
85 changes: 85 additions & 0 deletions tests/testsuite/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,3 +1136,88 @@ fn default_run_workspace() {
.with_stdout("run-a")
.run();
}

#[test]
#[cfg(target_os = "macos")]
fn run_link_system_path_macos() {
use crate::support::paths::{self, CargoPathExt};
use std::fs;
// Check that the default system library path is honored.
// First, build a shared library that will be accessed from
// DYLD_FALLBACK_LIBRARY_PATH.
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
[lib]
crate-type = ["cdylib"]
"#,
)
.file(
"src/lib.rs",
"#[no_mangle] pub extern fn something_shared() {}",
)
.build();
p.cargo("build").run();

// This is convoluted. Since this test can't modify things in /usr,
// this needs to dance around to check that things work.
//
// The default DYLD_FALLBACK_LIBRARY_PATH is:
// $(HOME)/lib:/usr/local/lib:/lib:/usr/lib
//
// This will make use of ~/lib in the path, but the default cc link
// path is /usr/lib:/usr/local/lib. So first need to build in one
// location, and then move it to ~/lib.
//
// 1. Build with rustc-link-search pointing to libfoo so the initial
// binary can be linked.
// 2. Move the library to ~/lib
// 3. Run `cargo run` to make sure it can still find the library in
// ~/lib.
//
// This should be equivalent to having the library in /usr/local/lib.
let p2 = project()
.at("bar")
.file("Cargo.toml", &basic_bin_manifest("bar"))
.file(
"src/main.rs",
r#"
extern {
fn something_shared();
}
fn main() {
unsafe { something_shared(); }
}
"#,
)
.file(
"build.rs",
&format!(
r#"
fn main() {{
println!("cargo:rustc-link-lib=foo");
println!("cargo:rustc-link-search={}");
}}
"#,
p.target_debug_dir().display()
),
)
.build();
p2.cargo("build").run();
p2.cargo("test").run();

let libdir = paths::home().join("lib");
fs::create_dir(&libdir).unwrap();
fs::rename(
p.target_debug_dir().join("libfoo.dylib"),
libdir.join("libfoo.dylib"),
)
.unwrap();
p.root().rm_rf();
p2.cargo("run").run();
p2.cargo("test").run();
}