rustc_driver: Frob the global PATH less #26382
Merged
+11
−8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Environment variables are global state so this can lead to surprising results if the driver is called in a multithreaded environment (e.g. doctests). There shouldn't be any memory corruption that's possible, but a lot of the bots have been failing because they can't find `cc` or `gcc` in the path during doctests, and I highly suspect that it is due to the compiler modifying `PATH` in a multithreaded fashion. This commit moves the logic for appending to `PATH` to only affect the child process instead of also affecting the parent, at least for the linking stage. When loading dynamic libraries the compiler still modifies `PATH` on Windows, but this may be more difficult to fix than spawning off a new process.
|
r? @brson |
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
This is an example of problems I expect this PR to fix. That recently happened on the Windows bots. |
|
@bors r+ |
|
📌 Commit fcd99aa has been approved by |
bors
added a commit
that referenced
this pull request
Jun 20, 2015
Environment variables are global state so this can lead to surprising results if the driver is called in a multithreaded environment (e.g. doctests). There shouldn't be any memory corruption that's possible, but a lot of the bots have been failing because they can't find `cc` or `gcc` in the path during doctests, and I highly suspect that it is due to the compiler modifying `PATH` in a multithreaded fashion. This commit moves the logic for appending to `PATH` to only affect the child process instead of also affecting the parent, at least for the linking stage. When loading dynamic libraries the compiler still modifies `PATH` on Windows, but this may be more difficult to fix than spawning off a new process.
|
As of this PR, the msvc buildbot is now always failing to compile: |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Jun 22, 2015
This commit ensures that the modifications made in rust-lang#26382 also apply to the archive commands run in addition to the linker commands.
bors
added a commit
that referenced
this pull request
Jun 23, 2015
This commit ensures that the modifications made in #26382 also apply to the archive commands run in addition to the linker commands.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Environment variables are global state so this can lead to surprising results if
the driver is called in a multithreaded environment (e.g. doctests). There
shouldn't be any memory corruption that's possible, but a lot of the bots have
been failing because they can't find
ccorgccin the path during doctests,and I highly suspect that it is due to the compiler modifying
PATHin amultithreaded fashion.
This commit moves the logic for appending to
PATHto only affect the childprocess instead of also affecting the parent, at least for the linking stage.
When loading dynamic libraries the compiler still modifies
PATHon Windows,but this may be more difficult to fix than spawning off a new process.