From 5f95cc388df56c058b974a5b55a9a06680d43b38 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 24 Aug 2022 13:22:00 +0800 Subject: [PATCH 1/8] formula_auditor: refactor GCC dependency check into separate method The GCC dependency check is adding a couple of minutes to our `tap_syntax` jobs. Let's fix that by moving the check into a separate method so we can exclude it from `tap_syntax`. --- Library/Homebrew/formula_auditor.rb | 36 +++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index 21da794564..e151e6a139 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -335,13 +335,6 @@ module Homebrew end return unless @core_tap - - bad_gcc_dep = linux_only_gcc_dep?(formula) && (@strict || begin - fv = FormulaVersions.new(formula) - fv.formula_at_revision("origin/HEAD") { |prev_f| !linux_only_gcc_dep?(prev_f) } - end) - problem "Formulae in homebrew/core should not have a Linux-only dependency on GCC." if bad_gcc_dep - return if formula.tap&.audit_exception :versioned_dependencies_conflicts_allowlist, formula.name # The number of conflicts on Linux is absurd. @@ -415,6 +408,20 @@ module Homebrew end end + def audit_gcc_dependency + return unless @core_tap + return unless Homebrew::SimulateSystem.simulating_or_running_on_linux? + return unless linux_only_gcc_dep?(formula) + + bad_gcc_dep = @strict || begin + fv = FormulaVersions.new(formula) + fv.formula_at_revision("origin/HEAD") { |prev_f| !linux_only_gcc_dep?(prev_f) } + end + return unless bad_gcc_dep + + problem "Formulae in homebrew/core should not have a Linux-only dependency on GCC." + end + def audit_postgresql return unless formula.name == "postgresql" return unless @core_tap @@ -865,14 +872,19 @@ module Homebrew end def linux_only_gcc_dep?(formula) - # TODO: Make this check work when running on Linux and not simulating macOS too. - return false unless Homebrew::SimulateSystem.simulating_or_running_on_macos? + odie "`#linux_only_gcc_dep?` works only on Linux!" if Homebrew::SimulateSystem.simulating_or_running_on_macos? formula_hash = formula.to_hash_with_variations - deps = formula_hash["dependencies"] - linux_deps = formula_hash.dig("variations", :x86_64_linux, "dependencies") + linux_deps = formula_hash["dependencies"] + variations_deps = [] + formula_hash["variations"].map do |variation, data| + next if variation == :x86_64_linux - deps.exclude?("gcc") && linux_deps&.include?("gcc") + variations_deps += data["dependencies"] + end + variations_deps.uniq! + + linux_deps.include?("gcc") && variations_deps&.exclude?("gcc") end end end From e68b1a4a89f3b844bb13aead7c1fb9b221e33db7 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 24 Aug 2022 15:09:53 +0800 Subject: [PATCH 2/8] Tweak `#linux_only_gcc_dep?`. --- Library/Homebrew/formula_auditor.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index e151e6a139..f13a2caf8c 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -876,15 +876,25 @@ module Homebrew formula_hash = formula.to_hash_with_variations linux_deps = formula_hash["dependencies"] - variations_deps = [] - formula_hash["variations"].map do |variation, data| - next if variation == :x86_64_linux + return false if linux_deps.exclude?("gcc") + variations = formula_hash["variations"] + # The formula has no variations, so all versions depend on GCC. + return false if variations.blank? + + # FIXME: This returns a false positive for formulae that do, for example: + # ```ruby + # on_system :linux, macos: :catalina_or_newer do + # depends_on "gcc" + # end + # ``` + variations_deps = [] + formula_hash["variations"].each_value do |data| variations_deps += data["dependencies"] end variations_deps.uniq! - linux_deps.include?("gcc") && variations_deps&.exclude?("gcc") + variations_deps.exclude?("gcc") end end end From 69fbaf2dbf001fdeaaf6491a9ff898766ce2a135 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 24 Aug 2022 15:12:58 +0800 Subject: [PATCH 3/8] Fix comment; reuse `variations`. --- Library/Homebrew/formula_auditor.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index f13a2caf8c..b226eaf598 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -879,7 +879,7 @@ module Homebrew return false if linux_deps.exclude?("gcc") variations = formula_hash["variations"] - # The formula has no variations, so all versions depend on GCC. + # The formula has no variations, so all OS-version-arch triples depend on GCC. return false if variations.blank? # FIXME: This returns a false positive for formulae that do, for example: @@ -889,7 +889,7 @@ module Homebrew # end # ``` variations_deps = [] - formula_hash["variations"].each_value do |data| + variations.each_value do |data| variations_deps += data["dependencies"] end variations_deps.uniq! From be4e926b15ec58ff6273e6bf9377cfc8d11e57f9 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 24 Aug 2022 18:18:10 +0800 Subject: [PATCH 4/8] Fix `"dependencies"` being `nil`. --- Library/Homebrew/formula_auditor.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index b226eaf598..f394d01dad 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -888,11 +888,10 @@ module Homebrew # depends_on "gcc" # end # ``` - variations_deps = [] - variations.each_value do |data| - variations_deps += data["dependencies"] - end - variations_deps.uniq! + variations_deps = variations.values + .flat_map { |data| data["dependencies"] } + .compact + .uniq variations_deps.exclude?("gcc") end From 8e09ec4bf4f4c025eb8b9aeadfbe86847442b8b9 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Wed, 24 Aug 2022 20:34:43 +0800 Subject: [PATCH 5/8] Handle `on_system` blocks. --- Library/Homebrew/formula_auditor.rb | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index f394d01dad..903fcebe73 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -882,18 +882,20 @@ module Homebrew # The formula has no variations, so all OS-version-arch triples depend on GCC. return false if variations.blank? - # FIXME: This returns a false positive for formulae that do, for example: - # ```ruby - # on_system :linux, macos: :catalina_or_newer do - # depends_on "gcc" - # end - # ``` - variations_deps = variations.values - .flat_map { |data| data["dependencies"] } - .compact - .uniq + MacOSVersions::SYMBOLS.each_key do |macos_version| + [:arm, :intel].each do |arch| + bottle_tag = Utils::Bottles::Tag.new(system: macos_version, arch: arch) + next unless bottle_tag.valid_combination? - variations_deps.exclude?("gcc") + variation = variations[bottle_tag.to_sym] + # This variation does not exist, so it must match Linux. + return false if variation.blank? + # We found a non-Linux variation that depends on GCC. + return false if variation["dependencies"]&.include?("gcc") + end + end + + true end end end From 378ff06f53259229cb51c7634fc33a084c4fc309 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Thu, 25 Aug 2022 20:44:27 +0800 Subject: [PATCH 6/8] Speed up `#linux_only_gcc_dep?`. `#to_hash_with_variations` is slow, let's avoid doing it unless needed. --- Library/Homebrew/formula_auditor.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index 903fcebe73..aafc5bfe5a 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -873,12 +873,9 @@ module Homebrew def linux_only_gcc_dep?(formula) odie "`#linux_only_gcc_dep?` works only on Linux!" if Homebrew::SimulateSystem.simulating_or_running_on_macos? + return false if formula.deps.map(&:name).exclude?("gcc") - formula_hash = formula.to_hash_with_variations - linux_deps = formula_hash["dependencies"] - return false if linux_deps.exclude?("gcc") - - variations = formula_hash["variations"] + variations = formula.to_hash_with_variations["variations"] # The formula has no variations, so all OS-version-arch triples depend on GCC. return false if variations.blank? From 6175b3fe4c9b00fe7ea06aa4dbff1d288b40db80 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Thu, 25 Aug 2022 20:46:57 +0800 Subject: [PATCH 7/8] Require `--git`. --- Library/Homebrew/formula_auditor.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index aafc5bfe5a..6cdf12b80d 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -409,7 +409,9 @@ module Homebrew end def audit_gcc_dependency + return unless @git return unless @core_tap + return unless formula.tap.git? # git log is required return unless Homebrew::SimulateSystem.simulating_or_running_on_linux? return unless linux_only_gcc_dep?(formula) From 84f544f08ff280ea1750a647933e4bb6420c5b22 Mon Sep 17 00:00:00 2001 From: Carlo Cabrera <30379873+carlocab@users.noreply.github.com> Date: Thu, 25 Aug 2022 20:54:19 +0800 Subject: [PATCH 8/8] Require git log only when not strict --- Library/Homebrew/formula_auditor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index 6cdf12b80d..bcff035602 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -411,7 +411,7 @@ module Homebrew def audit_gcc_dependency return unless @git return unless @core_tap - return unless formula.tap.git? # git log is required + return if !@strict && !formula.tap.git? # git log is required for non-strict audit return unless Homebrew::SimulateSystem.simulating_or_running_on_linux? return unless linux_only_gcc_dep?(formula)