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

Don't consider $HOME when finding the home dir on Windows #2604

Closed
wants to merge 1 commit into from

Conversation

brson
Copy link
Contributor

@brson brson commented Apr 21, 2016

Same as rustup. HOME is not a windows thing so allowing it
to control the cargo directory means that the directory can
change depending on whether you are working under mingw or not.

This change makes Cargo always use the windows AppData path.

This is a breaking change but if anybody is actually impacted
Cargo should just silently start using a different directory. There
may be surprises related to cargo install.

Users of rustup and multirust won't be affected because both
override CARGO_HOME.

Same as rustup. HOME is not a windows thing so allowing it
to control the cargo directory means that the directory can
change depending on whether you are working under mingw or not.

This change makes Cargo always use the windows AppData path.

This is a breaking change but if anybody is actually impacted
Cargo should just silently start using a different directory. There
may be surprises related to `cargo install`.

Users of rustup and multirust won't be affected because both
override CARGO_HOME.
@rust-highfive
Copy link

r? @huonw

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

@alexcrichton
Copy link
Member

I wonder if a simpler implementation may be to just remove HOME from the environment? It also seems like to keep cargo install working as it was previously that we should have logic like:

  1. If $HOME/.cargo exists, use that.
  2. Remove $HOME and check env::home_dir() and use that

@brson
Copy link
Contributor Author

brson commented Apr 26, 2016

Sure. I'll make those changes.

alexcrichton added a commit to alexcrichton/cargo that referenced this pull request May 13, 2016
This commit ensures that we always return the same fallback value on Windows
regardless of whichever shell we happen to be run from. We do this by removing
the `$HOME` environment variable which `std::env::home_dir` will inspect to
force it to fall back to the system APIs. If the old directory exists then we
favor that one, but otherwise we favor locations like `C:\Users\$user`

Supercedes and closes rust-lang#2604
bors added a commit that referenced this pull request May 13, 2016
Canonicalize CARGO_HOME fallback on Windows

This commit ensures that we always return the same fallback value on Windows
regardless of whichever shell we happen to be run from. We do this by removing
the `$HOME` environment variable which `std::env::home_dir` will inspect to
force it to fall back to the system APIs. If the old directory exists then we
favor that one, but otherwise we favor locations like `C:\Users\$user`

Supercedes and closes #2604
@bors bors closed this in #2681 May 13, 2016
@bors
Copy link
Contributor

bors commented May 13, 2016

☔ The latest upstream changes (presumably #2681) made this pull request unmergeable. Please resolve the merge conflicts.

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.

5 participants