dependency: fix merging tags in 'merge_repeats'

While it may suffice to merge string and non-reserved tags by forming a
union of all tags of dependencies of the same name, this approach fails
to work for the reserved tags. These are now merged such that the most
restrictive tag (meaning sometimes an empty tag) is preserved.

The previous behavior caused essential dependencies to be omitted and
builds to fail in response. E.g., multiple `:fortran` dependencies with
tags `[]`, `[:recommended]`, and `[:optional]` would have been expanded
and merged to `"gcc"` with tags `[:recommended, :optional]`, causing it
to be no longer seen as a required dependency.

Closes Homebrew/homebrew#47040.

Signed-off-by: Martin Afanasjew <martin@afanasjew.de>

3 files changed, 37 insertions, 3 deletions

diff --git a/Library/Homebrew/dependable.rb b/Library/Homebrew/dependable.rb
index 4a3d2de88..4060973c4 100644
--- a/Library/Homebrew/dependable.rb
+++ b/Library/Homebrew/dependable.rb
@@ -20,10 +20,16 @@ module Dependable
   end
 
   def required?
+    # FIXME: Should `required?` really imply `!build?`? And if so, why doesn't
+    #        any of `optional?` and `recommended?` equally imply `!build?`?
     !build? && !optional? && !recommended?
   end
 
+  def option_tags
+    tags - RESERVED_TAGS
+  end
+
   def options
-    Options.create(tags - RESERVED_TAGS)
+    Options.create(option_tags)
   end
 end
diff --git a/Library/Homebrew/dependency.rb b/Library/Homebrew/dependency.rb
index a3b9d7b51..0e03ae0ea 100644
--- a/Library/Homebrew/dependency.rb
+++ b/Library/Homebrew/dependency.rb
@@ -124,11 +124,39 @@ class Dependency
       all.map(&:name).uniq.map do |name|
         deps = grouped.fetch(name)
         dep  = deps.first
-        tags = deps.flat_map(&:tags).uniq
+        tags = merge_tags(deps)
         option_names = deps.flat_map(&:option_names).uniq
         dep.class.new(name, tags, dep.env_proc, option_names)
       end
     end
+
+    private
+
+    def merge_tags(deps)
+      options = deps.flat_map(&:option_tags).uniq
+      merge_necessity(deps) + merge_temporality(deps) + options
+    end
+
+    def merge_necessity(deps)
+      # Cannot use `deps.any?(&:required?)` here due to its definition.
+      if deps.any? { |dep| !dep.recommended? && !dep.optional? }
+        [] # Means required dependency.
+      elsif deps.any?(&:recommended?)
+        [:recommended]
+      else # deps.all?(&:optional?)
+        [:optional]
+      end
+    end
+
+    def merge_temporality(deps)
+      if deps.all?(&:build?)
+        [:build]
+      elsif deps.all?(&:run?)
+        [:run]
+      else
+        [] # Means both build and runtime dependency.
+      end
+    end
   end
 end
 
diff --git a/Library/Homebrew/test/test_dependency.rb b/Library/Homebrew/test/test_dependency.rb
index 4ba7033d9..16bad6507 100644
--- a/Library/Homebrew/test/test_dependency.rb
+++ b/Library/Homebrew/test/test_dependency.rb
@@ -48,7 +48,7 @@ class DependencyTests < Homebrew::TestCase
     assert_equal Dependency, merged.first.class
 
     foo_named_dep = merged.find {|d| d.name == "foo"}
-    assert_equal [:build, "bar"], foo_named_dep.tags
+    assert_equal ["bar"], foo_named_dep.tags
     assert_includes foo_named_dep.option_names, "foo"
     assert_includes foo_named_dep.option_names, "foo2"