Merge repeated deps with differing options
When expanding dependencies, repeated deps are treated as equal and all but the first are discarded when #uniq is called on the resulting array. However, they may have different sets of options attached, so we cannot assume they are the same. After the initial expansion, we group them by name and then create a new Dependency object for each name, merging the options from each group. Fixes Homebrew/homebrew#20335.
This commit is contained in:
		
							parent
							
								
									80745a97e2
								
							
						
					
					
						commit
						8cb861c695
					
				@ -70,13 +70,15 @@ class Dependency
 | 
			
		||||
    # The default filter, which is applied when a block is not given, omits
 | 
			
		||||
    # optionals and recommendeds based on what the dependent has asked for.
 | 
			
		||||
    def expand(dependent, &block)
 | 
			
		||||
      dependent.deps.map do |dep|
 | 
			
		||||
      deps = dependent.deps.map do |dep|
 | 
			
		||||
        if prune?(dependent, dep, &block)
 | 
			
		||||
          next
 | 
			
		||||
        else
 | 
			
		||||
          expand(dep.to_formula, &block) << dep
 | 
			
		||||
        end
 | 
			
		||||
      end.flatten.compact.uniq
 | 
			
		||||
      end.flatten.compact
 | 
			
		||||
 | 
			
		||||
      merge_repeats(deps)
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    def prune?(dependent, dep, &block)
 | 
			
		||||
@ -93,5 +95,13 @@ class Dependency
 | 
			
		||||
    def prune
 | 
			
		||||
      throw(:prune, true)
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    def merge_repeats(deps)
 | 
			
		||||
      grouped = deps.group_by(&:name)
 | 
			
		||||
 | 
			
		||||
      deps.uniq.map do |dep|
 | 
			
		||||
        new(dep.name, grouped.fetch(dep.name).map(&:tags).flatten)
 | 
			
		||||
      end
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
 | 
			
		||||
@ -2,8 +2,8 @@ require 'testing_env'
 | 
			
		||||
require 'dependency'
 | 
			
		||||
 | 
			
		||||
class DependencyExpansionTests < Test::Unit::TestCase
 | 
			
		||||
  def build_dep(name, deps=[])
 | 
			
		||||
    dep = Dependency.new(name.to_s)
 | 
			
		||||
  def build_dep(name, tags=[], deps=[])
 | 
			
		||||
    dep = Dependency.new(name.to_s, tags)
 | 
			
		||||
    dep.stubs(:to_formula).returns(stub(:deps => deps))
 | 
			
		||||
    dep
 | 
			
		||||
  end
 | 
			
		||||
@ -58,4 +58,12 @@ class DependencyExpansionTests < Test::Unit::TestCase
 | 
			
		||||
    @f = stub(:deps => @deps, :build => stub(:with? => true))
 | 
			
		||||
    assert_equal @deps, Dependency.expand(@f)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  def test_merges_repeated_deps_with_differing_options
 | 
			
		||||
    @foo2 = build_dep(:foo, ['option'])
 | 
			
		||||
    @baz2 = build_dep(:baz, ['option'])
 | 
			
		||||
    @deps << @foo2 << @baz2
 | 
			
		||||
    deps = [@foo2, @bar, @baz2, @qux]
 | 
			
		||||
    assert_equal deps, Dependency.expand(@f)
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
 | 
			
		||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user