From 2e994a9abbec71ea57ad3cf82813cfafee1d3342 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 24 Oct 2025 17:28:31 +0100 Subject: [PATCH 1/2] formula_auditor: audit `revision` and `compatibility_version` Add a new audit method to check if the `revision` and `compatibility_version` are consistent with each other. This ensures that when a formula's `compatibility_version` is increased, the `revision` of dependent formulas was also increased. Inversely, when a formula's `revision` is increased in the same PR as one of its dependencies, the `compatibility_version` of dependent formulae must be increased by 1. While we're here, DRY things up and improve some naming to me more readable/obvious. Co-authored-by: Rylan Polster --- Library/Homebrew/formula_auditor.rb | 215 +++++++--- Library/Homebrew/test/formula_auditor_spec.rb | 387 +++++++++++++----- 2 files changed, 440 insertions(+), 162 deletions(-) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index 336984a4e2ddb..af52ebbff621b 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -7,6 +7,7 @@ require "resource_auditor" require "utils/shared_audits" require "utils/output" +require "utils/git" module Homebrew # Auditor for checking common violations in {Formula}e. @@ -39,8 +40,7 @@ def initialize(formula, options = {}) @spdx_license_data = options[:spdx_license_data] @spdx_exception_data = options[:spdx_exception_data] @tap_audit = options[:tap_audit] - @previous_committed = {} - @newest_committed = {} + @committed_version_info_cache = {} end def audit_style @@ -858,12 +858,12 @@ def audit_stable_version current_version = formula.stable.version current_version_scheme = formula.version_scheme - previous_committed, newest_committed = committed_version_info + previous_version_info, origin_head_version_info = committed_version_info - if !newest_committed[:version].nil? && - current_version < newest_committed[:version] && - current_version_scheme == previous_committed[:version_scheme] - problem "Stable: version should not decrease (from #{newest_committed[:version]} to #{current_version})" + if (origin_head_version = origin_head_version_info[:version]) && + current_version < origin_head_version && + current_version_scheme == previous_version_info[:version_scheme] + problem "Stable: version should not decrease (from #{origin_head_version} to #{current_version})" end end @@ -871,29 +871,101 @@ def audit_revision new_formula_problem("New formulae should not define a revision.") if @new_formula && !formula.revision.zero? return unless @git - return unless formula.tap # skip formula not from core or any taps - return unless formula.tap.git? # git log is required + + tap = formula.tap + return if tap.nil? + return unless tap.git? return if formula.stable.blank? current_version = formula.stable.version current_revision = formula.revision - previous_committed, newest_committed = committed_version_info + previous_version_info, origin_head_version_info = committed_version_info + + previous_version = previous_version_info[:version] + previous_revision = previous_version_info[:revision] + origin_head_version = origin_head_version_info[:version] + origin_head_revision = origin_head_version_info[:revision] - if (previous_committed[:version] != newest_committed[:version] || - current_version != newest_committed[:version]) && - !current_revision.zero? && - current_revision == newest_committed[:revision] && - current_revision == previous_committed[:revision] + if (previous_version != origin_head_version || current_version != origin_head_version) && + !current_revision.zero? && current_revision == origin_head_revision && current_revision == previous_revision problem "`revision #{current_revision}` should be removed" - elsif current_version == previous_committed[:version] && - !previous_committed[:revision].nil? && - current_revision < previous_committed[:revision] - problem "`revision` should not decrease (from #{previous_committed[:revision]} to #{current_revision})" - elsif newest_committed[:revision] && - current_revision > (newest_committed[:revision] + 1) + elsif current_version == previous_version && previous_revision && current_revision < previous_revision + problem "`revision` should not decrease (from #{previous_revision} to #{current_revision})" + elsif origin_head_revision && current_revision > (origin_head_revision + 1) problem "`revision` should only increment by 1" end + + revision_increment = current_revision - previous_revision.to_i + return if revision_increment != 1 + + dependency_names = formula.recursive_dependencies.map(&:name) + return if dependency_names.empty? + + changed_dependency_paths = changed_formulae_paths(tap, only_names: dependency_names) + return if changed_dependency_paths.empty? + + missing_compatibility_bumps = changed_dependency_paths.filter_map do |path| + changed_formula = Formulary.factory(path) + # Each changed dependency must raise its compatibility_version by exactly one. + _, origin_head_dependency_version_info = committed_version_info(formula: changed_formula) + previous_compatibility_version = origin_head_dependency_version_info[:compatibility_version] || 0 + current_compatibility_version = changed_formula.compatibility_version || 0 + next if current_compatibility_version == previous_compatibility_version + 1 + + "#{changed_formula.name} (#{previous_compatibility_version} to #{current_compatibility_version})" + end + return if missing_compatibility_bumps.empty? + + problem "`revision` increased but recursive dependencies must increase `compatibility_version` by 1: " \ + "#{missing_compatibility_bumps.join(", ")}." + end + + def audit_compatibility_version + return unless @git + + tap = formula.tap + return if tap.nil? + return unless tap.git? + + _, origin_head_version_info = committed_version_info + return if origin_head_version_info.empty? + + previous_compatibility_version = origin_head_version_info[:compatibility_version] || 0 + current_compatibility_version = formula.compatibility_version || previous_compatibility_version + + if current_compatibility_version < previous_compatibility_version + problem "`compatibility_version` should not decrease " \ + "from #{previous_compatibility_version} to #{current_compatibility_version}" + return + elsif current_compatibility_version > (previous_compatibility_version + 1) + problem "`compatibility_version` should only increment by 1" + return + end + + compatibility_increment = current_compatibility_version - previous_compatibility_version + return if compatibility_increment.zero? + + dependent_revision_bumps = changed_formulae_paths(tap).filter_map do |path| + changed_formula = Formulary.factory(path) + next if changed_formula.name == formula.name + + dependencies = changed_formula.recursive_dependencies.map(&:name) + # Only formulae that depend (recursively) on the audited formula can justify the bump. + next unless dependencies.include?(formula.name) + + _, origin_head_dependent_version_info = committed_version_info(formula: changed_formula) + previous_revision = origin_head_dependent_version_info[:revision] || 0 + current_revision = changed_formula.revision + next if current_revision != previous_revision + 1 + + changed_formula.name + end + return if dependent_revision_bumps.any? + + problem "`compatibility_version` increased from #{previous_compatibility_version} to " \ + "#{current_compatibility_version} but no recursive dependent formulae increased " \ + "`revision` by 1 in this PR." end def audit_version_scheme @@ -904,14 +976,13 @@ def audit_version_scheme current_version_scheme = formula.version_scheme - previous_committed, = committed_version_info + _, origin_head_version_info = committed_version_info + previous_version_scheme = origin_head_version_info[:version_scheme] + return if previous_version_scheme.nil? - return if previous_committed[:version_scheme].nil? - - if current_version_scheme < previous_committed[:version_scheme] - problem "`version_scheme` should not decrease (from #{previous_committed[:version_scheme]} " \ - "to #{current_version_scheme})" - elsif current_version_scheme > (previous_committed[:version_scheme] + 1) + if current_version_scheme < previous_version_scheme + problem "`version_scheme` should not decrease (from #{previous_version_scheme} to #{current_version_scheme})" + elsif current_version_scheme > (previous_version_scheme + 1) problem "`version_scheme` should only increment by 1" end end @@ -926,12 +997,13 @@ def audit_unconfirmed_checksum_change current_checksum = formula.stable.checksum current_url = formula.stable.url - _, newest_committed = committed_version_info + _, origin_head_version_info = committed_version_info + origin_head_checksum = origin_head_version_info[:checksum] - if current_version == newest_committed[:version] && - current_url == newest_committed[:url] && - current_checksum != newest_committed[:checksum] && - current_checksum.present? && newest_committed[:checksum].present? + if current_version == origin_head_version_info[:version] && + current_url == origin_head_version_info[:url] && + current_checksum.present? && origin_head_checksum.present? && + current_checksum != origin_head_checksum problem( "stable sha256 changed without the url/version also changing; " \ "please create an issue upstream to rule out malicious " \ @@ -1058,12 +1130,47 @@ def linux_only_gcc_dep?(formula) true end - def committed_version_info - return [] unless @git - return [] unless formula.tap # skip formula not from core or any taps - return [] unless formula.tap.git? # git log is required - return [] if formula.stable.blank? - return [@previous_committed, @newest_committed] if @previous_committed.present? || @newest_committed.present? + sig { params(tap: Tap, only_names: T::Array[String]).returns(T::Array[Pathname]) } + def changed_formulae_paths(tap, only_names: [].freeze) + return [] unless tap.git? + + base_ref = "origin/HEAD" + changed_paths = Utils.safe_popen_read(Utils::Git.git, "-C", tap.path, "diff", "--name-only", base_ref) + .lines + .filter_map do |line| + relative_path = line.chomp + next unless relative_path.end_with?(".rb") + + absolute_path = tap.path/relative_path + next unless absolute_path.exist? + next unless absolute_path.to_s.start_with?(tap.formula_dir.to_s) + + absolute_path + end + return changed_paths if only_names.blank? + + expected_paths = only_names.filter_map do |name| + relative_path = Pathname(name.to_s.delete_prefix("#{tap.name}/")) + relative_path = relative_path.sub_ext(".rb") if relative_path.extname.empty? + absolute_path = tap.formula_dir/relative_path + absolute_path.expand_path if absolute_path.exist? + end.map(&:to_s) + + changed_paths.select { |path| expected_paths.include?(path.expand_path.to_s) } + end + + sig { params(formula: Formula).returns([T::Hash[Symbol, T.untyped], T::Hash[Symbol, T.untyped]]) } + def committed_version_info(formula: @formula) + empty_result = [{}, {}] + return empty_result unless @git + return empty_result unless formula.tap # skip formula not from core or any taps + return empty_result unless formula.tap.git? # git log is required + return empty_result if formula.stable.blank? + + return @committed_version_info_cache[formula.full_name] if @committed_version_info_cache.key?(formula.full_name) + + previous_version_info = {} + origin_head_version_info = {} current_version = formula.stable.version current_revision = formula.revision @@ -1075,28 +1182,30 @@ def committed_version_info stable = f.stable next if stable.blank? - @previous_committed[:version] = stable.version - @previous_committed[:checksum] = stable.checksum - @previous_committed[:version_scheme] = f.version_scheme - @previous_committed[:revision] = f.revision - - @newest_committed[:version] ||= @previous_committed[:version] - @newest_committed[:checksum] ||= @previous_committed[:checksum] - @newest_committed[:revision] ||= @previous_committed[:revision] - @newest_committed[:url] ||= stable.url + previous_version_info[:version] = stable.version + previous_version_info[:checksum] = stable.checksum + previous_version_info[:revision] = f.revision + previous_version_info[:version_scheme] = f.version_scheme + previous_version_info[:compatibility_version] = f.compatibility_version + + origin_head_version_info[:url] ||= stable.url + origin_head_version_info[:version] ||= previous_version_info[:version] + origin_head_version_info[:checksum] ||= previous_version_info[:checksum] + origin_head_version_info[:revision] ||= previous_version_info[:revision] + origin_head_version_info[:compatibility_version] ||= previous_version_info[:compatibility_version] end rescue MacOSVersion::Error break end - break if @previous_committed[:version] && current_version != @previous_committed[:version] - break if @previous_committed[:revision] && current_revision != @previous_committed[:revision] + break if previous_version_info[:version] && current_version != previous_version_info[:version] + break if previous_version_info[:revision] && current_revision != previous_version_info[:revision] end - @previous_committed.compact! - @newest_committed.compact! + previous_version_info.compact! + origin_head_version_info.compact! - [@previous_committed, @newest_committed] + @committed_version_info_cache[formula.full_name] = [previous_version_info, origin_head_version_info] end end end diff --git a/Library/Homebrew/test/formula_auditor_spec.rb b/Library/Homebrew/test/formula_auditor_spec.rb index abee6896f7ced..26de51c8e7f4d 100644 --- a/Library/Homebrew/test/formula_auditor_spec.rb +++ b/Library/Homebrew/test/formula_auditor_spec.rb @@ -1,9 +1,12 @@ # frozen_string_literal: true require "formula_auditor" +require "git_repository" +require "securerandom" RSpec.describe Homebrew::FormulaAuditor do include FileUtils + include Test::Helper::Formula let(:dir) { mktmpdir } let(:foo_version) do @@ -41,6 +44,26 @@ def formula_gsub(before, after = "") formula_path.write text end + def test_formula_source(name:, compatibility_version: nil, revision: 0, depends_on: []) + class_name = name.gsub(/[^0-9a-z]/i, "_").split("_").reject(&:empty?).map(&:capitalize).join + class_name = "TestFormula#{SecureRandom.hex(2)}" if class_name.empty? + + lines = [] + lines << "class #{class_name} < Formula" + lines << ' desc "Test formula"' + lines << ' homepage "https://brew.sh"' + lines << %Q( url "https://brew.sh/#{name}-1.0.tar.gz") + lines << ' sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"' + lines << " compatibility_version #{compatibility_version}" if compatibility_version + lines << " revision #{revision}" if revision.positive? + Array(depends_on).each { |dep| lines << %Q( depends_on "#{dep}") } + lines << " def install" + lines << " bin.mkpath" + lines << " end" + lines << "end" + "#{lines.join("\n")}\n" + end + def formula_gsub_origin_commit(before, after = "") text = origin_formula_path.read text.gsub!(before, after) @@ -955,6 +978,7 @@ class Foo < Formula fa.problems.first&.fetch(:message) end + # Mock tap behaviour the Formula helper expects (e.g. PyPI lookups, audit exceptions). before do origin_formula_path.dirname.mkpath origin_formula_path.write <<~RUBY @@ -1007,7 +1031,7 @@ class Foo#{foo_version} < Formula end end - describe "#audit_revision" do + describe "#audit_revision dependency relationships" do subject do fa = described_class.new(Formulary.factory(formula_path), git: true) fa.audit_revision @@ -1131,154 +1155,299 @@ class Foo < Formula end end - describe "#audit_version_scheme" do - subject do - fa = described_class.new(Formulary.factory(formula_path), git: true) - fa.audit_version_scheme - fa.problems.first&.fetch(:message) + def build_formula_for_audit(tap:, tap_path:, name:, compatibility_version: nil, revision: 0, depends_on: []) + path = tap_path/"Formula/#{name}.rb" + path.dirname.mkpath + path.write(test_formula_source(name:, compatibility_version:, revision:, depends_on:)) + + Formulary.clear_cache + formula = Formulary.factory(path) + allow(formula).to receive_messages(tap:, full_name: "#{tap.name}/#{name}") + + formula + end + + def dependency_stub(name) + instance_double(Dependency, name:) + end + + def stub_committed_info(auditor, default:, overrides: {}) + allow(auditor).to receive(:committed_version_info) do |*_args, **kwargs| + formula = kwargs.fetch(:formula, auditor.formula) + raw = overrides.fetch(formula, default) + committed = raw.map { |info| info ? info.dup : {} } + committed.each do |info| + info[:version] ||= formula.stable&.version + info[:revision] = info.fetch(:revision, formula.revision) + info[:compatibility_version] = info.fetch(:compatibility_version, formula.compatibility_version) + info[:version_scheme] = info.fetch(:version_scheme, formula.version_scheme) + end + committed.each(&:compact!) + committed end + end + + def stub_changed_paths(auditor, all_paths:, filtered_paths: all_paths) + allow(auditor).to receive(:changed_formulae_paths) do |_tap_arg, only_names: nil| + only_names ? filtered_paths : all_paths + end + end + + describe "#audit_compatibility_version" do + let(:tap_path) { Pathname("#{dir}/compat-tap") } + let(:tap) do + instance_double( + Tap, + git?: true, + core_tap?: false, + git_repository: instance_double(GitRepository, origin_branch_name: "main"), + audit_exceptions: {}, + formula_renames: {}, + path: tap_path, + name: "test/tap", + ) + end + let(:current_compatibility_version) { 2 } + let(:target_formula) do + build_formula_for_audit( + tap:, + tap_path:, + name: "foo", + compatibility_version: current_compatibility_version, + ) + end + let(:auditor) { described_class.new(target_formula, git: true) } + let(:foo_path) { tap_path/"Formula/foo.rb" } + let(:bar_path) { tap_path/"Formula/bar.rb" } before do - origin_formula_path.dirname.mkpath - origin_formula_path.write <<~RUBY - class Foo#{foo_version} < Formula - url "https://brew.sh/foo-1.0.tar.gz" - sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e" - revision 2 - version_scheme 1 - end - RUBY + allow(tap).to receive_messages(formula_dir: tap_path/"Formula", pypi_formula_mappings: {}) + allow(target_formula).to receive_messages(full_name: "test/tap/foo", recursive_dependencies: []) + allow(Formulary).to receive(:factory).and_call_original + allow(Formulary).to receive(:factory).with(foo_path).and_return(target_formula) + end - origin_tap_path.mkpath - origin_tap_path.cd do - system "git", "init" - system "git", "add", "--all" - system "git", "commit", "-m", "init" + it "ignores formulae without a previous commit" do + stub_committed_info(auditor, default: [{}, {}]) + stub_changed_paths(auditor, all_paths: []) + + auditor.audit_compatibility_version + + expect(auditor.problems).to be_empty + end + + context "with existing committed compatibility_version" do + before do + stub_changed_paths(auditor, all_paths: []) end - tap_path.mkpath - tap_path.cd do - system "git", "clone", origin_tap_path, "." + it "flags decreases" do + stub_committed_info( + auditor, + default: [{ compatibility_version: 2 }, { compatibility_version: 2 }], + ) + allow(target_formula).to receive(:compatibility_version).and_return(1) + + auditor.audit_compatibility_version + + expect(auditor.problems).to include( + a_hash_including(message: a_string_matching(/should not decrease/)), + ) + end + + it "flags increments larger than one" do + allow(target_formula).to receive(:compatibility_version).and_return(3) + stub_committed_info( + auditor, + default: [{ compatibility_version: 1 }, { compatibility_version: 1 }], + ) + + auditor.audit_compatibility_version + + expect(auditor.problems).to include( + a_hash_including(message: a_string_matching(/should only increment by 1/)), + ) + end + + it "allows unchanged compatibility_version" do + allow(target_formula).to receive(:compatibility_version).and_return(1) + stub_committed_info( + auditor, + default: [{ compatibility_version: 1 }, { compatibility_version: 1 }], + ) + + auditor.audit_compatibility_version + + expect(auditor.problems).to be_empty end end - describe "version_schemes" do - describe "should not decrease with the same version" do - before { formula_gsub_origin_commit "version_scheme 1" } + context "when compatibility_version increments by one" do + let(:dependent_formula) do + build_formula_for_audit( + tap:, + tap_path:, + name: "bar", + revision: 2, + depends_on: ["foo"], + ) + end - it { is_expected.to match("`version_scheme` should not decrease (from 1 to 0)") } + before do + allow(dependent_formula).to receive_messages(full_name: "test/tap/bar", + recursive_dependencies: [dependency_stub("foo")]) + allow(Formulary).to receive(:factory).with(bar_path).and_return(dependent_formula) + stub_changed_paths(auditor, all_paths: [foo_path, bar_path]) end - describe "should not decrease with a new version" do - before do - formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub_origin_commit "revision 2", "" - formula_gsub_origin_commit "version_scheme 1", "" - end + it "flags missing dependent revision bumps" do + stub_committed_info( + auditor, + default: [{ compatibility_version: 1 }, { compatibility_version: 1 }], + overrides: { dependent_formula => [{ revision: 2 }, { revision: 2 }] }, + ) + + auditor.audit_compatibility_version - it { is_expected.to match("`version_scheme` should not decrease (from 1 to 0)") } + expect(auditor.problems).to include( + a_hash_including(message: a_string_matching(/no recursive dependent formulae increased `revision` by 1/)), + ) end - describe "should only increment by 1" do - before do - formula_gsub_origin_commit "version_scheme 1", "# no version_scheme" - formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub_origin_commit "revision 2", "" - formula_gsub_origin_commit "# no version_scheme", "version_scheme 3" - end + it "accepts a dependent revision bump" do + stub_committed_info( + auditor, + default: [{ compatibility_version: 1 }, { compatibility_version: 1 }], + overrides: { dependent_formula => [{ revision: 1 }, { revision: 1 }] }, + ) - it { is_expected.to match("`version_scheme` should only increment by 1") } + auditor.audit_compatibility_version + + expect(auditor.problems).to be_empty end end end - describe "#audit_unconfirmed_checksum_change" do - subject do - fa = described_class.new(Formulary.factory(formula_path), git: true) - fa.audit_unconfirmed_checksum_change - fa.problems.first&.fetch(:message) + describe "#audit_revision" do + let(:tap_path) { Pathname("#{dir}/revision-tap") } + let(:tap) do + instance_double( + Tap, + git?: true, + core_tap?: false, + git_repository: instance_double(GitRepository, origin_branch_name: "main"), + audit_exceptions: {}, + formula_renames: {}, + path: tap_path, + name: "test/tap", + ) + end + let(:current_revision) { 2 } + let(:dependency_names) { ["foo"] } + let(:dependency_list) { dependency_names.map { |name| dependency_stub(name) } } + let(:target_formula) do + build_formula_for_audit( + tap:, + tap_path:, + name: "bar", + revision: current_revision, + depends_on: dependency_names, + ) + end + let(:auditor) { described_class.new(target_formula, git: true) } + let(:bar_path) { tap_path/"Formula/bar.rb" } + let(:foo_path) { tap_path/"Formula/foo.rb" } + let(:current_dependency_compatibility) { 1 } + let(:dependency_revision) { 0 } + let(:dependency_formula) do + build_formula_for_audit( + tap:, + tap_path:, + name: "foo", + compatibility_version: current_dependency_compatibility, + revision: dependency_revision, + ) end before do - origin_formula_path.dirname.mkpath - origin_formula_path.write <<~RUBY - class Foo#{foo_version} < Formula - url "https://brew.sh/foo-1.0.tar.gz" - sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e" - revision 2 - version_scheme 1 - end - RUBY + allow(tap).to receive_messages(formula_dir: tap_path/"Formula", pypi_formula_mappings: {}) + allow(target_formula).to receive_messages(full_name: "test/tap/bar", recursive_dependencies: dependency_list) + allow(Formulary).to receive(:factory).and_call_original + allow(Formulary).to receive(:factory).with(bar_path).and_return(target_formula) + allow(Formulary).to receive(:factory).with(foo_path).and_return(dependency_formula) + end - origin_tap_path.mkpath - origin_tap_path.cd do - system "git", "init" - system "git", "add", "--all" - system "git", "commit", "-m", "init" - end + it "ignores revision changes when not incremented by one" do + stub_committed_info( + auditor, + default: [{ revision: current_revision }, { revision: current_revision }], + ) + stub_changed_paths(auditor, all_paths: [], filtered_paths: []) - tap_path.mkpath - tap_path.cd do - system "git", "clone", origin_tap_path, "." - end + auditor.audit_revision + + expect(auditor.problems).to be_empty end - describe "checksums" do - describe "should not change with the same version" do - before do - formula_gsub( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - end + context "with a revision increment" do + before do + stub_committed_info( + auditor, + default: [{ revision: current_revision - 1 }, { revision: current_revision - 1 }], + ) + end - it { is_expected.to match("stable sha256 changed without the url/version also changing") } + it "allows revision increases when there are no recursive dependencies" do + allow(target_formula).to receive(:recursive_dependencies).and_return([]) + stub_changed_paths(auditor, all_paths: [], filtered_paths: []) + + auditor.audit_revision + + expect(auditor.problems).to be_empty end - describe "should not change with the same version when not the first commit" do - before do - formula_gsub_origin_commit( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - formula_gsub_origin_commit "revision 2" - formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub( - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', - ) - end + it "allows revision increases when dependencies are unchanged" do + stub_changed_paths(auditor, all_paths: [], filtered_paths: []) + + auditor.audit_revision - it { is_expected.to match("stable sha256 changed without the url/version also changing") } + expect(auditor.problems).to be_empty end - describe "can change with the different version" do + context "when dependencies change" do before do - formula_gsub_origin_commit( - 'sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - ) - formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz" - formula_gsub_origin_commit( - 'sha256 "3622d2a53236ed9ca62de0616a7e80fd477a9a3f862ba09d503da188f53ca523"', - 'sha256 "e048c5e6144f5932d8672c2fade81d9073d5b3ca1517b84df006de3d25414fc1"', - ) + stub_changed_paths(auditor, all_paths: [foo_path], filtered_paths: [foo_path]) end - it { is_expected.to be_nil } - end + it "flags missing compatibility_version bumps" do + stub_committed_info( + auditor, + default: [{ revision: current_revision - 1 }, { revision: current_revision - 1 }], + overrides: { dependency_formula => [{ compatibility_version: 1 }, { compatibility_version: 1 }] }, + ) + allow(dependency_formula).to receive(:compatibility_version).and_return(1) - describe "can be removed when switching schemes" do - before do - formula_gsub_origin_commit( - 'url "https://brew.sh/foo-1.0.tar.gz"', - 'url "https://foo.com/brew/bar.git", tag: "1.0", revision: "f5e00e485e7aa4c5baa20355b27e3b84a6912790"', + auditor.audit_revision + + expect(auditor.problems).to include( + a_hash_including( + message: a_string_matching(/must increase `compatibility_version` by 1: foo \(1 to 1\)/), + ), ) - formula_gsub_origin_commit('sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"', - "") end - it { is_expected.to be_nil } + it "accepts compatibility_version bumps of one" do + stub_committed_info( + auditor, + default: [{ revision: current_revision - 1 }, { revision: current_revision - 1 }], + overrides: { dependency_formula => [{ compatibility_version: 1 }, { compatibility_version: 1 }] }, + ) + allow(dependency_formula).to receive(:compatibility_version).and_return(2) + + auditor.audit_revision + + expect(auditor.problems).to be_empty + end end end end From 2cd8da4e39b1f3a6196ddd3895c7994c6a49f38e Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 14 Nov 2025 08:54:09 +0000 Subject: [PATCH 2/2] formula_auditor: add a version_scheme. Co-authored-by: Rylan Polster --- Library/Homebrew/formula_auditor.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/Library/Homebrew/formula_auditor.rb b/Library/Homebrew/formula_auditor.rb index af52ebbff621b..5935afa0fef27 100644 --- a/Library/Homebrew/formula_auditor.rb +++ b/Library/Homebrew/formula_auditor.rb @@ -1192,6 +1192,7 @@ def committed_version_info(formula: @formula) origin_head_version_info[:version] ||= previous_version_info[:version] origin_head_version_info[:checksum] ||= previous_version_info[:checksum] origin_head_version_info[:revision] ||= previous_version_info[:revision] + origin_head_version_info[:version_scheme] ||= previous_version_info[:version_scheme] origin_head_version_info[:compatibility_version] ||= previous_version_info[:compatibility_version] end rescue MacOSVersion::Error