diff --git a/Library/Homebrew/cask.rb b/Library/Homebrew/cask.rb index dacb4c9da3..2f9df69d02 100644 --- a/Library/Homebrew/cask.rb +++ b/Library/Homebrew/cask.rb @@ -20,6 +20,5 @@ require "cask/metadata" require "cask/pkg" require "cask/quarantine" require "cask/staged" -require "cask/topological_hash" require "cask/url" require "cask/utils" diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 863880fd54..393ce5ead7 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -3,8 +3,8 @@ require "formula_installer" require "unpack_strategy" +require "utils/topological_hash" -require "cask/topological_hash" require "cask/config" require "cask/download" require "cask/staged" @@ -290,43 +290,14 @@ module Cask "but you are running #{@current_arch}." end - def graph_dependencies(cask_or_formula, acc = TopologicalHash.new) - return acc if acc.key?(cask_or_formula) - - if cask_or_formula.is_a?(Cask) - formula_deps = cask_or_formula.depends_on.formula.map { |f| Formula[f] } - cask_deps = cask_or_formula.depends_on.cask.map { |c| CaskLoader.load(c, config: nil) } - else - formula_deps = cask_or_formula.deps.reject(&:build?).map(&:to_formula) - cask_deps = cask_or_formula.requirements.map(&:cask).compact - .map { |c| CaskLoader.load(c, config: nil) } - end - - acc[cask_or_formula] ||= [] - acc[cask_or_formula] += formula_deps - acc[cask_or_formula] += cask_deps - - formula_deps.each do |f| - graph_dependencies(f, acc) - end - - cask_deps.each do |c| - graph_dependencies(c, acc) - end - - acc - end - def collect_cask_and_formula_dependencies return @cask_and_formula_dependencies if @cask_and_formula_dependencies - graph = graph_dependencies(@cask) + graph = ::Utils::TopologicalHash.graph_package_dependencies(@cask) raise CaskSelfReferencingDependencyError, cask.token if graph[@cask].include?(@cask) - primary_container.dependencies.each do |dep| - graph_dependencies(dep, graph) - end + ::Utils::TopologicalHash.graph_package_dependencies(primary_container.dependencies, graph) begin @cask_and_formula_dependencies = graph.tsort - [@cask] diff --git a/Library/Homebrew/cask/topological_hash.rb b/Library/Homebrew/cask/topological_hash.rb deleted file mode 100644 index 12b6f588e7..0000000000 --- a/Library/Homebrew/cask/topological_hash.rb +++ /dev/null @@ -1,21 +0,0 @@ -# typed: true -# frozen_string_literal: true - -require "tsort" - -module Cask - # Topologically sortable hash map. - class TopologicalHash < Hash - include TSort - - private - - def tsort_each_node(&block) - each_key(&block) - end - - def tsort_each_child(node, &block) - fetch(node).each(&block) - end - end -end diff --git a/Library/Homebrew/test/utils/topological_hash_spec.rb b/Library/Homebrew/test/utils/topological_hash_spec.rb new file mode 100644 index 0000000000..8292a9f7cf --- /dev/null +++ b/Library/Homebrew/test/utils/topological_hash_spec.rb @@ -0,0 +1,99 @@ +# typed: false +# frozen_string_literal: true + +require "utils/topological_hash" + +describe Utils::TopologicalHash do + describe "#tsort" do + it "returns a topologically sorted array" do + hash = described_class.new + hash[1] = [2, 3] + hash[2] = [3] + hash[3] = [] + hash[4] = [] + expect(hash.tsort).to eq [3, 2, 1, 4] + end + end + + describe "#strongly_connected_components" do + it "returns an array of arrays" do + hash = described_class.new + hash[1] = [2] + hash[2] = [3, 4] + hash[3] = [2] + hash[4] = [] + expect(hash.strongly_connected_components).to eq [[4], [2, 3], [1]] + end + end + + describe "::graph_package_dependencies" do + it "returns a topological hash" do + formula1 = formula "homebrew-test-formula1" do + url "foo" + version "0.5" + end + + formula2 = formula "homebrew-test-formula2" do + url "foo" + version "0.5" + depends_on "homebrew-test-formula1" + end + + formula3 = formula "homebrew-test-formula3" do + url "foo" + version "0.5" + depends_on "homebrew-test-formula4" + end + + formula4 = formula "homebrew-test-formula4" do + url "foo" + version "0.5" + depends_on "homebrew-test-formula3" + end + + cask1 = Cask::Cask.new("homebrew-test-cask1") do + url "foo" + version "1.2.3" + end + + cask2 = Cask::Cask.new("homebrew-test-cask2") do + url "foo" + version "1.2.3" + depends_on cask: "homebrew-test-cask1" + depends_on formula: "homebrew-test-formula1" + end + + cask3 = Cask::Cask.new("homebrew-test-cask3") do + url "foo" + version "1.2.3" + depends_on cask: "homebrew-test-cask2" + end + + stub_formula_loader formula1 + stub_formula_loader formula2 + stub_formula_loader formula3 + stub_formula_loader formula4 + + stub_cask_loader cask1 + stub_cask_loader cask2 + stub_cask_loader cask3 + + packages = [formula1, formula2, formula3, formula4, cask1, cask2, cask3] + expect(described_class.graph_package_dependencies(packages)).to eq({ + formula1 => [], + formula2 => [formula1], + formula3 => [formula4], + formula4 => [formula3], + cask1 => [], + cask2 => [formula1, cask1], + cask3 => [cask2], + }) + + sorted = [formula1, cask1, cask2, cask3, formula2] + expect(described_class.graph_package_dependencies([cask3, cask2, cask1, formula2, formula1]).tsort).to eq sorted + expect(described_class.graph_package_dependencies([cask3, formula2]).tsort).to eq sorted + + expect { described_class.graph_package_dependencies([formula3, formula4]).tsort }.to raise_error TSort::Cyclic + end + end +end diff --git a/Library/Homebrew/upgrade.rb b/Library/Homebrew/upgrade.rb index 62a38a9dc0..8f783f079d 100644 --- a/Library/Homebrew/upgrade.rb +++ b/Library/Homebrew/upgrade.rb @@ -6,6 +6,7 @@ require "formula_installer" require "development_tools" require "messages" require "cleanup" +require "utils/topological_hash" module Homebrew # Helper functions for upgrading formulae. @@ -42,6 +43,14 @@ module Homebrew end end + dependency_graph = Utils::TopologicalHash.graph_package_dependencies(formulae_to_install) + begin + formulae_to_install = dependency_graph.tsort & formulae_to_install + rescue TSort::Cyclic + # Failed to sort formulae topologically because there are cyclic + # dependencies. Let FormulaInstaller handle it. + end + formula_installers = formulae_to_install.map do |formula| Migrator.migrate_if_needed(formula, force: force, dry_run: dry_run) begin diff --git a/Library/Homebrew/utils/topological_hash.rb b/Library/Homebrew/utils/topological_hash.rb new file mode 100644 index 0000000000..c6a88b3268 --- /dev/null +++ b/Library/Homebrew/utils/topological_hash.rb @@ -0,0 +1,63 @@ +# typed: true +# frozen_string_literal: true + +require "tsort" + +module Utils + # Topologically sortable hash map. + class TopologicalHash < Hash + extend T::Sig + + include TSort + + sig { + params( + packages: T.any(Cask::Cask, Formula, T::Array[T.any(Cask::Cask, Formula)]), + accumulator: TopologicalHash, + ).returns(TopologicalHash) + } + def self.graph_package_dependencies(packages, accumulator = TopologicalHash.new) + packages = Array(packages) + + packages.each do |cask_or_formula| + next accumulator if accumulator.key?(cask_or_formula) + + if cask_or_formula.is_a?(Cask::Cask) + formula_deps = cask_or_formula.depends_on + .formula + .map { |f| Formula[f] } + cask_deps = cask_or_formula.depends_on + .cask + .map { |c| Cask::CaskLoader.load(c, config: nil) } + else + formula_deps = cask_or_formula.deps + .reject(&:build?) + .map(&:to_formula) + cask_deps = cask_or_formula.requirements + .map(&:cask) + .compact + .map { |c| Cask::CaskLoader.load(c, config: nil) } + end + + accumulator[cask_or_formula] ||= [] + accumulator[cask_or_formula] += formula_deps + accumulator[cask_or_formula] += cask_deps + + graph_package_dependencies(formula_deps, accumulator) + graph_package_dependencies(cask_deps, accumulator) + end + + accumulator + end + + private + + def tsort_each_node(&block) + each_key(&block) + end + + def tsort_each_child(node, &block) + fetch(node).each(&block) + end + end +end