Skip to content

Minor improvements to bootstrap #52729

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

Closed
wants to merge 1 commit into from

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Jul 26, 2018

  • prefer Path-specific methods to String ones
  • don't add file extensions if they are removed right afterwards

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 26, 2018
@alexcrichton
Copy link
Member

@bors: r+ rollup

Thanks!

@bors
Copy link
Collaborator

bors commented Jul 26, 2018

📌 Commit db303c1 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jul 27, 2018
…hton

Minor improvements to bootstrap

- prefer `Path`-specific methods to `String` ones
- don't add file extensions if they are removed right afterwards
@kennytm
Copy link
Member

kennytm commented Jul 28, 2018

@bors r-

distcheck failed with

[00:11:29] tar (child): /checkout/obj/build/dist/rustc-1.29.tar.gz: Cannot open: No such file or directory
[00:11:29] tar (child): Error is not recoverable: exiting now
[00:11:29] tar: Child returned status 2

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 28, 2018
@ljedrz
Copy link
Contributor Author

ljedrz commented Jul 28, 2018

I investigated this and it appears that adding extensions with the PathBuf API might result in some weird results; this is totally expected:

let mut p1 = PathBuf::from(format!("{}.tar.gz", "rustc-1.29.0-dev-src"));
p1.set_extension(""); // removes .gz
p1.set_extension(""); // removes .tar

But this... less so:

let mut p2 = PathBuf::from("rustc-1.29.0-dev-src").with_extension("tar.gz");
p2.set_extension(""); // removes .gz
p2.set_extension(""); // removes .0-dev-src.tar !

In the second case I would either expect set_extension("") to first remove .gz and then .tar or remove .tar.gz with a single call, but I definitely didn't expect it to remove .gz and then the whole .0-dev-src.tar bit... Or am I missing something?

@kennytm
Copy link
Member

kennytm commented Jul 28, 2018

@ljedrz Path::with_extension replaces the original extension:

use std::path::{Path, PathBuf};

let path = Path::new("foo.rs");
assert_eq!(path.with_extension("txt"), PathBuf::from("foo.txt"));

Therefore PathBuf::from("rustc-1.29.0-dev-src").with_extension("tar.gz") returned rustc-1.29.tar.gz.

@ljedrz
Copy link
Contributor Author

ljedrz commented Jul 28, 2018

In that case it seems too risky to use it in case the path might contain any dots; the added value isn't too high, as it's just the bootstrap; I'll close the PR, then.

@ljedrz ljedrz closed this Jul 28, 2018
@ljedrz ljedrz deleted the bootstrap_tweaks branch July 31, 2018 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants