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

rustbuilds rebuilds libstd even when it did not change [recent regression] #57142

Closed
RalfJung opened this issue Dec 27, 2018 · 5 comments · Fixed by #57336 or rust-lang/cargo#6493
Closed

rustbuilds rebuilds libstd even when it did not change [recent regression] #57142

RalfJung opened this issue Dec 27, 2018 · 5 comments · Fixed by #57336 or rust-lang/cargo#6493
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Dec 27, 2018 •
edited
Loading

For just a few weeks now, I am seeing many cases where rustbuild rebuilds stage 0 libstd (and hence rebuilds everything) even though nothing changed there. I have not yet tried to reproduce this in a clean dir (that would take forever due to all the rebuilding), but the steps are roughly as follows:

  • Start with a clean dir
  • ./x.py --stage 0 build src/librustc
  • ./x.py --stage 0 build src/librustc

The last step will rebuild everything even though nothing changed.

The timing coincides a little with libstd depending on crates io, but that could be mere coincidence.

Cc @alexcrichton
@RalfJung
Copy link
Member Author

Hm, I tried in a clean dir and cannot reproduce. I could have sworn I saw this multiple times. I'll reopen if I can reproduce later.

@petrochenkov petrochenkov reopened this Dec 27, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 27, 2018 •
edited
Loading

I see this too, it doesn't reproduce with clean build.
To reproduce you need to

  • Run ./x.py test --stage 1 src/test/ui --test-args something
  • Make some changes to libcore or liballoc (?) (not necessarily yourself, but for example by rebasing on master).
  • Run ./x.py test --stage 1 src/test/ui --test-args something again.
  • Run ./x.py test --stage 1 src/test/ui --test-args something again, which should be no-op, but it will actually start building almost everything starting with liballoc.

@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 27, 2018 •
edited
Loading

I have confirmed that it's indeed can be reproduced by touching libcore.

@petrochenkov petrochenkov added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Dec 27, 2018
@ehuss
Copy link
Contributor

ehuss commented Dec 27, 2018

This has been bugging me, too. I bisected the regression to #56092. cc @alexcrichton. I don't fully understand why, but for some reason Cargo thinks that one of liballoc's dependencies has changed, but I don't see any of them being different. I'll keep poking to better understand why the fingerprint check is failing.

The following is a relatively quick repro:
touch src/libcore/lib.rs ; ./x.py build src/libstd --stage=0 ; ./x.py build src/libstd --stage=0
The second build should be fresh, but it isn't.

@ehuss ehuss mentioned this issue Dec 28, 2018
Merged
@ehuss
Copy link
Contributor

ehuss commented Dec 28, 2018

I have posted a proposed fix for this.

@Centril Centril added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Dec 28, 2018
bors added a commit to rust-lang/cargo that referenced this issue Jan 2, 2019
@bors
Auto merge of #6493 - ehuss:fix-fingerprint-patch, r=alexcrichton
9bfaf2e 
Fix fingerprint calculation for patched deps.

If you have A→B→C where B and C are in a registry, and you `[patch]` C, the fingerprint calculation wasn't working correctly when C changes. The following sequence illustrates the problem:

1. Do a build from scratch.
2. Touch a file in C.
3. Build again. Everything rebuilds as expected.
4. Build again. You would expect this to be all fresh, but it rebuilds A.

The problem is the hash-busting doesn't propagate up to parents from dependencies. Normal targets normally aren't a problem because they have a `LocalFingerprint::MtimeBased` style local value which always recomputes the hash. However, registry dependencies have a `Precalculated` style local value which never recomputes the hash.

The solution here is to always recompute the hash. This shouldn't be too expensive, and is only done when writing the fingerprint, which should only happen when the target is dirty. I'm not entirely certain why the caching logic was added in #4125.

Fixes rust-lang/rust#57142
This was referenced Jan 2, 2019
Merged
Merged
bors added a commit that referenced this issue Jan 7, 2019
@bors
Auto merge of #57336 - ehuss:bump-stage0, r=alexcrichton
1f7c44c 
Bump stage0

Updates stage 0
From: rustc 1.32.0-beta.2 (a01e476 2018-12-08)
To:   rustc 1.32.0-beta.11 (e64fee6 2019-01-04)

Intended to pull in #57292 which will fix #57142.

The following is a list of PRs this also pulls in in case anyone is interested in seeing the changes:

#56930
#56961
#57236
#57305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bump stage0 ehuss/rust
Fix fingerprint calculation for patched deps. ehuss/cargo
4 participants
@ehuss @RalfJung @Centril @petrochenkov