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

rustc: Fix more verbatim paths leaking to gcc #25134

Merged
merged 1 commit into from May 6, 2015

Conversation

alexcrichton
Copy link
Member

Turns out that a verbatim path was leaking through to gcc via the PATH
environment variable (pointing to the bundled gcc provided by the main
distribution) which was wreaking havoc when gcc itself was run. The fix here is
to just stop passing verbatim paths down by adding more liberal uses of
fix_windows_verbatim_for_gcc.

Closes #25072

Turns out that a verbatim path was leaking through to gcc via the PATH
environment variable (pointing to the bundled gcc provided by the main
distribution) which was wreaking havoc when gcc itself was run. The fix here is
to just stop passing verbatim paths down by adding more liberal uses of
`fix_windows_verbatim_for_gcc`.

Closes rust-lang#25072
@rust-highfive
Copy link
Collaborator

r? @nrc

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton alexcrichton force-pushed the fix-issue-25072-for-realsies branch from 02a3fef to 2dc0e56 Compare May 5, 2015 22:23
@brson
Copy link
Contributor

brson commented May 5, 2015

@bors r+ p=1

@bors
Copy link
Contributor

bors commented May 5, 2015

📌 Commit 2dc0e56 has been approved by brson

bors added a commit that referenced this pull request May 6, 2015
…rson

Turns out that a verbatim path was leaking through to gcc via the PATH
environment variable (pointing to the bundled gcc provided by the main
distribution) which was wreaking havoc when gcc itself was run. The fix here is
to just stop passing verbatim paths down by adding more liberal uses of
`fix_windows_verbatim_for_gcc`.

Closes #25072
@bors
Copy link
Contributor

bors commented May 6, 2015

⌛ Testing commit 2dc0e56 with merge 7bd7163...

@bors bors merged commit 2dc0e56 into rust-lang:master May 6, 2015
@klutzy
Copy link
Contributor

klutzy commented May 6, 2015

Uh. This raises another question: is there more risks to use "verbatim" \\?\ path? It seems that Windows 7 doesn't even understand \\?\-path in PATH environment variable. (set PATH=... didn't work on cmd) If so, it may be a bad idea to canonicalize all paths to \\?\-paths.

@alexcrichton alexcrichton deleted the fix-issue-25072-for-realsies branch May 6, 2015 04:32
@alexcrichton
Copy link
Member Author

@klutzy we're not intentionally canonicalizing to verbatim paths, it's just a side effect of using GetFinalPathNameByHandle, but I'd be open for trying to not return a verbatim path when possible!

@retep998
Copy link
Member

retep998 commented May 6, 2015

@alexcrichton Perhaps the env functions can strip the verbatim prefix when taking paths?

@alexcrichton
Copy link
Member Author

I'd prefer to not add special behavior to various modules to work around bugs in MinGW's gcc or ld, although Windows 7 not understanding them at all in PATH is also somewhat surprising. I'd probably still feel the same though that the standard library shouldn't be silently modifying the values passed to env::set_var to something different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nothing builds under latest nightly release on windows
7 participants