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

Add PATH in post-install message when not modifying PATH #1126

Merged
merged 5 commits into from
May 23, 2017
Merged

Add PATH in post-install message when not modifying PATH #1126

merged 5 commits into from
May 23, 2017

Conversation

f-bro
Copy link
Contributor

@f-bro f-bro commented May 18, 2017 •
edited
Loading

Fixes #374

Unix:

To get started you need Cargo's bin directory ($HOME/.cargo/bin) in your PATH
environment variable.

Windows:

To get started you need Cargo's bin directory (C:\Users\f-bro\.cargo\bin) in
your PATH environment variable. This has not been done automatically.

@Diggsey
Copy link
Contributor

Diggsey commented May 21, 2017

Thanks for the PR!

I think it would be good to unify some of the logic here, across windows and linux:

  • Give canonical_cargo_home logic for both windows and non-windows.
  • Switch to using canonical_cargo_home() on all platforms
  • Add the bin path to the post_install_msg_win as well (it's currently missed)

@f-bro
Copy link
Contributor Author

f-bro commented May 21, 2017

Yes, it would be more consistent.
I will have a look tomorrow.

@f-bro
Copy link
Contributor Author

f-bro commented May 23, 2017

It looks like AppVeyor couldn't download rust-std:

Downloading rustup-init.exe (7,601,125 bytes)...100%
rustup-init.exe -y --default-toolchain=nightly-2017-04-25-x86_64-pc-windows-msvc
info: syncing channel updates for 'nightly-2017-04-25-x86_64-pc-windows-msvc'
info: downloading component 'rustc'
info: downloading component 'rust-std'

The console output stops here.

@Diggsey
Copy link
Contributor

Diggsey commented May 23, 2017

Probably just a network failure.

LGTM, except HOMEPATH should be USERPROFILE, (I just fixed that in the online editor)

@bors r

@bors
Copy link
Contributor

bors commented May 23, 2017

📌 Commit a2e465c has been approved by Diggsey

@bors
Copy link
Contributor

bors commented May 23, 2017

⌛ Testing commit a2e465c with merge cc79a6e...

bors added a commit that referenced this pull request May 23, 2017
@bors
Auto merge of #1126 - f-bro:issue-374, r=Diggsey
cc79a6e 
Add PATH in post-install message when not modifying PATH

Fixes #374

Unix:
```
To get started you need Cargo's bin directory ($HOME/.cargo/bin) in your PATH
environment variable.
```

Windows:
```
To get started you need Cargo's bin directory (C:\Users\f-bro\.cargo\bin) in
your PATH environment variable. This has not been done automatically.
```
@bors
Copy link
Contributor

bors commented May 23, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: Diggsey
Pushing cc79a6e to master...

@bors bors merged commit a2e465c into rust-lang:master May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@f-bro @Diggsey @bors