Skip to content

Commit

Permalink
Don't use self.date unconditionally for program_out_of_date()
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jyn514 committed Dec 28, 2020
1 parent dc6121c commit 2f063cd
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 10 deletions.
10 changes: 5 additions & 5 deletions src/bootstrap/bootstrap.py
Expand Up @@ -391,7 +391,7 @@ def download_stage0(self):

if self.rustc().startswith(self.bin_root()) and \
(not os.path.exists(self.rustc()) or
self.program_out_of_date(self.rustc_stamp())):
self.program_out_of_date(self.rustc_stamp(), self.date)):
if os.path.exists(self.bin_root()):
shutil.rmtree(self.bin_root())
tarball_suffix = '.tar.xz' if support_xz() else '.tar.gz'
Expand Down Expand Up @@ -427,7 +427,7 @@ def download_stage0(self):
self.fix_bin_or_dylib("{}/bin/rustfmt".format(self.bin_root()))
self.fix_bin_or_dylib("{}/bin/cargo-fmt".format(self.bin_root()))
with output(self.rustfmt_stamp()) as rustfmt_stamp:
rustfmt_stamp.write(self.date + self.rustfmt_channel)
rustfmt_stamp.write(self.rustfmt_channel)

if self.downloading_llvm():
# We want the most recent LLVM submodule update to avoid downloading
Expand All @@ -454,7 +454,7 @@ def download_stage0(self):
for binary in ["llvm-config", "FileCheck"]:
self.fix_bin_or_dylib("{}/bin/{}".format(self.llvm_root(), binary))
with output(self.llvm_stamp()) as llvm_stamp:
llvm_stamp.write(self.date + llvm_sha + str(llvm_assertions))
llvm_stamp.write(llvm_sha + str(llvm_assertions))

def downloading_llvm(self):
opt = self.get_toml('download-ci-llvm', 'llvm')
Expand Down Expand Up @@ -616,12 +616,12 @@ def llvm_stamp(self):
return os.path.join(self.llvm_root(), '.llvm-stamp')


def program_out_of_date(self, stamp_path, extra=""):
def program_out_of_date(self, stamp_path, key):
"""Check if the given program stamp is out of date"""
if not os.path.exists(stamp_path) or self.clean:
return True
with open(stamp_path, 'r') as stamp:
return (self.date + extra) != stamp.read()
return key != stamp.read()

def bin_root(self):
"""Return the binary root directory
Expand Down
11 changes: 6 additions & 5 deletions src/bootstrap/bootstrap_test.py
Expand Up @@ -70,6 +70,7 @@ def setUp(self):
self.build.build_dir = self.container
self.rustc_stamp_path = os.path.join(self.container, "stage0",
".rustc-stamp")
self.key = self.build.date + str(None)

def tearDown(self):
rmtree(self.container)
Expand All @@ -78,19 +79,19 @@ def test_stamp_path_does_not_exists(self):
"""Return True when the stamp file does not exists"""
if os.path.exists(self.rustc_stamp_path):
os.unlink(self.rustc_stamp_path)
self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path))
self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path, self.key))

def test_dates_are_different(self):
"""Return True when the dates are different"""
with open(self.rustc_stamp_path, "w") as rustc_stamp:
rustc_stamp.write("2017-06-14")
self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path))
rustc_stamp.write("2017-06-14None")
self.assertTrue(self.build.program_out_of_date(self.rustc_stamp_path, self.key))

def test_same_dates(self):
"""Return False both dates match"""
with open(self.rustc_stamp_path, "w") as rustc_stamp:
rustc_stamp.write("2017-06-15")
self.assertFalse(self.build.program_out_of_date(self.rustc_stamp_path))
rustc_stamp.write("2017-06-15None")
self.assertFalse(self.build.program_out_of_date(self.rustc_stamp_path, self.key))


if __name__ == '__main__':
Expand Down

0 comments on commit 2f063cd

Please sign in to comment.