Skip to content

Commit ea885ba

Browse files
committed
formula_auditor: audit no_autobump! stanza on version bump
I skipped cask auditor as there is no casks that use `:requires_manual_review` Signed-off-by: botantony <antonsm21@gmail.com>
1 parent ea0351c commit ea885ba

File tree

4 files changed

+92
-24
lines changed

4 files changed

+92
-24
lines changed

Library/Homebrew/cask/audit.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1242,7 +1242,7 @@ def audit_no_autobump
12421242
return if cask.autobump?
12431243
return unless new_cask?
12441244

1245-
error = SharedAudits.no_autobump_new_package_message(cask.no_autobump_message)
1245+
error = SharedAudits.no_autobump_audit_message(cask.no_autobump_message, new_package: true)
12461246
add_error error if error
12471247
end
12481248

Library/Homebrew/formula_auditor.rb

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,10 +1081,27 @@ def audit_deprecate_disable
10811081
def audit_no_autobump
10821082
return if formula.autobump?
10831083

1084-
return unless @new_formula_inclusive
1084+
if @new_formula_inclusive
1085+
error = SharedAudits.no_autobump_audit_message(formula.no_autobump_message, new_package: true)
1086+
new_formula_problem error if error
1087+
return
1088+
end
10851089

1086-
error = SharedAudits.no_autobump_new_package_message(formula.no_autobump_message)
1087-
new_formula_problem error if error
1090+
return unless @git
1091+
return unless formula.tap
1092+
return unless formula.tap.git?
1093+
return if formula.stable.blank?
1094+
1095+
current_version = formula.stable.version
1096+
1097+
_, origin_head_version_info = committed_version_info
1098+
return if origin_head_version_info.empty?
1099+
1100+
# Check `no_autobump!` if on version change
1101+
return if current_version == origin_head_version_info[:version]
1102+
1103+
error = SharedAudits.no_autobump_audit_message(formula.no_autobump_message)
1104+
problem error if error
10881105
end
10891106

10901107
def quote_dep(dep)

Library/Homebrew/test/formula_auditor_spec.rb

Lines changed: 66 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,32 +1728,81 @@ class Foo < Formula
17281728
end
17291729

17301730
describe "#audit_no_autobump" do
1731-
it "warns when autobump exclusion reason is not suitable for new formula" do
1732-
fa = formula_auditor "foo", <<~RUBY, new_formula: true
1733-
class Foo < Formula
1734-
url "https://brew.sh/foo-1.0.tgz"
1731+
subject(:no_autobump_audit) do
1732+
fa = described_class.new(Formulary.factory(formula_path), git: true)
1733+
fa.audit_no_autobump
1734+
fa.problems.first&.fetch(:message)
1735+
end
1736+
1737+
before do
1738+
origin_formula_path.dirname.mkpath
1739+
origin_formula_path.write <<~RUBY
1740+
class Foo#{foo_version} < Formula
1741+
url "https://brew.sh/foo-1.0.tar.gz"
1742+
sha256 "31cccfc6630528db1c8e3a06f6decf2a370060b982841cfab2b8677400a5092e"
17351743
1736-
no_autobump! because: :requires_manual_review
1744+
no_autobump! because: "no_autobump_placeholder"
17371745
end
17381746
RUBY
17391747

1740-
fa.audit_no_autobump
1741-
expect(fa.new_formula_problems.first[:message])
1742-
.to match("`:requires_manual_review` is a temporary reason intended for existing packages, " \
1743-
"use a different reason instead.")
1748+
origin_tap_path.mkpath
1749+
origin_tap_path.cd do
1750+
system "git", "init"
1751+
system "git", "add", "--all"
1752+
system "git", "commit", "-m", "init"
1753+
end
1754+
1755+
tap_path.mkpath
1756+
tap_path.cd do
1757+
system "git", "clone", origin_tap_path, "."
1758+
end
17441759
end
17451760

1746-
it "does not warn when autobump exclusion reason is allowed" do
1747-
fa = formula_auditor "foo", <<~RUBY, new_formula: true
1748-
class Foo < Formula
1749-
url "https://brew.sh/foo-1.0.tgz"
1761+
describe "no_autobump" do
1762+
context "when version is changed" do
1763+
before do
1764+
formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz"
1765+
formula_gsub '"no_autobump_placeholder"', ":requires_manual_review"
1766+
end
17501767

1751-
no_autobump! because: "foo bar"
1768+
it do
1769+
expect(no_autobump_audit).to match("`:requires_manual_review` is a temporary to-be deprecated reason, " \
1770+
"change or remove autobump exclusion reason.")
17521771
end
1753-
RUBY
1772+
end
17541773

1755-
fa.audit_no_autobump
1756-
expect(fa.new_formula_problems).to be_empty
1774+
context "when version is not changed" do
1775+
before { formula_gsub '"no_autobump_placeholder"', ":requires_manual_review" }
1776+
1777+
it { is_expected.to be_nil }
1778+
end
1779+
1780+
context "when version is changed but the autobump exclusion reason is not `:requires_manual_review`" do
1781+
before { formula_gsub "foo-1.0.tar.gz", "foo-1.1.tar.gz" }
1782+
1783+
it { is_expected.to be_nil }
1784+
end
1785+
1786+
context "when autobump exclusion reason is not `:requires_manual_review`" do
1787+
it { is_expected.to be_nil }
1788+
end
1789+
1790+
context "when a new formula is created" do
1791+
it "warns when autobump exclusion reason is not suitable for new formula" do
1792+
fa = formula_auditor "foo", <<~RUBY, new_formula: true
1793+
class Foo < Formula
1794+
url "https://brew.sh/foo-1.0.tgz"
1795+
1796+
no_autobump! because: :requires_manual_review
1797+
end
1798+
RUBY
1799+
1800+
fa.audit_no_autobump
1801+
expect(fa.new_formula_problems.first[:message])
1802+
.to match("`:requires_manual_review` is a temporary to-be deprecated reason, " \
1803+
"use a different reason instead.")
1804+
end
1805+
end
17571806
end
17581807
end
17591808
end

Library/Homebrew/utils/shared_audits.rb

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,10 +368,12 @@ def self.check_deprecate_disable_reason(formula_or_cask)
368368
"#{reason} is not a valid deprecate! or disable! reason" unless reasons.include?(reason)
369369
end
370370

371-
sig { params(message: T.any(String, Symbol)).returns(T.nilable(String)) }
372-
def self.no_autobump_new_package_message(message)
371+
sig { params(message: T.any(String, Symbol), new_package: T::Boolean).returns(T.nilable(String)) }
372+
def self.no_autobump_audit_message(message, new_package: false)
373373
return if message.is_a?(String) || message != :requires_manual_review
374374

375-
"`:requires_manual_review` is a temporary reason intended for existing packages, use a different reason instead."
375+
msg = new_package ? "use a different reason instead" : "change or remove autobump exclusion reason"
376+
377+
"`:requires_manual_review` is a temporary to-be deprecated reason, #{msg}."
376378
end
377379
end

0 commit comments

Comments
 (0)