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

Introduce unified manifest file #1465

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Introduce unified manifest file #1465

merged 3 commits into from
Oct 23, 2024

Conversation

fasterthanlime
Copy link
Contributor

@fasterthanlime fasterthanlime commented Oct 17, 2024

Closes #1321

This writes out unified checksum files like sha256.sum or sha512.sum (which are compatible with GNU-style sha256sum -c sha256.sum invocations) depending on which checksum style is chosen from the dist config.

It's a new global artifact which is built after all the local artifacts, re-using the checksums computed earlier. The plan is to eventually stop publishing individual checksum files like some-binary.sha256 and only publish the unified checksum file, which will reduce noise / make publishing faster, etc.

@fasterthanlime fasterthanlime marked this pull request as ready for review October 18, 2024 16:39
Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I popped this into BSD sha256sum, and confirmed it was happy using this as a check file.

I wonder if we might want to consider adding the hash file to the snapshots, so we can confirm the format stays stable? Not that I expect us to change code that modifies it often, but it feels roughly as snapshottable as the shell installers.

cargo-dist/src/tasks.rs Outdated Show resolved Hide resolved
@fasterthanlime
Copy link
Contributor Author

I wonder if we might want to consider adding the hash file to the snapshots, so we can confirm the format stays stable?

Just did that, which taught me about the snapshotting system for tests.

@mistydemeo
Copy link
Contributor

Hm, I guess the NPM package hash isn't actually stable, is it?

If you're interested in learning about snapshot filtering, we could use that here to remove the unstable hash from the snapshots. Or we could punt on snapshotting the file contents at this point.

@fasterthanlime
Copy link
Contributor Author

Just noting here that @ashleygwilliams asked me to document why the npm packages aren't stable — which I will do ~around merging this PR, because I'll probably just end up skipping/censoring the checksums from the snapshots for the time being.

@fasterthanlime
Copy link
Contributor Author

CleanShot 2024-10-23 at 16 42 46@2x

Okay I guess none of our tars are stable right now so.. I'll just remove the unified manifest file from the snapshots for the time being? or just censor everything, but that's silly

@mistydemeo
Copy link
Contributor

Right, I forget it doesn't hash the text installers, which are the things that would be stable. Yes, fine to drop it at this point.

@fasterthanlime fasterthanlime merged commit d6c9e11 into main Oct 23, 2024
17 checks passed
@fasterthanlime fasterthanlime deleted the unified-manifest branch October 23, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add unified checksum file
2 participants