Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upPrint rustup version after update #614
Conversation
julienXX
referenced this pull request
Jul 25, 2016
Closed
Print new version while `rustup self update` #542
brson
reviewed
Jul 25, 2016
| @@ -1069,7 +1071,7 @@ pub fn update() -> Result<()> { | |||
| } | |||
| let setup_path = try!(prepare_update()); | |||
| if let Some(ref p) = setup_path { | |||
| info!("rustup updated successfully"); | |||
| info!("rustup updated successfully to {}", VERSION); | |||
This comment has been minimized.
This comment has been minimized.
brson
Jul 25, 2016
Contributor
By printing CARGO_PKG_VERSION here rustup will be printing the version of the currently running version of rustup. Instead, what we want is to print the version we just downloaded (here represented by setup_path). So this is instead going to need to run the executable at setup_path with --version and parse out the new version number.
This comment has been minimized.
This comment has been minimized.
|
Thanks @julienXX ! |
julienXX
force-pushed the
julienXX:print-new-version-after-update
branch
from
b35bb04
to
a23c403
Jul 26, 2016
This comment has been minimized.
This comment has been minimized.
|
@brson Oh okay, I should have thought about that. Thanks! |
brson
reviewed
Jul 26, 2016
| let re = Regex::new(r"\d+.\d+.\d+").unwrap(); | ||
| let capture = re.captures(&string_version).unwrap().at(0).unwrap(); | ||
| Ok(String::from(capture)) | ||
| } |
This comment has been minimized.
This comment has been minimized.
brson
Jul 26, 2016
Contributor
Since this code is on the super-important upgrade path, let's be a little more defensive. What happens if the new executable doesn't run? Seems like it could happen. What happens if it fails? What happens if the output of --version changes? Seems like we might want to do that (for example returning a version like "2.0.0-rc1"). Instead of unwraps here can they return errors instead? Then have the caller convert those errors to a placeholder string like "(unknown)".
An aside, if the executable fails to run that would be a bad sign - it might be worth thinking about turning that into a hard error and failing the upgrade. But that can be a different PR.
This comment has been minimized.
This comment has been minimized.
julienXX
Jul 26, 2016
Author
Contributor
Yes absolutely! I'll update the code to be more defensive. Do you feel like regexp is the way to go or could a string split be sufficient?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The test is good enough I think. Yeah, it could be better by having a different version number, but the effort to make that happen isn't worth it. |
julienXX
force-pushed the
julienXX:print-new-version-after-update
branch
from
a23c403
to
5ea4473
Aug 1, 2016
This comment has been minimized.
This comment has been minimized.
|
@brson I updated the code to be more defensive. Also it now exit the process if the executable fails to run. |
brson
merged commit a181b2b
into
rust-lang:master
Aug 1, 2016
This comment has been minimized.
This comment has been minimized.
|
Looks great. Thanks @julienXX ! |
julienXX commentedJul 25, 2016
•
edited
Fixes #542