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.

Sign up for GitHub

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

Jump to bottom

Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling #6856

Merged
merged 4 commits into from
Apr 17, 2019
Merged

Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling #6856

merged 4 commits into from
Apr 17, 2019

Conversation

glandium
Copy link
Contributor

No description provided.

glandium added 2 commits April 16, 2019 09:53
@glandium
Remove /lib from DYLD_FALLBACK_LIBRARY_PATH
4a49a79 
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.
@glandium
Use env::var_os("HOME") rather than env::var("HOME")
03b2087 
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).
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 16, 2019
@glandium glandium force-pushed the DYLD_FALLBACK_LIBRARY_PATH branch from 989406f to baca037 Compare April 16, 2019 04:59
alexcrichton
alexcrichton reviewed Apr 16, 2019
View reviewed changes
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me, thanks! Did this end up causing problems in practice out of curiosity or was this just cleanup?

tests/testsuite/run.rs Outdated
// was set by the cargo that invoked the test.
const VAR: &str = "DYLD_FALLBACK_LIBRARY_PATH";
let old_value = std::env::var_os(VAR);
std::env::remove_var(VAR);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can unfortunately poison the state for other tests since env vars are global and the tests are all currently multi-threaded, but this could be extracted to its own test suite if necessary which isn't run in parallel with other tests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The env var can be set on the process builder, I don't think it is necessary to set it globally like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't realized this was a process builder wrapper.

