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

checksum: simplify, use only sha256 #10247

Merged
merged 1 commit into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Library/Homebrew/cask/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def sha256(arg = nil)
when :no_check
arg
when String
Checksum.new(:sha256, arg)
Checksum.new(arg)
else
raise CaskInvalidError.new(cask, "invalid 'sha256' value: '#{arg.inspect}'")
end
Expand Down
9 changes: 3 additions & 6 deletions Library/Homebrew/checksum.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,9 @@
class Checksum
extend Forwardable

attr_reader :hash_type, :hexdigest
attr_reader :hexdigest

TYPES = [:sha256].freeze

def initialize(hash_type, hexdigest)
@hash_type = hash_type
def initialize(hexdigest)
@hexdigest = hexdigest.downcase
end

Expand All @@ -23,7 +20,7 @@ def ==(other)
when String
to_s == other.downcase
when Checksum
hash_type == other.hash_type && hexdigest == other.hexdigest
hexdigest == other.hexdigest
else
false
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/cleanup.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def stale_formula?(scrub)

if resource_name == "patch"
patch_hashes = formula.stable&.patches&.select(&:external?)&.map(&:resource)&.map(&:version)
return true unless patch_hashes&.include?(Checksum.new(:sha256, version.to_s))
return true unless patch_hashes&.include?(Checksum.new(version.to_s))
elsif resource_name && resource_version = formula.stable&.resources&.dig(resource_name)&.version
return true if resource_version != version
elsif version.is_a?(PkgVersion)
Expand Down
10 changes: 5 additions & 5 deletions Library/Homebrew/cmd/fetch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,28 +132,28 @@ def fetch_resource(r, args:)
fetch_fetchable r, args: args
rescue ChecksumMismatchError => e
retry if retry_fetch?(r, args: args)
opoo "Resource #{r.name} reports different #{e.hash_type}: #{e.expected}"
opoo "Resource #{r.name} reports different sha256: #{e.expected}"
end

def fetch_formula(f, args:)
fetch_fetchable f, args: args
rescue ChecksumMismatchError => e
retry if retry_fetch?(f, args: args)
opoo "Formula reports different #{e.hash_type}: #{e.expected}"
opoo "Formula reports different sha256: #{e.expected}"
end

def fetch_cask(cask_download, args:)
fetch_fetchable cask_download, args: args
rescue ChecksumMismatchError => e
retry if retry_fetch?(cask_download, args: args)
opoo "Cask reports different #{e.hash_type}: #{e.expected}"
opoo "Cask reports different sha256: #{e.expected}"
end

def fetch_patch(p, args:)
fetch_fetchable p, args: args
rescue ChecksumMismatchError => e
Homebrew.failed = true
opoo "Patch reports different #{e.hash_type}: #{e.expected}"
opoo "Patch reports different sha256: #{e.expected}"
end

def retry_fetch?(f, args:)
Expand Down Expand Up @@ -183,7 +183,7 @@ def fetch_fetchable(f, args:)
return unless download.file?

puts "Downloaded to: #{download}" unless already_fetched
puts Checksum::TYPES.map { |t| "#{t.to_s.upcase}: #{download.send(t)}" }
puts "SHA256: #{download.sha256}"

f.verify_download_integrity(download)
end
Expand Down
9 changes: 3 additions & 6 deletions Library/Homebrew/dev-cmd/bump-formula-pr.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,8 @@ def bump_formula_pr

check_for_mirrors(formula, old_mirrors, new_mirrors, args: args) if new_url.present?

hash_type, old_hash = if (checksum = formula_spec.checksum)
[checksum.hash_type, checksum.hexdigest]
end

new_hash = args[hash_type] if hash_type.present?
old_hash = formula_spec.checksum&.hexdigest
new_hash = args.sha256
new_tag = args.tag
new_revision = args.revision
old_url = formula_spec.url
Expand All @@ -180,7 +177,7 @@ def bump_formula_pr
elsif new_tag && new_revision
check_closed_pull_requests(formula, tap_full_name, url: old_url, tag: new_tag, args: args) if new_version.blank?
false
elsif hash_type.blank?
elsif old_hash.blank?
if new_tag.blank? && new_version.blank? && new_revision.blank?
raise UsageError, "#{formula}: no --tag= or --version= argument specified!"
end
Expand Down
5 changes: 2 additions & 3 deletions Library/Homebrew/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -626,14 +626,13 @@ class ChecksumMissingError < ArgumentError; end

# Raised by {Pathname#verify_checksum} when verification fails.
class ChecksumMismatchError < RuntimeError
attr_reader :expected, :hash_type
attr_reader :expected

def initialize(path, expected, actual)
@expected = expected
@hash_type = expected.hash_type.to_s.upcase

super <<~EOS
#{@hash_type} mismatch
SHA256 mismatch
Expected: #{Formatter.success(expected.to_s)}
Actual: #{Formatter.error(actual.to_s)}
File: #{path}
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/pathname.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ def sha256
def verify_checksum(expected)
raise ChecksumMissingError if expected.blank?

actual = Checksum.new(expected.hash_type, send(expected.hash_type).downcase)
actual = Checksum.new(sha256.downcase)
raise ChecksumMismatchError.new(self, expected, actual) unless expected == actual
end

