diff --git a/Library/Homebrew/dev-cmd/bottle.rb b/Library/Homebrew/dev-cmd/bottle.rb index 46144f0f3b..fd64520781 100644 --- a/Library/Homebrew/dev-cmd/bottle.rb +++ b/Library/Homebrew/dev-cmd/bottle.rb @@ -496,11 +496,14 @@ module Homebrew update_or_add = T.let(nil, T.nilable(String)) Utils::Inreplace.inreplace(path) do |s| - if s.inreplace_string.include? "bottle do" + formula_contents = s.inreplace_string + bottle_node = Utils::AST.bottle_block(formula_contents) + if bottle_node.present? update_or_add = "update" if args.keep_old? + old_keys = Utils::AST.body_children(bottle_node.body).map(&:method_name) old_bottle_spec = Formulary.factory(path).bottle_specification - mismatches, checksums = merge_bottle_spec(old_bottle_spec, bottle_hash["bottle"]) + mismatches, checksums = merge_bottle_spec(old_keys, old_bottle_spec, bottle_hash["bottle"]) if mismatches.present? odie <<~EOS --keep-old was passed but there are changes in: @@ -511,12 +514,12 @@ module Homebrew output = bottle_output bottle end puts output - Utils::AST.replace_bottle_stanza!(s.inreplace_string, output) + Utils::AST.replace_bottle_stanza!(formula_contents, output) else odie "--keep-old was passed but there was no existing bottle block!" if args.keep_old? puts output update_or_add = "add" - Utils::AST.add_bottle_stanza!(s.inreplace_string, output) + Utils::AST.add_bottle_stanza!(formula_contents, output) end end @@ -539,33 +542,33 @@ module Homebrew end end - def merge_bottle_spec(old_bottle_spec, new_bottle_hash) + def merge_bottle_spec(old_keys, old_bottle_spec, new_bottle_hash) mismatches = [] checksums = [] - { + new_values = { root_url: new_bottle_hash["root_url"], prefix: new_bottle_hash["prefix"], - cellar: new_bottle_hash["cellar"].to_sym, + cellar: new_bottle_hash["cellar"], rebuild: new_bottle_hash["rebuild"], - }.each do |key, new_value| - old_value = old_bottle_spec.send(key) - next if key == :rebuild && old_value.zero? - next if key == :prefix && old_value == Homebrew::DEFAULT_PREFIX - next if key == :cellar && [ - Homebrew::DEFAULT_MACOS_CELLAR, - Homebrew::DEFAULT_MACOS_ARM_CELLAR, - Homebrew::DEFAULT_LINUX_CELLAR, - ].include?(old_value) - next if key == :cellar && old_value == :any && new_value == :any_skip_relocation + } + + old_keys.each do |key| + next if key == :sha256 + + old_value = old_bottle_spec.send(key).to_s + new_value = new_values[key].to_s + next if key == :cellar && old_value == "any" && new_value == "any_skip_relocation" next if old_value.present? && new_value == old_value mismatches << "#{key}: old: #{old_value.inspect}, new: #{new_value.inspect}" end + return [mismatches, checksums] unless old_keys.include? :sha256 + old_bottle_spec.collector.each_key do |tag| old_value = old_bottle_spec.collector[tag].hexdigest - new_value = new_bottle_hash["tags"][tag.to_s] + new_value = new_bottle_hash.dig("tags", tag.to_s) if new_value.present? mismatches << "sha256 => #{tag}" else diff --git a/Library/Homebrew/test/dev-cmd/bottle_spec.rb b/Library/Homebrew/test/dev-cmd/bottle_spec.rb index 299a1f583c..baceed730a 100644 --- a/Library/Homebrew/test/dev-cmd/bottle_spec.rb +++ b/Library/Homebrew/test/dev-cmd/bottle_spec.rb @@ -173,6 +173,66 @@ describe Homebrew do "d9cc50eec8ac243148a121049c236cba06af4a0b1156ab397d0a2850aa79c137", ) end + + describe "#merge_bottle_spec" do + it "allows new bottle hash to be empty" do + valid_keys = [:root_url, :prefix, :cellar, :rebuild, :sha256] + old_spec = BottleSpecification.new + old_spec.sha256("f59bc65c91e4e698f6f050e1efea0040f57372d4dcf0996cbb8f97ced320403b" => :big_sur) + expect { homebrew.merge_bottle_spec(valid_keys, old_spec, {}) }.not_to raise_error + end + + it "checks for conflicting root URL" do + old_spec = BottleSpecification.new + old_spec.root_url("https://failbrew.bintray.com/bottles") + new_hash = { "root_url" => "https://testbrew.bintray.com/bottles" } + expect(homebrew.merge_bottle_spec([:root_url], old_spec, new_hash)).to eq [ + ['root_url: old: "https://failbrew.bintray.com/bottles", new: "https://testbrew.bintray.com/bottles"'], + [], + ] + end + + it "checks for conflicting prefix" do + old_spec = BottleSpecification.new + old_spec.prefix("/opt/failbrew") + new_hash = { "prefix" => "/opt/testbrew" } + expect(homebrew.merge_bottle_spec([:prefix], old_spec, new_hash)).to eq [ + ['prefix: old: "/opt/failbrew", new: "/opt/testbrew"'], + [], + ] + end + + it "checks for conflicting cellar" do + old_spec = BottleSpecification.new + old_spec.cellar("/opt/failbrew/Cellar") + new_hash = { "cellar" => "/opt/testbrew/Cellar" } + expect(homebrew.merge_bottle_spec([:cellar], old_spec, new_hash)).to eq [ + ['cellar: old: "/opt/failbrew/Cellar", new: "/opt/testbrew/Cellar"'], + [], + ] + end + + it "checks for conflicting rebuild number" do + old_spec = BottleSpecification.new + old_spec.rebuild(1) + new_hash = { "rebuild" => 2 } + expect(homebrew.merge_bottle_spec([:rebuild], old_spec, new_hash)).to eq [ + ['rebuild: old: "1", new: "2"'], + [], + ] + end + + it "checks for conflicting checksums" do + old_spec = BottleSpecification.new + old_spec.sha256("109c0cb581a7b5d84da36d84b221fb9dd0f8a927b3044d82611791c9907e202e" => :catalina) + old_spec.sha256("7571772bf7a0c9fe193e70e521318b53993bee6f351976c9b6e01e00d13d6c3f" => :mojave) + new_hash = { "tags" => { "catalina" => "ec6d7f08412468f28dee2be17ad8cd8b883b16b34329efcecce019b8c9736428" } } + expect(homebrew.merge_bottle_spec([:sha256], old_spec, new_hash)).to eq [ + ["sha256 => catalina"], + [{ "7571772bf7a0c9fe193e70e521318b53993bee6f351976c9b6e01e00d13d6c3f" => :mojave }], + ] + end + end end describe "brew bottle --merge", :integration_test, :needs_linux do diff --git a/Library/Homebrew/utils/ast.rb b/Library/Homebrew/utils/ast.rb index 8cac1b66b9..80fe2f6113 100644 --- a/Library/Homebrew/utils/ast.rb +++ b/Library/Homebrew/utils/ast.rb @@ -9,6 +9,25 @@ module Utils class << self extend T::Sig + def body_children(body_node) + if body_node.nil? + [] + elsif body_node.begin_type? + body_node.children.compact + else + [body_node] + end + end + + def bottle_block(formula_contents) + formula_stanza(formula_contents, :bottle, type: :block_call) + end + + def formula_stanza(formula_contents, name, type: nil) + _, children = process_formula(formula_contents) + children.find { |child| call_node_match?(child, name: name, type: type) } + end + def replace_bottle_stanza!(formula_contents, bottle_output) replace_formula_stanza!(formula_contents, :bottle, bottle_output.strip, type: :block_call) end @@ -18,8 +37,7 @@ module Utils end def replace_formula_stanza!(formula_contents, name, replacement, type: nil) - processed_source, body_node = process_formula(formula_contents) - children = body_node.begin_type? ? body_node.children.compact : [body_node] + processed_source, children = process_formula(formula_contents) stanza_node = children.find { |child| call_node_match?(child, name: name, type: type) } raise "Could not find #{name} stanza!" if stanza_node.nil? @@ -29,10 +47,10 @@ module Utils end def add_formula_stanza!(formula_contents, name, text, type: nil) - processed_source, body_node = process_formula(formula_contents) + processed_source, children = process_formula(formula_contents) - preceding_component = if body_node.begin_type? - body_node.children.compact.reduce do |previous_child, current_child| + preceding_component = if children.length > 1 + children.reduce do |previous_child, current_child| if formula_component_before_target?(current_child, target_name: name, target_type: type) @@ -42,7 +60,7 @@ module Utils end end else - body_node + children.first end preceding_component = preceding_component.last_argument if preceding_component.send_type? @@ -84,10 +102,10 @@ module Utils raise "Could not find formula class!" if class_node.nil? - body_node = class_node.body - raise "Formula class is empty!" if body_node.nil? + children = body_children(class_node.body) + raise "Formula class is empty!" if children.empty? - [processed_source, body_node] + [processed_source, children] end def formula_component_before_target?(node, target_name:, target_type: nil)