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

Output the previous version of a toolchain when it is updated #2143

Merged
merged 1 commit into from Dec 6, 2019

Conversation

@RobbieClarken
Copy link
Contributor

RobbieClarken commented Dec 4, 2019

This change modifies the cli output of rustup update to display the previous version of rustc for toolchains that are updated.

Before:

$ rustup update

info: syncing channel updates for 'stable-x86_64-apple-darwin'
info: syncing channel updates for 'nightly-x86_64-apple-darwin'
--snip--
  stable-x86_64-apple-darwin unchanged - rustc 1.39.0 (4560ea788 2019-11-04)
   nightly-x86_64-apple-darwin updated - rustc 1.41.0-nightly (7afe6d9d1 2019-12-03)

After:

$ rustup update

info: syncing channel updates for 'stable-x86_64-apple-darwin'
info: syncing channel updates for 'nightly-x86_64-apple-darwin'
--snip--
  stable-x86_64-apple-darwin unchanged - rustc 1.39.0 (4560ea788 2019-11-04)
   nightly-x86_64-apple-darwin updated - rustc 1.41.0-nightly (7afe6d9d1 2019-12-03) (from rustc 1.40.0-nightly (22bc9e1d9 2019-09-30))

Fixes #2110.

@RobbieClarken RobbieClarken force-pushed the RobbieClarken:show-from-version branch from 21806e1 to 08370ae Dec 5, 2019
Copy link
Collaborator

kinnison left a comment

I feel like the work you did in fn install() in src/toolchain.rs could be simplified -- the exists boolean and the previous_version variable could be simplified into a single previous_version where instead of exists you have previous_version.is_some() -- However this isn't necessary right now.

If you want to clean up a little, then let me know, if you'd rather not attempt that now then that's fine too -- let me know that and I'll move on.

@kinnison kinnison added this to the 1.21.0 milestone Dec 5, 2019
@RobbieClarken

This comment has been minimized.

Copy link
Contributor Author

RobbieClarken commented Dec 5, 2019

I feel like the work you did in fn install() in src/toolchain.rs could be simplified -- the exists boolean and the previous_version variable could be simplified into a single previous_version where instead of exists you have previous_version.is_some()

Good point! I've made that change. Let me know if it looks good and I'll squash the commits.

@RobbieClarken

This comment has been minimized.

Copy link
Contributor Author

RobbieClarken commented Dec 6, 2019

The build failure looks to be a filesystem error related to rustup set default-host. I don't think it has anything to do with the changes in this PR:

failures:

---- env_override_beats_file_override stdout ----
Copying from /checkout/target/x86_64-unknown-linux-gnu/release/rustup-init to /checkout/target/x86_64-unknown-linux-gnu/tests/running-test-9VnOmv/rustup-exexL7Uwu/rustup
running "/checkout/target/x86_64-unknown-linux-gnu/tests/running-test-9VnOmv/rustup-exexL7Uwu/rustup" "set" "default-host" "x86_64-unknown-linux-gnu"
thread 'env_override_beats_file_override' panicked at 'failed to run test command: Os { code: 26, kind: Other, message: "Text file busy" }', src/libcore/result.rs:1165:5

failures:
    env_override_beats_file_override
src/toolchain.rs Outdated Show resolved Hide resolved
@kinnison

This comment has been minimized.

Copy link
Collaborator

kinnison commented Dec 6, 2019

The build failure looks to be a filesystem error related to rustup set default-host. I don't think it has anything to do with the changes in this PR:

Yep, that's a transient failure which only ever seems to manifest in CI, making it super-hard for me to track down.

@kinnison

This comment has been minimized.

Copy link
Collaborator

kinnison commented Dec 6, 2019

Assuming the CI passes, please rebase/squash ready for merge. Ensure your commit message is clean too, rebase sometimes splats them all together nastily. 👍

@RobbieClarken RobbieClarken force-pushed the RobbieClarken:show-from-version branch from 0a20521 to 48736f5 Dec 6, 2019
This change modifies the cli output of `rustup update` to display the
previous version of rustc for toolchains that are updated:

    $ rustup update
    --snip--
      stable-x86_64-apple-darwin unchanged - rustc 1.39.0 (4560ea788 2019-11-04)
       nightly-x86_64-apple-darwin updated - rustc 1.41.0-nightly (7afe6d9d1 2019-12-03) (from rustc 1.40.0-nightly (22bc9e1d9 2019-09-30))

Closes #2110.
@RobbieClarken RobbieClarken force-pushed the RobbieClarken:show-from-version branch from 48736f5 to 8a0b4ae Dec 6, 2019
@RobbieClarken

This comment has been minimized.

Copy link
Contributor Author

RobbieClarken commented Dec 6, 2019

Assuming the CI passes, please rebase/squash ready for merge. Ensure your commit message is clean too, rebase sometimes splats them all together nastily. 👍

Done and done.

Copy link
Collaborator

kinnison left a comment

👍

@kinnison kinnison merged commit 1249d1e into rust-lang:master Dec 6, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@RobbieClarken RobbieClarken deleted the RobbieClarken:show-from-version branch Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.