FormulaAuditor: Separate stable version audit

The "stable version should not decrease" formula audit currently
prevents us from being able to create bottles when downgrading a
formula version. We previously worked around this by bumping
`version_scheme` but this wasn't an intended use case and we now
avoid using it for this purpose.

We can handle simple formula downgrades by reverting changes in a
syntax-only PR but that isn't sufficient when we need new bottles
(i.e., if additional changes have been made to the formula in the
interim time). In the latter case, the only available solution may be
to revert all changes made after the previous version using a
syntax-only PR and then create another PR to reintroduce the other
changes and create new bottles.

To avoid the aforementioned approach, this splits the stable version
audit into a separate method, so we can use `brew audit
--except=stable_version` to selectively skip it.
This commit is contained in:
Sam Ford 2023-12-13 14:53:35 -05:00
parent af7f12b1bd
commit ccbde5952d
No known key found for this signature in database
GPG Key ID: 7AF5CBEE1DD6F76D
2 changed files with 134 additions and 66 deletions

View File

@ -777,6 +777,50 @@ module Homebrew
end
end
def audit_stable_version
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?
current_version = formula.stable.version
current_version_scheme = formula.version_scheme
current_revision = formula.revision
previous_version = T.let(nil, T.nilable(Version))
previous_version_scheme = T.let(nil, T.nilable(Integer))
previous_revision = T.let(nil, T.nilable(Integer))
newest_committed_version = T.let(nil, T.nilable(Version))
fv = FormulaVersions.new(formula)
fv.rev_list("origin/HEAD") do |revision, path|
begin
fv.formula_at_revision(revision, path) do |f|
stable = f.stable
next if stable.blank?
previous_version = stable.version
previous_version_scheme = f.version_scheme
previous_revision = f.revision
newest_committed_version ||= previous_version
end
rescue MacOSVersion::Error
break
end
break if previous_version && current_version != previous_version
break if previous_revision && current_revision != previous_revision
end
if !newest_committed_version.nil? &&
current_version < newest_committed_version &&
current_version_scheme == previous_version_scheme
problem "stable version should not decrease (from #{newest_committed_version} to #{current_version})"
end
end
def audit_revision_and_version_scheme
new_formula_problem("New formulae should not define a revision.") if @new_formula && !formula.revision.zero?
@ -785,8 +829,6 @@ module Homebrew
return unless formula.tap.git? # git log is required
return if formula.stable.blank?
fv = FormulaVersions.new(formula)
current_version = formula.stable.version
current_checksum = formula.stable.checksum
current_version_scheme = formula.version_scheme
@ -802,6 +844,7 @@ module Homebrew
newest_committed_revision = T.let(nil, T.nilable(Integer))
newest_committed_url = T.let(nil, T.nilable(String))
fv = FormulaVersions.new(formula)
fv.rev_list("origin/HEAD") do |revision, path|
begin
fv.formula_at_revision(revision, path) do |f|
@ -837,12 +880,6 @@ module Homebrew
)
end
if !newest_committed_version.nil? &&
current_version < newest_committed_version &&
current_version_scheme == previous_version_scheme
problem "stable version should not decrease (from #{newest_committed_version} to #{current_version})"
end
unless previous_version_scheme.nil?
if current_version_scheme < previous_version_scheme
problem "version_scheme should not decrease (from #{previous_version_scheme} " \

View File

