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

Change default for RUSTUP_WINDOWS_PATH_ADD_BIN #3703

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Mar 11, 2024

This changes the default for the RUSTUP_WINDOWS_PATH_ADD_BIN opt-in to fix the issue where adding the bin directory on windows was preventing recursive calls to programs like cargo from being able to use the rustup proxies.

There was a call for testing at https://internals.rust-lang.org/t/help-test-windows-behavior-between-rustup-and-cargo/20237 and https://users.rust-lang.org/t/help-test-windows-behavior-between-rustup-and-cargo/106145. AFAIK, there have not been any indications of anyone having problems.

Fixes #3036

@rami3l rami3l requested a review from djc March 12, 2024 01:18
src/toolchain/toolchain.rs Outdated Show resolved Hide resolved
@ehuss ehuss force-pushed the change-windows-bin-path-default branch from 8dc8a39 to a34d06a Compare March 12, 2024 14:55
@djc djc added this pull request to the merge queue Mar 12, 2024
Merged via the queue into rust-lang:master with commit ce3c09a Mar 12, 2024
21 checks passed
@rami3l rami3l mentioned this pull request Apr 14, 2024
3 tasks
@cwfitzgerald
Copy link

cwfitzgerald commented May 12, 2024

Just raising that this seems to have broken cargo-nextest (issue) whenever there are proc-macro crates in the test suite. I don't know the details involved or which end this should be fixed on, just wanted to make the issue visible here. It seems like the nextest people have a way to fix it on their end, and setting RUSTUP_WINDOWS_PATH_ADD_BIN=1 in env allows things to work in the mean time, so this is non-blocking.

@smoelius
Copy link
Contributor

As an additional data point, Dylint is currently using RUSTUP_WINDOWS_PATH_ADD_BIN=1 to support Windows.

It may be that we could set PATH ourselves, but having this environment variable in rustup is at least a convenience.

@djc
Copy link
Contributor

djc commented May 13, 2024

Can one of you open a new issue discussing what the use case here is and how/why we broke it?

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.

rustup 1.25: On Windows, nested cargo invocation with a toolchain specified fails
5 participants