Add sha256 checksum to rake build:checksum #6022
Conversation
|
Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality. We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below. If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack. For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide |
… SHA512 - Until now Rubygems.org has publicly displayed SHA256 checksum for published gems, but has only created the SHA512 checksum for the package via rake build:checksum task - This will allow more gem authors to easily verify integrity of published gems with an existing Rubygems.org feature
previously inconsistent platform discrepancies are normalized to include the following for all: x86_64-darwin-19 x86_64-darwin-20 x86_64-darwin-21
pboling
force-pushed
the
add-SHA512-checksum
branch
from
690ec12 to
06f312d
Compare
pboling
force-pushed
the
add-SHA512-checksum
branch
from
06f312d to
1c707cc
Compare
|
I am working on a PR updating rubygems.org documentation to be concordant with |
There was a problem hiding this comment.
Thanks for fixing this, @pboling! I just have one request regarding lazily loading digest/sha, so that it's only loaded when this task is used.
- address code review feedback
| require "digest/sha2" | ||
| checksum = ::Digest::SHA512.file(built_gem_path).hexdigest | ||
| CHECKSUMS.each do |extension, digest_klass| |
There was a problem hiding this comment.
Super minor but I'm thinking, do you think the CHECKSUMS constant buys us much, over
write_checksum(built_gem_path, "sha256", Digest::SHA256)
write_checksum(built_gem_path, "sha512", Digest::SHA512)There was a problem hiding this comment.
It doesn't get us much, agreed.
| built_gem_path ||= build_gem | ||
| SharedHelpers.filesystem_access(File.join(base, "checksums")) {|p| FileUtils.mkdir_p(p) } |
There was a problem hiding this comment.
Why was this moved inside the write_checksums method? It should be enough to run it just once I think?
There was a problem hiding this comment.
Good point. Oversight.
… in addition to the existing SHA512
What was the end-user or developer problem that led to this PR?
SHA256checksum for published gems, but has only created theSHA512checksum for the package viarake build:checksumtaskWhat is your fix for the problem, implemented in this PR?
Create a SHA256 checksum, in addition to the current SHA512 checksum.
Note that I've changed the Gem helper method from
build_checksumtobuild_checksums. Certainly could keep it the same if that's a problem, but it felt good to be grammatically correct with it. Not sure if internal or external API.Make sure the following tasks are checked