Expand Down
8 changes: 4 additions & 4 deletions Library/Homebrew/formula.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1823,8 +1823,8 @@ def to_hash
bottle_url = "#{bottle_spec.root_url}/#{Bottle::Filename.create(self, os, bottle_spec.rebuild).bintray}"
checksum = bottle_spec.collector[os]
bottle_info["files"][os] = {
"url" => bottle_url,
checksum.hash_type.to_s => checksum.hexdigest,
"url" => bottle_url,
"sha256" => checksum.hexdigest,
}
end
hsh["bottle"]["stable"] = bottle_info
Expand Down Expand Up @@ -2418,8 +2418,8 @@ def mirror(val)
# tell you the currently valid value.
#
# <pre>sha256 "2a2ba417eebaadcb4418ee7b12fe2998f26d6e6f7fda7983412ff66a741ab6f7"</pre>
Checksum::TYPES.each do |type|
define_method(type) { |val| stable.send(type, val) }
def sha256(val)
stable.sha256(val)
end

# @!attribute [w] bottle
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ def verify_download_integrity(fn)
EOS
end

Checksum::TYPES.each do |type|
define_method(type) { |val| @checksum = Checksum.new(type, val) }
def sha256(val)
@checksum = Checksum.new(val)
end

def url(val = nil, **specs)
Expand Down
17 changes: 7 additions & 10 deletions Library/Homebrew/software_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class SoftwareSpec
:cached_download, :clear_cache, :checksum, :mirrors, :specs, :using, :version, :mirror,
:downloader

def_delegators :@resource, *Checksum::TYPES
def_delegators :@resource, :sha256

def initialize(flags: [])
@resource = Resource.new
Expand Down Expand Up @@ -411,11 +411,9 @@ def tag?(tag)

# Checksum methods in the DSL's bottle block optionally take
# a Hash, which indicates the platform the checksum applies on.
Checksum::TYPES.each do |cksum|
define_method(cksum) do |val|
digest, tag = val.shift
collector[tag] = Checksum.new(cksum, digest)
end
def sha256(val)
digest, tag = val.shift
collector[tag] = Checksum.new(digest)
end

def checksum_for(tag)
Expand All @@ -429,13 +427,12 @@ def checksums
# Sort non-MacOS tags below MacOS tags.
"0.#{tag}"
end
checksums = {}
sha256s = []
tags.reverse_each do |tag|
checksum = collector[tag]
checksums[checksum.hash_type] ||= []
checksums[checksum.hash_type] << { checksum => tag }
sha256s << { checksum => tag }
end
checksums
{ sha256: sha256s }
end
end

Expand Down
6 changes: 3 additions & 3 deletions Library/Homebrew/test/cask/download_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module Cask
end

context "when expected and computed checksums match" do
let(:expected_sha256) { Checksum.new(:sha256, cafebabe) }
let(:expected_sha256) { Checksum.new(cafebabe) }

it "does not raise an error" do
expect { verification }.not_to raise_error
Expand All @@ -41,15 +41,15 @@ module Cask
end

context "when the expected checksum is empty" do
let(:expected_sha256) { Checksum.new(:sha256, "") }
let(:expected_sha256) { Checksum.new("") }

it "outputs an error" do
expect { verification }.to output(/sha256 "#{computed_sha256}"/).to_stderr
end
end

context "when expected and computed checksums do not match" do
let(:expected_sha256) { Checksum.new(:sha256, deadbeef) }
let(:expected_sha256) { Checksum.new(deadbeef) }

it "raises an error" do
expect { verification }.to raise_error ChecksumMismatchError
Expand Down
10 changes: 4 additions & 6 deletions Library/Homebrew/test/checksum_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,19 @@

describe Checksum do
describe "#empty?" do
subject { described_class.new(:sha256, "") }
subject { described_class.new("") }

it { is_expected.to be_empty }
end

describe "#==" do
subject { described_class.new(:sha256, TEST_SHA256) }
subject { described_class.new(TEST_SHA256) }

let(:other) { described_class.new(:sha256, TEST_SHA256) }
let(:other_reversed) { described_class.new(:sha256, TEST_SHA256.reverse) }
let(:other_sha1) { described_class.new(:sha1, TEST_SHA1) }
let(:other) { described_class.new(TEST_SHA256) }
let(:other_reversed) { described_class.new(TEST_SHA256.reverse) }

it { is_expected.to eq(other) }
it { is_expected.not_to eq(other_reversed) }
it { is_expected.not_to eq(other_sha1) }
it { is_expected.not_to eq(nil) }
end
end
2 changes: 1 addition & 1 deletion Library/Homebrew/test/resource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@

it "returns the checksum set with #sha256" do
subject.sha256(TEST_SHA256)
expect(subject.checksum).to eq(Checksum.new(:sha256, TEST_SHA256))
expect(subject.checksum).to eq(Checksum.new(TEST_SHA256))
end
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/software_spec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@
checksums.each_pair do |cat, digest|
subject.sha256(digest => cat)
checksum, = subject.checksum_for(cat)
expect(Checksum.new(:sha256, digest)).to eq(checksum)
expect(Checksum.new(digest)).to eq(checksum)
end
end

Expand Down