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>
This commit is contained in:
parent
0f9abe57ea
commit
ea4d137e87
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
@ -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"
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user