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

Use the same filename hash for pre-release channels. #8073

Merged
merged 2 commits into from
Apr 9, 2020

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Apr 5, 2020

This changes it so that filenames do not hash the entire verbose version from rustc if they are a pre-release version. The intent is to avoid leaving stale artifacts in the target directory whenever someone updates a nightly or beta release. This should help reduce disk space usage for someone who updates these toolchains frequently.

I tested with the rustc repo, and it seems to be OK. It keeps everything in separate target directories, so I think it should be generally safe. This should only affect someone switching between different nightlies and wanting to avoid recompiling when switching back. I suspect that is a rare use case, though if there are complaints this can be easily reverted (or made a config option). cargo-bisect-rustc should also be safe since it uses a different target directory for each toolchain.

One concern here for me was incremental support. It looks like (src) the incremental cache includes the detailed rustc version, so I think that is safe. It also looks like it is smart enough to delete stale files.

We will need to be more careful in the future when changing the target directory structure or the format of files. We previously relied on the fact that each new nightly will use different filenames. If we change the structure in a backwards-incompatible way, we will need to be careful to update the version (1.hash in compute_metadata).

@rust-highfive
Copy link

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 Apr 5, 2020
@eddyb
Copy link
Member

eddyb commented Apr 5, 2020

cc @Mark-Simulacrum This may just fix rust-lang/rust#69060 wholesale.
(For everyone else: see rust-lang/rust#69060 (comment) for my idea for how to cause Cargo to use the same -C metadata for different compilers, which is the effect this PR has by default)

@RalfJung
Copy link
Member

RalfJung commented Apr 5, 2020

So when I switch back and forth between a nightly and something installed via rustup-toolchain-install-master, this will do a full rebuild on each switch?
(I do that very rarely, so this is probably okay, I just want to understand what is likely the consequence here.)

@ehuss
Copy link
Contributor Author

ehuss commented Apr 6, 2020

Yes, it'll cause a rebuild. Let me know if you think that will be too annoying. One workaround is to use something like CARGO_TARGET_DIR=target2 cargo +master build.

@RalfJung
Copy link
Member

RalfJung commented Apr 6, 2020

No I think it's okay I think, I don't swap back and forth in the same checkout too often.

@alexcrichton
Copy link
Member

Seems reasonable to me implementation and feature-wise, thanks for this @ehuss! (and nice test too btw)

I'll leave this for a bit more to see if others have thoughts on it but otherwise r=me at any time

@ehuss
Copy link
Contributor Author

ehuss commented Apr 8, 2020

I added a -Z flag to disable the new behavior in case it is a problem for people. Kinda awkward to use, but I hope it won't be too necessary.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Apr 9, 2020

📌 Commit fdfdb3d 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 Apr 9, 2020
@bors
Copy link
Collaborator

bors commented Apr 9, 2020

⌛ Testing commit fdfdb3d with merge da54d6b...

@bors
Copy link
Collaborator

bors commented Apr 9, 2020

☀️ Test successful - checks-azure
Approved by: alexcrichton
Pushing da54d6b to master...

@bors bors merged commit da54d6b into rust-lang:master Apr 9, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2020
Update cargo

12 commits in 390e8f245ef2cd7ac698b8a76abf029f9abcab0d..74e3a7d5b756d7c0e94399fc29fcd154e792c22a
2020-04-07 17:46:45 +0000 to 2020-04-13 20:41:52 +0000
- Update dependencies to support illumos target (rust-lang/cargo#8093)
- Whitelist another known spurious curl error (rust-lang/cargo#8102)
- Fix nightly test matching rustc "warning" output. (rust-lang/cargo#8098)
- Update default for codegen-units. (rust-lang/cargo#8096)
- Fix freshness when linking is interrupted. (rust-lang/cargo#8087)
- Add `cargo tree` command. (rust-lang/cargo#8062)
- Add "build-finished" JSON message. (rust-lang/cargo#8069)
- Extend -Zpackage-features with more capabilities. (rust-lang/cargo#8074)
- Disallow invalid dependency names through crate renaming (rust-lang/cargo#8090)
- Use the same filename hash for pre-release channels. (rust-lang/cargo#8073)
- Index the commands section (rust-lang/cargo#8081)
- Upgrade to mdBook v0.3.7 (rust-lang/cargo#8083)
Mark-Simulacrum added a commit to Mark-Simulacrum/rustc-perf that referenced this pull request Jul 19, 2020
rust-lang/cargo#8073 has long since hit beta, so we no
longer need this step in perf. It's also true that rust-src is currently
incomplete so we can't build std in perf regardless.
@ehuss ehuss added this to the 1.44.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants