bottle: check actual bottle block contents when --keep-old

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,24 +542,22 @@ 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,
       rebuild:  new_bottle_hash["rebuild"],
-    }.each do |key, new_value|
+    }
+
+    old_keys.each do |key|
+      next if key == :sha256
+
       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)
+      new_value = new_values[key]
       next if key == :cellar && old_value == :any && new_value == :any_skip_relocation
       next if old_value.present? && new_value == old_value
 

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)