ehuss
ehuss reviewed Apr 16, 2019
View reviewed changes
src/cargo/core/compiler/compilation.rs
search_path.push(PathBuf::from(home).join("lib"));
}
search_path.push(PathBuf::from("/usr/local/lib"));
search_path.push(PathBuf::from("/lib"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove /lib?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's explained in the corresponding commit message. In short, contrary to what the dyld documentation says, it's not in the default, and never has been (and the doc has always been wrong).

@glandium
Copy link
Contributor Author

Looks reasonable to me, thanks! Did this end up causing problems in practice out of curiosity or was this just cleanup?

Cleanup for things I noticed while working on rust-lang/rustup#1752

@glandium glandium force-pushed the DYLD_FALLBACK_LIBRARY_PATH branch from baca037 to b60d7c8 Compare April 16, 2019 23:35
glandium added 2 commits April 17, 2019 08:36
@glandium
Reset DYLD_FALLBACK_LIBRARY_PATH during run_link_system_path_macos test
165be97
@glandium
Avoid adding DYLD_FALLBACK_LIBRARY_PATH defaults when it is set
416ed03 
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.
@glandium glandium force-pushed the DYLD_FALLBACK_LIBRARY_PATH branch from b60d7c8 to 416ed03 Compare April 16, 2019 23:38
@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Apr 17, 2019

📌 Commit 416ed03 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2019
@bors
Copy link
Contributor

bors commented Apr 17, 2019

⌛ Testing commit 416ed03 with merge b978d11...

bors added a commit that referenced this pull request Apr 17, 2019
@bors
Auto merge of #6856 - glandium:DYLD_FALLBACK_LIBRARY_PATH, r=alexcric…
b978d11 
…hton

Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling
@bors
Copy link
Contributor

bors commented Apr 17, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing b978d11 to master...

@bors bors merged commit 416ed03 into rust-lang:master Apr 17, 2019
@ehuss ehuss mentioned this pull request Apr 22, 2019
Merged
kennytm added a commit to kennytm/rust that referenced this pull request Apr 23, 2019
@kennytm
Rollup merge of rust-lang#60180 - ehuss:update-cargo-books, r=alexcri…
2a2198c 
…chton

Update cargo, books

## cargo

5 commits in b6581d383ed596b133e330011658c6f83cf85c2f..6be12653dcefb46ee7b605f063ee75b5e6cba513
2019-04-16 16:02:11 +0000 to 2019-04-19 15:05:03 +0000
- Improved docs for `maintenance` options (rust-lang/cargo#6863)
- publish-lockfile: Various updates (rust-lang/cargo#6840)
- Treat HTTP/2 stream errors as spurious network errors. (rust-lang/cargo#6861)
- Validate registry token before operations that require it. (rust-lang/cargo#6854)
- Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling (rust-lang/cargo#6856)

## reference

2 commits in 98f90ff..2a2de9c
2019-04-06 09:29:08 -0700 to 2019-04-22 10:25:52 -0700
- Remove unused link references. (rust-lang/reference#560)
- Fix attribute redirects. (rust-lang/reference#562)

## book

22 commits in b93ec30bbc7b1b5c2f44223249ab359bed2ed5a6..db919bc6bb9071566e9c4f05053672133eaac33e
2019-03-26 16:54:10 -0400 to 2019-04-15 20:11:03 -0400
- Link to chapters mentioned in chapter 12
- Split up a long sentence
- Unclear wording 4.3 (rust-lang/book#1907)
- Corrected error for array out of bounds (rust-lang/book#1900)
- Make lifetime explanation clearer (rust-lang/book#1901)
- Replace `T: 'a + Messenger` with `T: Messenger` (rust-lang/book#1831)
- Update range so matches rust-fmt . (rust-lang/book#1890)
- Adding trailing comma (rust-lang/book#1891)
- point 2018 book redirects to existing pages instead of index (rust-lang/book#1919)
- Update ch04-03-slices.md (rust-lang/book#1921)
- Update link for Russian translation. (rust-lang/book#1915)
- Ch7 layout (rust-lang/book#1917)
- Update the version of mdbook we use in-tree to match rust-lang/rust (rust-lang/book#1912)
- Fix spellingz
- Update listings in ch 19-6 for nostarch
- Add a high-level overview of the changes in this version of the book
- Fix Travis CI badge url (rust-lang/book#1893)
- Redo listing numbers in chapter 19 after removals
- Remove Advanced Lifetimes section completely
- Merge branch 'gh1780'
- Merge remote-tracking branch 'origin/master' into gh1567
- remove lifetime subtyping

## rust-by-example

4 commits in f68ef3d0f4959f6a7d92a08d9994b117f0f4d32d..1ff0f8e018838a710ebc0cc1a7bf74ebe73ad9f1
2019-03-12 15:32:12 -0300 to 2019-04-15 08:15:32 -0300
- Fix borrow so it fails in 2018 edition Fixes rust-lang/rust-by-example#1141 (rust-lang/rust-by-example#1152)
- Replace lvalue and rvalue with place and value (rust-lang/rust-by-example#1160)
- Mutate array in iter_mut() example (rust-lang/rust-by-example#1165)
- Fix a typo ("half" -> "halve") (rust-lang/rust-by-example#1172)

## rustc-guide

8 commits in 464cb5b..99e1b1d
2019-03-23 18:39:14 -0500 to 2019-04-20 09:57:54 -0500
- Update BodyId description
- Update test-implementation chapter to current code
- update chalk with new organization
- move to subsection
- fix MovePathIndex link
- Update query chapter for the query macro rewrite
- subchapter with information about `--error-format json`
- Update query-evaluation-model-in-detail.md

## edition-guide

1 commits in b56ddb11548450a6df4edd1ed571b2bc304eb9e6..c413d42a207bd082f801ec0137c31b71e4bfed4c
2019-03-10 10:23:16 +0100 to 2019-04-22 01:14:56 +0200
- fix command (rust-lang/edition-guide#155)

## embedded-book

1 commits in 7989c723607ef5b13b57208022259e6c771e11d0..de3d55f521e657863df45260ebbca1b10527f662
2019-04-04 12:14:37 +0000 to 2019-04-22 12:58:28 +0000
- Minor fixes  (rust-embedded/book#185)

## nomicon

6 commits in c02e0e7754a76886e55b976a3a4fac20100cd35d..fb29b147be4d9a1f8e24aba753a7e1de537abf61
2019-03-25 16:52:56 -0400 to 2019-04-22 19:10:29 -0400
- Fix link to copy_nonoverlapping (rust-lang/nomicon#134)
- Various unchecked-uninit improvements (rust-lang/nomicon#130)
- OOM behaviour in `vec-alloc.md` (rust-lang/nomicon#133)
- Added missing "things". (rust-lang/nomicon#131)
- Fix number agreement in subtyping chapter (rust-lang/nomicon#128)
- Minor improvements (rust-lang/nomicon#129)
Centril added a commit to Centril/rust that referenced this pull request Apr 24, 2019
@Centril
Rollup merge of rust-lang#60180 - ehuss:update-cargo-books, r=alexcri…
4766b09 
…chton

Update cargo, books

## cargo

5 commits in b6581d383ed596b133e330011658c6f83cf85c2f..6be12653dcefb46ee7b605f063ee75b5e6cba513
2019-04-16 16:02:11 +0000 to 2019-04-19 15:05:03 +0000
- Improved docs for `maintenance` options (rust-lang/cargo#6863)
- publish-lockfile: Various updates (rust-lang/cargo#6840)
- Treat HTTP/2 stream errors as spurious network errors. (rust-lang/cargo#6861)
- Validate registry token before operations that require it. (rust-lang/cargo#6854)
- Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling (rust-lang/cargo#6856)

## reference

2 commits in 98f90ff..2a2de9c
2019-04-06 09:29:08 -0700 to 2019-04-22 10:25:52 -0700
- Remove unused link references. (rust-lang/reference#560)
- Fix attribute redirects. (rust-lang/reference#562)

## book

22 commits in b93ec30bbc7b1b5c2f44223249ab359bed2ed5a6..db919bc6bb9071566e9c4f05053672133eaac33e
2019-03-26 16:54:10 -0400 to 2019-04-15 20:11:03 -0400
- Link to chapters mentioned in chapter 12
- Split up a long sentence
- Unclear wording 4.3 (rust-lang/book#1907)
- Corrected error for array out of bounds (rust-lang/book#1900)
- Make lifetime explanation clearer (rust-lang/book#1901)
- Replace `T: 'a + Messenger` with `T: Messenger` (rust-lang/book#1831)
- Update range so matches rust-fmt . (rust-lang/book#1890)
- Adding trailing comma (rust-lang/book#1891)
- point 2018 book redirects to existing pages instead of index (rust-lang/book#1919)
- Update ch04-03-slices.md (rust-lang/book#1921)
- Update link for Russian translation. (rust-lang/book#1915)
- Ch7 layout (rust-lang/book#1917)
- Update the version of mdbook we use in-tree to match rust-lang/rust (rust-lang/book#1912)
- Fix spellingz
- Update listings in ch 19-6 for nostarch
- Add a high-level overview of the changes in this version of the book
- Fix Travis CI badge url (rust-lang/book#1893)
- Redo listing numbers in chapter 19 after removals
- Remove Advanced Lifetimes section completely
- Merge branch 'gh1780'
- Merge remote-tracking branch 'origin/master' into gh1567
- remove lifetime subtyping

## rust-by-example

4 commits in f68ef3d0f4959f6a7d92a08d9994b117f0f4d32d..1ff0f8e018838a710ebc0cc1a7bf74ebe73ad9f1
2019-03-12 15:32:12 -0300 to 2019-04-15 08:15:32 -0300
- Fix borrow so it fails in 2018 edition Fixes rust-lang/rust-by-example#1141 (rust-lang/rust-by-example#1152)
- Replace lvalue and rvalue with place and value (rust-lang/rust-by-example#1160)
- Mutate array in iter_mut() example (rust-lang/rust-by-example#1165)
- Fix a typo ("half" -> "halve") (rust-lang/rust-by-example#1172)

## rustc-guide

8 commits in 464cb5b..99e1b1d
2019-03-23 18:39:14 -0500 to 2019-04-20 09:57:54 -0500
- Update BodyId description
- Update test-implementation chapter to current code
- update chalk with new organization
- move to subsection
- fix MovePathIndex link
- Update query chapter for the query macro rewrite
- subchapter with information about `--error-format json`
- Update query-evaluation-model-in-detail.md

## edition-guide

1 commits in b56ddb11548450a6df4edd1ed571b2bc304eb9e6..c413d42a207bd082f801ec0137c31b71e4bfed4c
2019-03-10 10:23:16 +0100 to 2019-04-22 01:14:56 +0200
- fix command (rust-lang/edition-guide#155)

## embedded-book

1 commits in 7989c723607ef5b13b57208022259e6c771e11d0..de3d55f521e657863df45260ebbca1b10527f662
2019-04-04 12:14:37 +0000 to 2019-04-22 12:58:28 +0000
- Minor fixes  (rust-embedded/book#185)

## nomicon

6 commits in c02e0e7754a76886e55b976a3a4fac20100cd35d..fb29b147be4d9a1f8e24aba753a7e1de537abf61
2019-03-25 16:52:56 -0400 to 2019-04-22 19:10:29 -0400
- Fix link to copy_nonoverlapping (rust-lang/nomicon#134)
- Various unchecked-uninit improvements (rust-lang/nomicon#130)
- OOM behaviour in `vec-alloc.md` (rust-lang/nomicon#133)
- Added missing "things". (rust-lang/nomicon#131)
- Fix number agreement in subtyping chapter (rust-lang/nomicon#128)
- Minor improvements (rust-lang/nomicon#129)
bors added a commit to rust-lang/rust that referenced this pull request Apr 24, 2019
@bors
Auto merge of #60180 - ehuss:update-cargo-books, r=alexcrichton
e305df1 
Update cargo, books

## cargo

5 commits in b6581d383ed596b133e330011658c6f83cf85c2f..6be12653dcefb46ee7b605f063ee75b5e6cba513
2019-04-16 16:02:11 +0000 to 2019-04-19 15:05:03 +0000
- Improved docs for `maintenance` options (rust-lang/cargo#6863)
- publish-lockfile: Various updates (rust-lang/cargo#6840)
- Treat HTTP/2 stream errors as spurious network errors. (rust-lang/cargo#6861)
- Validate registry token before operations that require it. (rust-lang/cargo#6854)
- Cleanups wrt DYLD_FALLBACK_LIBRARY_PATH handling (rust-lang/cargo#6856)

## reference

2 commits in 98f90ff..2a2de9c
2019-04-06 09:29:08 -0700 to 2019-04-22 10:25:52 -0700
- Remove unused link references. (rust-lang/reference#560)
- Fix attribute redirects. (rust-lang/reference#562)

## book

22 commits in b93ec30bbc7b1b5c2f44223249ab359bed2ed5a6..db919bc6bb9071566e9c4f05053672133eaac33e
2019-03-26 16:54:10 -0400 to 2019-04-15 20:11:03 -0400
- Link to chapters mentioned in chapter 12
- Split up a long sentence
- Unclear wording 4.3 (rust-lang/book#1907)
- Corrected error for array out of bounds (rust-lang/book#1900)
- Make lifetime explanation clearer (rust-lang/book#1901)
- Replace `T: 'a + Messenger` with `T: Messenger` (rust-lang/book#1831)
- Update range so matches rust-fmt . (rust-lang/book#1890)
- Adding trailing comma (rust-lang/book#1891)
- point 2018 book redirects to existing pages instead of index (rust-lang/book#1919)
- Update ch04-03-slices.md (rust-lang/book#1921)
- Update link for Russian translation. (rust-lang/book#1915)
- Ch7 layout (rust-lang/book#1917)
- Update the version of mdbook we use in-tree to match rust-lang/rust (rust-lang/book#1912)
- Fix spellingz
- Update listings in ch 19-6 for nostarch
- Add a high-level overview of the changes in this version of the book
- Fix Travis CI badge url (rust-lang/book#1893)
- Redo listing numbers in chapter 19 after removals
- Remove Advanced Lifetimes section completely
- Merge branch 'gh1780'
- Merge remote-tracking branch 'origin/master' into gh1567
- remove lifetime subtyping

## rust-by-example

4 commits in f68ef3d0f4959f6a7d92a08d9994b117f0f4d32d..1ff0f8e018838a710ebc0cc1a7bf74ebe73ad9f1
2019-03-12 15:32:12 -0300 to 2019-04-15 08:15:32 -0300
- Fix borrow so it fails in 2018 edition Fixes rust-lang/rust-by-example#1141 (rust-lang/rust-by-example#1152)
- Replace lvalue and rvalue with place and value (rust-lang/rust-by-example#1160)
- Mutate array in iter_mut() example (rust-lang/rust-by-example#1165)
- Fix a typo ("half" -> "halve") (rust-lang/rust-by-example#1172)

## rustc-guide

8 commits in 464cb5b..99e1b1d
2019-03-23 18:39:14 -0500 to 2019-04-20 09:57:54 -0500
- Update BodyId description
- Update test-implementation chapter to current code
- update chalk with new organization
- move to subsection
- fix MovePathIndex link
- Update query chapter for the query macro rewrite
- subchapter with information about `--error-format json`
- Update query-evaluation-model-in-detail.md

## edition-guide

1 commits in b56ddb11548450a6df4edd1ed571b2bc304eb9e6..c413d42a207bd082f801ec0137c31b71e4bfed4c
2019-03-10 10:23:16 +0100 to 2019-04-22 01:14:56 +0200
- fix command (rust-lang/edition-guide#155)

## embedded-book

1 commits in 7989c723607ef5b13b57208022259e6c771e11d0..de3d55f521e657863df45260ebbca1b10527f662
2019-04-04 12:14:37 +0000 to 2019-04-22 12:58:28 +0000
- Minor fixes  (rust-embedded/book#185)

## nomicon

6 commits in c02e0e7754a76886e55b976a3a4fac20100cd35d..fb29b147be4d9a1f8e24aba753a7e1de537abf61
2019-03-25 16:52:56 -0400 to 2019-04-22 19:10:29 -0400
- Fix link to copy_nonoverlapping (rust-lang/nomicon#134)
- Various unchecked-uninit improvements (rust-lang/nomicon#130)
- OOM behaviour in `vec-alloc.md` (rust-lang/nomicon#133)
- Added missing "things". (rust-lang/nomicon#131)
- Fix number agreement in subtyping chapter (rust-lang/nomicon#128)
- Minor improvements (rust-lang/nomicon#129)
@ehuss ehuss added this to the 1.36.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

@ehuss ehuss ehuss left review comments

Assignees

@nrc nrc

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.36.0
Development

Successfully merging this pull request may close these issues.
6 participants
@glandium @rust-highfive @alexcrichton @bors @ehuss @nrc