From 4a49a79ee9ecc3bf0f09294775617e42e33596ff Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 16 Apr 2019 09:53:48 +0900 Subject: [PATCH 1/4] Remove /lib from DYLD_FALLBACK_LIBRARY_PATH While the manual page for dyld says the default used when DYLD_FALLBACK_LIBRARY_PATH is not set is $(HOME)/lib:/usr/local/lib:/lib:/usr/lib, its code actually says ``` sLibraryFallbackPaths[] = { "$HOME/lib", "/usr/local/lib", "/usr/lib", NULL }; ``` as far back as the first version of dyld released in OSX 10.4: https://opensource.apple.com/source/dyld/dyld-43/src/dyld.cpp.auto.html (and the manual page was wrong back then too https://opensource.apple.com/source/dyld/dyld-43/doc/man/man1/dyld.1.auto.html) It is better to avoid putting more paths than necessary in this variable. --- src/cargo/core/compiler/compilation.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 13d5d96be57..84444184ec1 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -187,7 +187,6 @@ impl<'cfg> Compilation<'cfg> { 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())?; From 03b2087c03d681cbd3ef58c95cd67df45a7b0a9b Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 16 Apr 2019 10:17:26 +0900 Subject: [PATCH 2/4] Use env::var_os("HOME") rather than env::var("HOME") Because the value is used to create a Path, there's no reason to go all the way validating it is utf-8 (although in practice, it almost certainly is, but it doesn't even matter). --- src/cargo/core/compiler/compilation.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 84444184ec1..8dfdf748f36 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -183,7 +183,7 @@ impl<'cfg> Compilation<'cfg> { // 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") { + if let Some(home) = env::var_os("HOME") { search_path.push(PathBuf::from(home).join("lib")); } search_path.push(PathBuf::from("/usr/local/lib")); From 165be97df4c6b8960b89ad7ef82b87fdc8dea579 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Tue, 16 Apr 2019 13:49:40 +0900 Subject: [PATCH 3/4] Reset DYLD_FALLBACK_LIBRARY_PATH during run_link_system_path_macos test --- tests/testsuite/run.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index e596dede15f..fbc6a1e0e4d 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1223,6 +1223,9 @@ fn run_link_system_path_macos() { ) .unwrap(); p.root().rm_rf(); - p2.cargo("run").run(); - p2.cargo("test").run(); + const VAR: &str = "DYLD_FALLBACK_LIBRARY_PATH"; + // Reset DYLD_FALLBACK_LIBRARY_PATH so that we don't inherit anything that + // was set by the cargo that invoked the test. + p2.cargo("run").env_remove(VAR).run(); + p2.cargo("test").env_remove(VAR).run(); } From 416ed031d0beca3a3e5e02b0c3c6018daa452536 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Wed, 17 Apr 2019 08:37:46 +0900 Subject: [PATCH 4/4] Avoid adding DYLD_FALLBACK_LIBRARY_PATH defaults when it is set The macos dynamic linker behavior wrt DYLD_FALLBACK_LIBRARY_PATH is to use the value it is set with, and if there is no such value (the environment variable is either not set or set but empty), it uses a default value of $HOME/lib:/usr/local/lib:/usr/lib. Currently, cargo takes the value of DYLD_FALLBACK_LIBRARY_PATH, prepends its paths to it, and then unconditionally adds $HOME/lib:/usr/local/lib:/usr/lib, which in principle, shouldn't happen if DYLD_FALLBACK_LIBRARY_PATH was set originally. --- src/cargo/core/compiler/compilation.rs | 10 ++++++---- tests/testsuite/run.rs | 4 ++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index 8dfdf748f36..99fab990ae8 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -178,11 +178,13 @@ impl<'cfg> Compilation<'cfg> { search_path }; - search_path.extend(util::dylib_path().into_iter()); - if cfg!(target_os = "macos") { + let dylib_path = util::dylib_path(); + let dylib_path_is_empty = dylib_path.is_empty(); + 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. Since Cargo is explicitly setting the value, make sure the - // defaults still work. + // set or set to an empty string. Since Cargo is explicitly setting + // the value, make sure the defaults still work. if let Some(home) = env::var_os("HOME") { search_path.push(PathBuf::from(home).join("lib")); } diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index fbc6a1e0e4d..25494a51cc4 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1228,4 +1228,8 @@ fn run_link_system_path_macos() { // was set by the cargo that invoked the test. p2.cargo("run").env_remove(VAR).run(); p2.cargo("test").env_remove(VAR).run(); + // Ensure this still works when DYLD_FALLBACK_LIBRARY_PATH has + // a value set. + p2.cargo("run").env(VAR, &libdir).run(); + p2.cargo("test").env(VAR, &libdir).run(); }