From ea4d137e871ace8200743f237c48189e3b930da1 Mon Sep 17 00:00:00 2001
From: Martin Afanasjew <martin@afanasjew.de>
Date: Tue, 15 Dec 2015 10:50:38 +0100
Subject: [PATCH] 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>
---
 Library/Homebrew/dependable.rb           |  8 ++++++-
 Library/Homebrew/dependency.rb           | 30 +++++++++++++++++++++++-
 Library/Homebrew/test/test_dependency.rb |  2 +-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/Library/Homebrew/dependable.rb b/Library/Homebrew/dependable.rb
index 4a3d2de88b..4060973c4c 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 a3b9d7b51a..0e03ae0ea3 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 4ba7033d9c..16bad65072 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"