@ -57,6 +57,14 @@ module Homebrew
end
describe FormulaAuditor do
let(:dir) { mktmpdir }
let(:foo_version) { Count.increment }
let(:formula_subpath) { "Formula/foo#{foo_version}.rb" }
let(:origin_tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" }
let(:origin_formula_path) { origin_tap_path/formula_subpath }
let(:tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-bar" }
let(:formula_path) { tap_path/formula_subpath }
def formula_auditor(name, text, options = {})
path = Pathname.new "#{dir}/#{name}.rb"
path.open("w") do |f|
@ -75,7 +83,28 @@ module Homebrew
described_class.new(formula, options)
end
let(:dir) { mktmpdir }
def formula_gsub(before, after = "")
text = formula_path.read
text.gsub! before, after
formula_path.unlink
formula_path.write text
end
def formula_gsub_origin_commit(before, after = "")
text = origin_formula_path.read
text.gsub!(before, after)
origin_formula_path.unlink
origin_formula_path.write text
origin_tap_path.cd do
system "git", "commit", "-am", "commit"
end
tap_path.cd do
system "git", "fetch"
system "git", "reset", "--hard", "origin/HEAD"
end
end
describe "#problems" do
it "is empty by default" do
@ -873,6 +902,65 @@ module Homebrew
end
end
describe "#audit_stable_version" do
subject do
fa = described_class.new(Formulary.factory(formula_path), git: true)
fa.audit_stable_version
fa.problems.first&.fetch(:message)
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
origin_tap_path.mkpath
origin_tap_path.cd do
system "git", "init"
system "git", "add", "--all"
system "git", "commit", "-m", "init"
end
tap_path.mkpath
tap_path.cd do
system "git", "clone", origin_tap_path, "."
end
end
describe "versions" do
context "when uncommitted should not decrease" do
before { formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz" }
it { is_expected.to match("stable version should not decrease (from 1.0 to 0.9)") }
end
context "when committed can decrease" do
before do
formula_gsub_origin_commit "revision 2"
formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-0.9.tar.gz"
end
it { is_expected.to be_nil }
end
describe "can decrease with version_scheme increased" do
before do
formula_gsub "revision 2"
formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz"
formula_gsub "version_scheme 1", "version_scheme 2"
end
it { is_expected.to be_nil }
end
end
end
describe "#audit_revision_and_version_scheme" do
subject do
fa = described_class.new(Formulary.factory(formula_path), git: true)
@ -880,13 +968,6 @@ module Homebrew
fa.problems.first&.fetch(:message)
end
let(:origin_tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-foo" }
let(:foo_version) { Count.increment }
let(:formula_subpath) { "Formula/foo#{foo_version}.rb" }
let(:origin_formula_path) { origin_tap_path/formula_subpath }
let(:tap_path) { Tap::TAP_DIRECTORY/"homebrew/homebrew-bar" }
let(:formula_path) { tap_path/formula_subpath }
before do
origin_formula_path.dirname.mkpath
origin_formula_path.write <<~RUBY
@ -928,29 +1009,6 @@ module Homebrew
end
end
def formula_gsub(before, after = "")
text = formula_path.read
text.gsub! before, after
formula_path.unlink
formula_path.write text
end
def formula_gsub_origin_commit(before, after = "")
text = origin_formula_path.read
text.gsub!(before, after)
origin_formula_path.unlink
origin_formula_path.write text
origin_tap_path.cd do
system "git", "commit", "-am", "commit"
end
tap_path.cd do
system "git", "fetch"
system "git", "reset", "--hard", "origin/HEAD"
end
end
describe "checksums" do
describe "should not change with the same version" do
before do
@ -1113,33 +1171,6 @@ module Homebrew
it { is_expected.to match("version_schemes should only increment by 1") }
end
end
describe "versions" do
context "when uncommitted should not decrease" do
before { formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz" }
it { is_expected.to match("stable version should not decrease (from 1.0 to 0.9)") }
end
context "when committed can decrease" do
before do
formula_gsub_origin_commit "revision 2"
formula_gsub_origin_commit "foo-1.0.tar.gz", "foo-0.9.tar.gz"
end
it { is_expected.to be_nil }
end
describe "can decrease with version_scheme increased" do
before do
formula_gsub "revision 2"
formula_gsub "foo-1.0.tar.gz", "foo-0.9.tar.gz"
formula_gsub "version_scheme 1", "version_scheme 2"
end
it { is_expected.to be_nil }
end
end
end
describe "#audit_versioned_keg_only" do