diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 1af1cf4f68..a5cf751af2 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -350,9 +350,9 @@ module Homebrew tab_json = Utils::Bottles.file_from_bottle(bottle_path, tab_path) tab = Tab.from_file_content(tab_json, tab_path) - _, _, bottle_cellar = Formula[f.name].bottle_specification.checksum_for(bottle_tag, no_older_versions: true) - relocatable = [:any, :any_skip_relocation].include?(bottle_cellar) - skip_relocation = bottle_cellar == :any_skip_relocation + tag_spec = Formula[f.name].bottle_specification.tag_specification_for(bottle_tag, no_older_versions: true) + relocatable = [:any, :any_skip_relocation].include?(tag_spec.cellar) + skip_relocation = tag_spec.cellar == :any_skip_relocation prefix = bottle_tag.default_prefix cellar = bottle_tag.default_cellar @@ -506,7 +506,7 @@ module Homebrew old_spec = f.bottle_specification if args.keep_old? && !old_spec.checksums.empty? - mismatches = [:root_url, :prefix, :rebuild].reject do |key| + mismatches = [:root_url, :rebuild].reject do |key| old_spec.send(key) == bottle.send(key) end unless mismatches.empty? @@ -551,7 +551,7 @@ module Homebrew }, "bottle" => { "root_url" => bottle.root_url, - "prefix" => bottle.prefix, + "prefix" => prefix.to_s, # TODO: 3.3.0: deprecate this "cellar" => bottle_cellar.to_s, "rebuild" => bottle.rebuild, "date" => Pathname(filename.to_s).mtime.strftime("%F"), @@ -640,16 +640,16 @@ module Homebrew bottle_hash["formula"]["pkg_version"] == formula.pkg_version.to_s && bottle.rebuild != old_bottle_spec.rebuild && bottle.root_url == old_bottle_spec.root_url - bottle.collector.keys.all? do |tag| - bottle_collector_tag = bottle.collector[tag] - next false if bottle_collector_tag.blank? + bottle.collector.tags.all? do |tag| + tag_spec = bottle.collector.specification_for(tag) + next false if tag_spec.blank? - old_bottle_spec_collector_tag = old_bottle_spec.collector[tag] - next false if old_bottle_spec_collector_tag.blank? + old_tag_spec = old_bottle_spec.collector.specification_for(tag) + next false if old_tag_spec.blank? - next false if bottle_collector_tag[:cellar] != old_bottle_spec_collector_tag[:cellar] + next false if tag_spec.cellar != old_tag_spec.cellar - bottle_collector_tag[:checksum].hexdigest == old_bottle_spec_collector_tag[:checksum].hexdigest + tag_spec.checksum.hexdigest == old_tag_spec.checksum.hexdigest end end @@ -744,7 +744,6 @@ module Homebrew new_values = { root_url: new_bottle_hash["root_url"], - prefix: new_bottle_hash["prefix"], rebuild: new_bottle_hash["rebuild"], } @@ -762,17 +761,17 @@ module Homebrew return [mismatches, checksums] if old_keys.exclude? :sha256 - old_bottle_spec.collector.each_key do |tag| - old_checksum_hash = old_bottle_spec.collector[tag] - old_hexdigest = old_checksum_hash[:checksum].hexdigest - old_cellar = old_checksum_hash[:cellar] + old_bottle_spec.collector.each_tag do |tag| + old_tag_spec = old_bottle_spec.collector.specification_for(tag) + old_hexdigest = old_tag_spec.checksum.hexdigest + old_cellar = old_tag_spec.cellar new_value = new_bottle_hash.dig("tags", tag.to_s) if new_value.present? && new_value["sha256"] != old_hexdigest mismatches << "sha256 #{tag}: old: #{old_hexdigest.inspect}, new: #{new_value["sha256"].inspect}" elsif new_value.present? && new_value["cellar"] != old_cellar.to_s mismatches << "cellar #{tag}: old: #{old_cellar.to_s.inspect}, new: #{new_value["cellar"].inspect}" else - checksums << { cellar: old_cellar, tag => old_hexdigest } + checksums << { cellar: old_cellar, tag.to_sym => old_hexdigest } end end diff --git a/Library/Homebrew/extend/os/linux/software_spec.rb b/Library/Homebrew/extend/os/linux/software_spec.rb index 6240666cf1..7f5b4d2890 100644 --- a/Library/Homebrew/extend/os/linux/software_spec.rb +++ b/Library/Homebrew/extend/os/linux/software_spec.rb @@ -3,8 +3,9 @@ class BottleSpecification extend T::Sig - sig { returns(T::Boolean) } - def skip_relocation? + + sig { params(tag: Utils::Bottles::Tag).returns(T::Boolean) } + def skip_relocation?(tag: Utils::Bottles.tag) false end end diff --git a/Library/Homebrew/extend/os/mac/utils/bottles.rb b/Library/Homebrew/extend/os/mac/utils/bottles.rb index 6701e886e4..7d601f14f4 100644 --- a/Library/Homebrew/extend/os/mac/utils/bottles.rb +++ b/Library/Homebrew/extend/os/mac/utils/bottles.rb @@ -41,11 +41,10 @@ module Utils return if tag_version.blank? - keys.find do |key| - key_tag = Tag.from_symbol(key) - next if key_tag.arch != tag.arch + tags.find do |candidate| + next if candidate.arch != tag.arch - key_tag.to_macos_version <= tag_version + candidate.to_macos_version <= tag_version rescue MacOSVersionError false end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index e4d3929b99..f977ea62dc 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2034,17 +2034,17 @@ class Formula "root_url" => bottle_spec.root_url, "files" => {}, } - bottle_spec.collector.each_key do |os| - collector_os = bottle_spec.collector[os] - os_cellar = collector_os[:cellar] + bottle_spec.collector.each_tag do |tag| + tag_spec = bottle_spec.collector.specification_for(tag) + os_cellar = tag_spec.cellar os_cellar = os_cellar.inspect if os_cellar.is_a?(Symbol) - checksum = collector_os[:checksum].hexdigest - filename = Bottle::Filename.create(self, os, bottle_spec.rebuild) + checksum = tag_spec.checksum.hexdigest + filename = Bottle::Filename.create(self, tag, bottle_spec.rebuild) path, = Utils::Bottles.path_resolved_basename(bottle_spec.root_url, name, checksum, filename) url = "#{bottle_spec.root_url}/#{path}" - hash["files"][os] = { + hash["files"][tag.to_sym] = { "cellar" => os_cellar, "url" => url, "sha256" => checksum, diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb index 7a7fc8b8ef..61fac3b8a7 100644 --- a/Library/Homebrew/formula_installer.rb +++ b/Library/Homebrew/formula_installer.rb @@ -150,13 +150,14 @@ class FormulaInstaller return false end - bottle = formula.bottle_specification - unless bottle.compatible_locations? + bottle = formula.bottle + if bottle && !bottle.compatible_locations? if output_warning + prefix = Pathname(bottle.cellar).parent opoo <<~EOS Building #{formula.full_name} from source as the bottle needs: - HOMEBREW_CELLAR: #{bottle.cellar} (yours is #{HOMEBREW_CELLAR}) - - HOMEBREW_PREFIX: #{bottle.prefix} (yours is #{HOMEBREW_PREFIX}) + - HOMEBREW_PREFIX: #{prefix} (yours is #{HOMEBREW_PREFIX}) EOS end return false diff --git a/Library/Homebrew/software_spec.rb b/Library/Homebrew/software_spec.rb index e796653be6..de44c202c3 100644 --- a/Library/Homebrew/software_spec.rb +++ b/Library/Homebrew/software_spec.rb @@ -91,7 +91,7 @@ class SoftwareSpec end def bottle_defined? - !bottle_specification.collector.keys.empty? + !bottle_specification.collector.tags.empty? end def bottle_tag?(tag = nil) @@ -302,7 +302,7 @@ class Bottle extend Forwardable - attr_reader :name, :resource, :prefix, :cellar, :rebuild + attr_reader :name, :resource, :cellar, :rebuild def_delegators :resource, :url, :verify_download_integrity def_delegators :resource, :cached_download @@ -314,15 +314,14 @@ class Bottle @resource.specs[:bottle] = true @spec = spec - checksum, tag, cellar = spec.checksum_for(Utils::Bottles.tag(tag)) + tag_spec = spec.tag_specification_for(Utils::Bottles.tag(tag)) - @prefix = spec.prefix - @tag = tag - @cellar = cellar + @tag = tag_spec.tag + @cellar = tag_spec.cellar @rebuild = spec.rebuild @resource.version = formula.pkg_version - @resource.checksum = checksum + @resource.checksum = tag_spec.checksum root_url(spec.root_url, spec.root_url_specs) end @@ -342,12 +341,12 @@ class Bottle end def compatible_locations? - @spec.compatible_locations? + @spec.compatible_locations?(tag: @tag) end # Does the bottle need to be relocated? def skip_relocation? - @spec.skip_relocation? + @spec.skip_relocation?(tag: @tag) end def stage @@ -469,12 +468,11 @@ class BottleSpecification attr_rw :rebuild attr_accessor :tap - attr_reader :all_tags_cellar, :collector, :root_url_specs, :repository, :prefix + attr_reader :collector, :root_url_specs, :repository sig { void } def initialize @rebuild = 0 - @prefix = Homebrew::DEFAULT_PREFIX @repository = Homebrew::DEFAULT_REPOSITORY @collector = Utils::Bottles::Collector.new @root_url_specs = {} @@ -497,27 +495,26 @@ class BottleSpecification end end - def cellar(val = nil) - if val.present? - odisabled( - "`cellar` in a bottle block", - "`brew style --fix` on the formula to update the style or use `sha256` with a `cellar:` argument", - ) - end - - if val.nil? - return collector.dig(Utils::Bottles.tag.to_sym, :cellar) || @all_tags_cellar || Homebrew::DEFAULT_CELLAR - end - - @all_tags_cellar = val + def cellar(_val = nil) + odisabled( + "`cellar` in a bottle block", + "`brew style --fix` on the formula to update the style or use `sha256` with a `cellar:` argument", + ) end - def compatible_locations? - # this looks like it should check prefix and repository too but to be - # `cellar :any` actually requires no references to the cellar, prefix or - # repository. + sig { params(tag: Utils::Bottles::Tag).returns(T::Boolean) } + def compatible_locations?(tag: Utils::Bottles.tag) + spec = collector.specification_for(tag) + cellar = if spec.present? + spec.cellar + else + tag.default_cellar + end + return true if [:any, :any_skip_relocation].include?(cellar) + prefix = Pathname(cellar).parent.to_s + compatible_cellar = cellar == HOMEBREW_CELLAR.to_s compatible_prefix = prefix == HOMEBREW_PREFIX.to_s @@ -525,14 +522,15 @@ class BottleSpecification end # Does the {Bottle} this {BottleSpecification} belongs to need to be relocated? - sig { returns(T::Boolean) } - def skip_relocation? - cellar == :any_skip_relocation + sig { params(tag: Utils::Bottles::Tag).returns(T::Boolean) } + def skip_relocation?(tag: Utils::Bottles.tag) + spec = collector.specification_for(tag) + spec&.cellar == :any_skip_relocation end sig { params(tag: T.any(Symbol, Utils::Bottles::Tag), no_older_versions: T::Boolean).returns(T::Boolean) } def tag?(tag, no_older_versions: false) - checksum_for(tag, no_older_versions: no_older_versions) ? true : false + collector.tag?(tag, no_older_versions: no_older_versions) end # Checksum methods in the DSL's bottle block take @@ -569,40 +567,35 @@ class BottleSpecification tag = Utils::Bottles::Tag.from_symbol(tag) - cellar ||= all_tags_cellar cellar ||= tag.default_cellar - collector[tag.to_sym] = { checksum: Checksum.new(digest), cellar: cellar } + collector.add(tag, checksum: Checksum.new(digest), cellar: cellar) end sig { - params( - tag: T.any(Symbol, Utils::Bottles::Tag), - no_older_versions: T::Boolean, - ).returns( - T.nilable([Checksum, Symbol, T.any(Symbol, String)]), - ) + params(tag: Utils::Bottles::Tag, no_older_versions: T::Boolean) + .returns(T.nilable(Utils::Bottles::TagSpecification)) } - def checksum_for(tag, no_older_versions: false) - collector.fetch_checksum_for(tag, no_older_versions: no_older_versions) + def tag_specification_for(tag, no_older_versions: false) + collector.specification_for(tag, no_older_versions: no_older_versions) end def checksums - tags = collector.keys.sort_by do |sym| - tag = Utils::Bottles::Tag.from_symbol(sym) + tags = collector.tags.sort_by do |tag| version = tag.to_macos_version # Give arm64 bottles a higher priority so they are first priority = tag.arch == :arm64 ? "2" : "1" - "#{priority}.#{version}_#{sym}" + "#{priority}.#{version}_#{tag}" rescue MacOSVersionError # Sort non-MacOS tags below MacOS tags. - "0.#{sym}" + "0.#{tag}" end tags.reverse.map do |tag| + spec = collector.specification_for(tag) { - "tag" => tag, - "digest" => collector[tag][:checksum], - "cellar" => collector[tag][:cellar], + "tag" => spec.tag.to_sym, + "digest" => spec.checksum, + "cellar" => spec.cellar, } end end diff --git a/Library/Homebrew/test/dev-cmd/bottle_spec.rb b/Library/Homebrew/test/dev-cmd/bottle_spec.rb index 92949c5f82..114eba3192 100644 --- a/Library/Homebrew/test/dev-cmd/bottle_spec.rb +++ b/Library/Homebrew/test/dev-cmd/bottle_spec.rb @@ -386,7 +386,7 @@ describe "brew bottle" do describe "#merge_bottle_spec" do it "allows new bottle hash to be empty" do - valid_keys = [:root_url, :prefix, :cellar, :rebuild, :sha256] + valid_keys = [:root_url, :cellar, :rebuild, :sha256] old_spec = BottleSpecification.new old_spec.sha256(big_sur: "f59bc65c91e4e698f6f050e1efea0040f57372d4dcf0996cbb8f97ced320403b") expect { homebrew.merge_bottle_spec(valid_keys, old_spec, {}) }.not_to raise_error diff --git a/Library/Homebrew/test/software_spec/bottle_spec.rb b/Library/Homebrew/test/software_spec/bottle_spec.rb index fae438cef2..01e45373fe 100644 --- a/Library/Homebrew/test/software_spec/bottle_spec.rb +++ b/Library/Homebrew/test/software_spec/bottle_spec.rb @@ -17,8 +17,8 @@ describe BottleSpecification do checksums.each_pair do |cat, digest| bottle_spec.sha256(cat => digest) - checksum, = bottle_spec.checksum_for(cat) - expect(Checksum.new(digest)).to eq(checksum) + tag_spec = bottle_spec.tag_specification_for(Utils::Bottles::Tag.from_symbol(cat)) + expect(Checksum.new(digest)).to eq(tag_spec.checksum) end end @@ -32,11 +32,11 @@ describe BottleSpecification do checksums.each do |checksum| bottle_spec.sha256(cellar: checksum[:cellar], checksum[:tag] => checksum[:digest]) - digest, tag, cellar = bottle_spec.checksum_for(checksum[:tag]) - expect(Checksum.new(checksum[:digest])).to eq(digest) - expect(checksum[:tag]).to eq(tag) + tag_spec = bottle_spec.tag_specification_for(Utils::Bottles::Tag.from_symbol(checksum[:tag])) + expect(Checksum.new(checksum[:digest])).to eq(tag_spec.checksum) + expect(checksum[:tag]).to eq(tag_spec.tag.to_sym) checksum[:cellar] ||= Homebrew::DEFAULT_CELLAR - expect(checksum[:cellar]).to eq(cellar) + expect(checksum[:cellar]).to eq(tag_spec.cellar) end end end diff --git a/Library/Homebrew/test/utils/bottles/collector_spec.rb b/Library/Homebrew/test/utils/bottles/collector_spec.rb index e43035113c..696cabc76f 100644 --- a/Library/Homebrew/test/utils/bottles/collector_spec.rb +++ b/Library/Homebrew/test/utils/bottles/collector_spec.rb @@ -6,43 +6,50 @@ require "utils/bottles" describe Utils::Bottles::Collector do subject(:collector) { described_class.new } - describe "#fetch_checksum_for" do + let(:catalina) { Utils::Bottles::Tag.from_symbol(:catalina) } + let(:mojave) { Utils::Bottles::Tag.from_symbol(:mojave) } + + describe "#specification_for" do it "returns passed tags" do - collector[:mojave] = { checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar" } - collector[:catalina] = { checksum: Checksum.new("bar_checksum"), cellar: "bar_cellar" } - expect(collector.fetch_checksum_for(:catalina)).to eq(["bar_checksum", :catalina, "bar_cellar"]) + collector.add(mojave, checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar") + collector.add(catalina, checksum: Checksum.new("bar_checksum"), cellar: "bar_cellar") + spec = collector.specification_for(catalina) + expect(spec).not_to be_nil + expect(spec.tag).to eq(catalina) + expect(spec.checksum).to eq("bar_checksum") + expect(spec.cellar).to eq("bar_cellar") end it "returns nil if empty" do - expect(collector.fetch_checksum_for(:foo)).to be nil + expect(collector.specification_for(Utils::Bottles::Tag.from_symbol(:foo))).to be nil end it "returns nil when there is no match" do - collector[:catalina] = "foo" - expect(collector.fetch_checksum_for(:foo)).to be nil + collector.add(catalina, checksum: Checksum.new("bar_checksum"), cellar: "bar_cellar") + expect(collector.specification_for(Utils::Bottles::Tag.from_symbol(:foo))).to be nil end it "uses older tags when needed", :needs_macos do - collector[:mojave] = "foo" - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:mojave))).to eq(:mojave) - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:catalina))).to eq(:mojave) + collector.add(mojave, checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar") + expect(collector.send(:find_matching_tag, mojave)).to eq(mojave) + expect(collector.send(:find_matching_tag, catalina)).to eq(mojave) end it "does not use older tags when requested not to", :needs_macos do allow(Homebrew::EnvConfig).to receive(:developer?).and_return(true) allow(Homebrew::EnvConfig).to receive(:skip_or_later_bottles?).and_return(true) allow(OS::Mac.version).to receive(:prerelease?).and_return(true) - collector[:mojave] = "foo" - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:mojave))).to eq(:mojave) - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:catalina))).to be_nil + collector.add(mojave, checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar") + expect(collector.send(:find_matching_tag, mojave)).to eq(mojave) + expect(collector.send(:find_matching_tag, catalina)).to be_nil end it "ignores HOMEBREW_SKIP_OR_LATER_BOTTLES on release versions", :needs_macos do allow(Homebrew::EnvConfig).to receive(:skip_or_later_bottles?).and_return(true) allow(OS::Mac.version).to receive(:prerelease?).and_return(false) - collector[:mojave] = "foo" - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:mojave))).to eq(:mojave) - expect(collector.send(:find_matching_tag, Utils::Bottles::Tag.from_symbol(:catalina))).to eq(:mojave) + collector.add(mojave, checksum: Checksum.new("foo_checksum"), cellar: "foo_cellar") + expect(collector.send(:find_matching_tag, mojave)).to eq(mojave) + expect(collector.send(:find_matching_tag, catalina)).to eq(mojave) end end end diff --git a/Library/Homebrew/utils/bottles.rb b/Library/Homebrew/utils/bottles.rb index ad121c6151..faf350f5f6 100644 --- a/Library/Homebrew/utils/bottles.rb +++ b/Library/Homebrew/utils/bottles.rb @@ -11,6 +11,7 @@ module Utils class << self extend T::Sig + # Gets the tag for the running OS. def tag(symbol = nil) return Tag.from_symbol(symbol) if symbol.present? @@ -160,6 +161,14 @@ module Utils end end + def eql?(other) + self.class == other.class && self == other + end + + def hash + [system, arch].hash + end + sig { returns(Symbol) } def to_sym if system == :all && arch == :all @@ -217,40 +226,74 @@ module Utils end end + # The specification for a specific tag + class TagSpecification + extend T::Sig + + sig { returns(Utils::Bottles::Tag) } + attr_reader :tag + + sig { returns(Checksum) } + attr_reader :checksum + + sig { returns(T.any(Symbol, String)) } + attr_reader :cellar + + def initialize(tag:, checksum:, cellar:) + @tag = tag + @checksum = checksum + @cellar = cellar + end + end + # Collector for bottle specifications. class Collector extend T::Sig - extend Forwardable - - def_delegators :@checksums, :keys, :[], :[]=, :key?, :each_key, :dig - sig { void } def initialize - @checksums = {} + @tag_specs = T.let({}, T::Hash[Utils::Bottles::Tag, Utils::Bottles::TagSpecification]) + end + + sig { returns(T::Array[Utils::Bottles::Tag]) } + def tags + @tag_specs.keys + end + + sig { params(tag: Utils::Bottles::Tag, checksum: Checksum, cellar: T.any(Symbol, String)).void } + def add(tag, checksum:, cellar:) + spec = Utils::Bottles::TagSpecification.new(tag: tag, checksum: checksum, cellar: cellar) + @tag_specs[tag] = spec + end + + sig { params(tag: Utils::Bottles::Tag, no_older_versions: T::Boolean).returns(T::Boolean) } + def tag?(tag, no_older_versions: false) + tag = find_matching_tag(tag, no_older_versions: no_older_versions) + tag.present? + end + + sig { params(block: T.proc.params(tag: Utils::Bottles::Tag).void).void } + def each_tag(&block) + @tag_specs.each_key(&block) end sig { - params( - tag: T.any(Symbol, Utils::Bottles::Tag), - no_older_versions: T::Boolean, - ).returns( - T.nilable([Checksum, Symbol, T.any(Symbol, String)]), - ) + params(tag: Utils::Bottles::Tag, no_older_versions: T::Boolean) + .returns(T.nilable(Utils::Bottles::TagSpecification)) } - def fetch_checksum_for(tag, no_older_versions: false) - tag = Utils::Bottles::Tag.from_symbol(tag) if tag.is_a?(Symbol) - tag = find_matching_tag(tag, no_older_versions: no_older_versions)&.to_sym - return self[tag][:checksum], tag, self[tag][:cellar] if tag + def specification_for(tag, no_older_versions: false) + tag = find_matching_tag(tag, no_older_versions: no_older_versions) + @tag_specs[tag] if tag end private def find_matching_tag(tag, no_older_versions: false) - if key?(tag.to_sym) + if @tag_specs.key?(tag) tag - elsif key?(:all) - Tag.from_symbol(:all) + else + all = Tag.from_symbol(:all) + all if @tag_specs.key?(all) end end end