Skip to content

Don't use self.date unconditionally for program_out_of_date() #80426

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

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 28, 2020

This avoids unnecessary cache invalidations for programs not affected by
the stage0 version (which is everything except the stage0 compiler
itself).

The redundant invalidations weren't noticed until now because they only
showed up on stage0 bumps, at which point people are used to rebuilding
everything anyway. I noticed it in #79540
because I wasn't adding self.date to the stamp file (because I didn't realize it was necessary). Rather than
adding self.date I thought it was better to remove it from the cache key.

@jyn514 jyn514 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Dec 28, 2020
@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(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 Dec 28, 2020
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-9 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
ERROR: test_dates_are_different (__main__.ProgramOutOfDate)
Return True when the dates are different
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/checkout/src/bootstrap/bootstrap_test.py", line 87, in test_dates_are_different
    self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path))
TypeError: program_out_of_date() missing 1 required positional argument: 'key'
======================================================================
ERROR: test_same_dates (__main__.ProgramOutOfDate)
Return False both dates match
----------------------------------------------------------------------
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/checkout/src/bootstrap/bootstrap_test.py", line 93, in test_same_dates
    self.assertFalse(self.build.program_out_of_date(self.rustc_stamp_path))
TypeError: program_out_of_date() missing 1 required positional argument: 'key'
======================================================================
ERROR: test_stamp_path_does_not_exists (__main__.ProgramOutOfDate)
Return True when the stamp file does not exists
----------------------------------------------------------------------
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/checkout/src/bootstrap/bootstrap_test.py", line 81, in test_stamp_path_does_not_exists
    self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path))
TypeError: program_out_of_date() missing 1 required positional argument: 'key'
----------------------------------------------------------------------
Ran 16 tests in 0.006s

FAILED (errors=3)
FAILED (errors=3)
make: *** [check-bootstrap] Error 1
Makefile:49: recipe for target 'check-bootstrap' failed

This avoids unnecessary cache invalidations for programs not affected by
the stage0 version (which is everything except the stage0 compiler
itself).

The redundant invalidations weren't noticed until now because they only
showed up on stage0 bumps, at which point people are used to rebuilding
everything anyway. I noticed it because I wasn't adding `self.date` to
the stamp file (because I didn't realize it was necessary). Rather than
adding self.date I thought it was better to remove it from the cache
key.
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 4, 2021

📌 Commit 2f063cd has been approved by Mark-Simulacrum

@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 Jan 4, 2021
@bors
Copy link
Collaborator

bors commented Jan 5, 2021

⌛ Testing commit 2f063cd with merge 9919ad6...

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 9919ad6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 5, 2021
@bors bors merged commit 9919ad6 into rust-lang:master Jan 5, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 5, 2021
@jyn514 jyn514 deleted the bootstrap-caching branch January 5, 2021 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants