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

Add checksums cache to build-manifest #78409

Merged
merged 2 commits into from
Oct 28, 2020
Merged

Add checksums cache to build-manifest #78409

merged 2 commits into from
Oct 28, 2020

Conversation

pietroalbini
Copy link
Member

@pietroalbini pietroalbini commented Oct 26, 2020 •
edited
Loading

During the release process we're currently calculating the SHA256 of each file three times:

  1. In build-manifest, to fill the hash = "f00" keys of the manifests.
  2. In promote-release, to generate the .sha256 files.
  3. In promote-release, to generate the .asc GPG signatures.

Calculations 1. and 2. could be merged into a single one if there was a way for build-manifest to pass the checksums it generated over to promote-release. Unfortunately calculation 3. can't be merged as GPG requires extra metadata to be hashed.

This PR adds support for merging 1. and 2. by creating the BUILD_MANIFEST_CHECKSUM_CACHE environment variable, which points to a JSON file storing a cache of all the calculated checksums. build-manifest will load it at startup and avoid generating existing checksums, and it will dump its internal checksums cache into it when it exits successfully.

This PR also allows to run build-manifest multiple times without the need to wait for checksums to be calculated in the following invocations. The speedup will allow to work torwards a fix for rust-lang/promote-release#15 without impacting the release process duration nor our storage costs.

This PR can be reviewed commit-by-commit.
r? @Mark-Simulacrum

@pietroalbini
build-manifest: refactor checksum generation into a struct
0cf3ce4
@pietroalbini
build-manifest: add BUILD_MANIFEST_CHECKSUM_CACHE
c2f4bbd 
The checksum cache allows to reuse the calculated checksums between
build-manifest and promote-release, or between multiple invocations of
build-manifest.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2020
@Mark-Simulacrum
Copy link
Member

Have we considered signing the hash files instead of the originals? That would eliminate 3 as well, or essentially so. I seem to recall some discussion around this but I forget the conclusions...

r=me regardless.

@pietroalbini
Copy link
Member Author

@bors r=Mark-Simulacrum

Have we considered signing the hash files instead of the originals? That would eliminate 3 as well, or essentially so. I seem to recall some discussion around this but I forget the conclusions...

I think if we want to reduce the amount of signatures we create we should just generate SHA256SUMS-like files and only sign them.

@bors
Copy link
Contributor

bors commented Oct 27, 2020

📌 Commit c2f4bbd 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 Oct 27, 2020
@jonas-schievink jonas-schievink mentioned this pull request Oct 27, 2020
Closed
@bors
Copy link
Contributor

bors commented Oct 28, 2020

⌛ Testing commit c2f4bbd with merge a65ddbaff75bba983cb612b2c4ec5dab6c0e54e6...

@bors
Copy link
Contributor

bors commented Oct 28, 2020

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 28, 2020
@pietroalbini
Copy link
Member Author

@bors retry

@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 Oct 28, 2020
@bors
Copy link
Contributor

bors commented Oct 28, 2020

⌛ Testing commit c2f4bbd with merge 717eb6c...

@bors
Copy link
Contributor

bors commented Oct 28, 2020

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 28, 2020
@bors bors merged commit 717eb6c into rust-lang:master Oct 28, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 28, 2020
@pietroalbini pietroalbini deleted the build-manifest-checksum-cache branch October 28, 2020 17:41
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
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.
Projects
None yet
Milestone
1.49.0
Development

Successfully merging this pull request may close these issues.
5 participants
@pietroalbini @Mark-Simulacrum @bors @rust-highfive @rustbot