use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS #6355
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Revert part of 08960a3 Fixes https://trac.macports.org/ticket/57692 See rust-lang/cargo#6355
|
Thanks for the PR and seems like a nifty solution! Could comments be added to the code though indicating why this fallback is being used? |
|
I have confirmed that this fixes the massive performance issue we were having with MacOS 10.13 (https://travis-ci.org/ehuss/cargo/builds/461986852). I think the same fix will need to be applied to rustup as well. I'm actually not sure why rustup sets |
|
@ehuss I think at the time it was added to rustup it may not have been configured right? Or was otherwise targeted at getting things like rustc plugins working (which have sort of long since gone away...). In any case, can investigate that later. That's good news though! @MarcusCalhoun-Lopez would you be up for adding rationale plus some other comments to this to land it? |
|
ping @MarcusCalhoun-Lopez have you had a chance to take another look at this? |
|
Thank you for the reminder @alexcrichton. I add further comments either today or tomorrow. |
When loading and linking a dynamic library or bundle, dlopen searches in LD_LIBRARY_PATH, DYLD_LIBRARY_PATH, PWD, and DYLD_FALLBACK_LIBRARY_PATH. In the Mach-O format, a dynamic library has an "install path." Clients linking against the library record this path, and the dynamic linker, dyld, uses it to locate the library. dyld searches DYLD_LIBRARY_PATH *before* the install path. dyld searches DYLD_FALLBACK_LIBRARY_PATH only if it cannot find the library in the install path. Setting DYLD_LIBRARY_PATH can easily have unintended consequences. See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10 See https://trac.macports.org/ticket/57692
MarcusCalhoun-Lopez
force-pushed
the
dyld_fix
branch
from
1a916cd
to
b7516d3
Compare
|
Sorry for the delay, but I have added more information in the commit message and in the code comments. |
|
I'm concerned that this overrides the default It might also be helpful to include a note that beginning in 10.13 |
|
📌 Commit b7516d3 has been approved by |
use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10 See https://trac.macports.org/ticket/57692
|
☀️ Test successful - status-appveyor, status-travis |
Update cargo 24 commits in 0d1f1bbeabd5b43a7f3ecfa16540af8e76d5efb4..34320d212dca8cd27d06ce93c16c6151f46fcf2e 2018-12-19 14:45:14 +0000 to 2019-01-03 19:12:38 +0000 - Display environment variables for rustc commands (rust-lang/cargo#6492) - Fix a very minor race condition in `cargo fix`. (rust-lang/cargo#6515) - Add a high-level overview of how `fix` works. (rust-lang/cargo#6516) - Add dependency `registry` to `cargo metadata`. (rust-lang/cargo#6500) - Fix fingerprint calculation for patched deps. (rust-lang/cargo#6493) - serialize version directly (rust-lang/cargo#6512) - use DYLD_FALLBACK_LIBRARY_PATH for dylib_path_envvar on macOS (rust-lang/cargo#6355) - Fix error message when resolving dependencies (rust-lang/cargo#6510) - use PathBuf in cargo metadata (rust-lang/cargo#6511) - Fixed link to testsuite in CONTRIBUTING.md (rust-lang/cargo#6506) - Update display of contents of Cargo.toml (rust-lang/cargo#6501) - Update display of contents of Cargo.toml (rust-lang/cargo#6502) - Fixup cargo install's help message (rust-lang/cargo#6495) - testsuite: Require failing commands to check output. (rust-lang/cargo#6497) - Delete unnecessary 'return' (rust-lang/cargo#6496) - Fix new unused patch warning. (rust-lang/cargo#6494) - Some minor documentation changes. (rust-lang/cargo#6481) - Add `links` to `cargo metadata`. (rust-lang/cargo#6480) - Salvaged semver work (rust-lang/cargo#6476) - Warn on unused patches. (rust-lang/cargo#6470) - don't write a an incorrect rustc version to the fingerprint file (rust-lang/cargo#6473) - Rewrite `login` and registry cleanups. (rust-lang/cargo#6466) - [issue#6461] Fix cargo commands list (rust-lang/cargo#6462) - Restrict registry names to same style as package names. (rust-lang/cargo#6469)
Fix default DYLD_FALLBACK_LIBRARY_PATH on MacOS. On MacOS, dyld uses the default `$(HOME)/lib:/usr/local/lib:/lib:/usr/lib` for `DYLD_FALLBACK_LIBRARY_PATH` when it is not set. #6355 switched cargo to use `DYLD_FALLBACK_LIBRARY_PATH`, which means the default paths no longer worked. This explicitly adds the defaults back to the end of the list. Fixes #6623
When loading and linking a dynamic library or bundle, dlopen searches in LD_LIBRARY_PATH, DYLD_LIBRARY_PATH, PWD, and DYLD_FALLBACK_LIBRARY_PATH. In the Mach-O format, a dynamic library has an "install path." Clients linking against the library record this path, and the dynamic linker, dyld, uses it to locate the library. dyld searches DYLD_LIBRARY_PATH *before* the install path. dyld searches DYLD_FALLBACK_LIBRARY_PATH only if it cannot find the library in the install path. Setting DYLD_LIBRARY_PATH can easily have unintended consequences. See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10 See https://trac.macports.org/ticket/57692 See https://bugzilla.mozilla.org/show_bug.cgi?id=1536486 This is the same change as was applied to cargo in rust-lang/cargo#6355
When loading and linking a dynamic library or bundle, dlopen searches in LD_LIBRARY_PATH, DYLD_LIBRARY_PATH, PWD, and DYLD_FALLBACK_LIBRARY_PATH. In the Mach-O format, a dynamic library has an "install path." Clients linking against the library record this path, and the dynamic linker, dyld, uses it to locate the library. dyld searches DYLD_LIBRARY_PATH *before* the install path. dyld searches DYLD_FALLBACK_LIBRARY_PATH only if it cannot find the library in the install path. Setting DYLD_LIBRARY_PATH can easily have unintended consequences. See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10 See https://trac.macports.org/ticket/57692 See https://bugzilla.mozilla.org/show_bug.cgi?id=1536486 This is the same change as was applied to cargo in rust-lang/cargo#6355
When loading and linking a dynamic library or bundle, dlopen searches in LD_LIBRARY_PATH, DYLD_LIBRARY_PATH, PWD, and DYLD_FALLBACK_LIBRARY_PATH. In the Mach-O format, a dynamic library has an "install path." Clients linking against the library record this path, and the dynamic linker, dyld, uses it to locate the library. dyld searches DYLD_LIBRARY_PATH *before* the install path. dyld searches DYLD_FALLBACK_LIBRARY_PATH only if it cannot find the library in the install path. Setting DYLD_LIBRARY_PATH can easily have unintended consequences. See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10 See https://trac.macports.org/ticket/57692 See https://bugzilla.mozilla.org/show_bug.cgi?id=1536486 This is the same change as was applied to cargo in rust-lang/cargo#6355 See also rust-lang/cargo#6625 and rust-lang/cargo#6625
See https://users.rust-lang.org/t/subprocess-and-dynamic-library-linking-problem-interaction/7873/10
See https://trac.macports.org/ticket/57692