Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion Library/Homebrew/formula_auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -754,6 +754,15 @@ def get_repo_data(regex)
def audit_specs
problem "HEAD-only (no stable download)" if head_only?(formula) && @core_tap

allowed_pypi_packages = if (resources_allowlist = formula
.tap
&.audit_exception(:pypi_resources_allowlist, formula.name)
.presence)
Comment thread
botantony marked this conversation as resolved.
Outdated
Set.new(resources_allowlist.split(/\s+/i))
else
Set.new
end

%w[Stable HEAD].each do |name|
spec_name = name.downcase.to_sym
next unless (spec = formula.send(spec_name))
Expand All @@ -776,9 +785,15 @@ def audit_specs
spec.resources.each_value do |resource|
problem "Resource name should be different from the formula name" if resource.name == formula.name

except = @except
if allowed_pypi_packages.include?(resource.name) || formula.deprecated?
except = [*Array(except), "pypi_resources"]
end

ra = ResourceAuditor.new(
resource, spec_name,
online: @online, strict: @strict, only: @only, except: @except,
online: @online, strict: @strict, only: @only, except:,
pypi_formulae: formula.tap&.pypi_dependencies_formulae,
use_homebrew_curl: resource.using == :homebrew_curl
).audit
ra.problems.each do |message|
Expand Down
14 changes: 11 additions & 3 deletions Library/Homebrew/resource_auditor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def initialize(resource, spec_name, options = {})
@only = options[:only]
@except = options[:except]
@core_tap = options[:core_tap]
@pypi_formulae = options[:pypi_formulae] || []
@use_homebrew_curl = options[:use_homebrew_curl]
@problems = []
end
Expand Down Expand Up @@ -108,7 +109,7 @@ def self.curl_deps
end
end

def audit_resource_name_matches_pypi_package_name_in_url
def audit_pypi_resources
return unless url.match?(%r{^https?://files\.pythonhosted\.org/packages/})
return if name == owner.name # Skip the top-level package name as we only care about `resource "foo"` blocks.

Expand All @@ -124,9 +125,16 @@ def audit_resource_name_matches_pypi_package_name_in_url

T.must(pypi_package_name).gsub!(/[_.]/, "-")

return if name.casecmp(pypi_package_name).zero?
if name.casecmp(pypi_package_name).nonzero?
problem "`resource` name should be '#{pypi_package_name}' to match the PyPI package name"
end

pypi_package_name = pypi_package_name.to_s.downcase

return if @pypi_formulae.exclude?(pypi_package_name)

Comment thread
botantony marked this conversation as resolved.
problem "`resource` name should be '#{pypi_package_name}' to match the PyPI package name"
problem "PyPI package should be replaced with `depends_on \"#{pypi_package_name}\"` " \
"and excluded using `pypi_package` method"
Comment thread
botantony marked this conversation as resolved.
Outdated
end

def audit_urls
Expand Down
16 changes: 16 additions & 0 deletions Library/Homebrew/tap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class Tap
private_constant :HOMEBREW_TAP_SYNCED_VERSIONS_FORMULAE_FILE
HOMEBREW_TAP_DISABLED_NEW_USR_LOCAL_RELOCATION_FORMULAE_FILE = "disabled_new_usr_local_relocation_formulae.json"
private_constant :HOMEBREW_TAP_DISABLED_NEW_USR_LOCAL_RELOCATION_FORMULAE_FILE
HOMEBREW_TAP_PYPI_DEPENDENCIES_FORMULAE = "pypi_dependencies_formulae.json"
private_constant :HOMEBREW_TAP_PYPI_DEPENDENCIES_FORMULAE
HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR = "audit_exceptions"
private_constant :HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR
HOMEBREW_TAP_STYLE_EXCEPTIONS_DIR = "style_exceptions"
Expand All @@ -40,6 +42,7 @@ class Tap
#{HOMEBREW_TAP_MIGRATIONS_FILE}
#{HOMEBREW_TAP_SYNCED_VERSIONS_FORMULAE_FILE}
#{HOMEBREW_TAP_DISABLED_NEW_USR_LOCAL_RELOCATION_FORMULAE_FILE}
#{HOMEBREW_TAP_PYPI_DEPENDENCIES_FORMULAE}
#{HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR}/*.json
#{HOMEBREW_TAP_STYLE_EXCEPTIONS_DIR}/*.json
].freeze, T::Array[String])
Expand Down Expand Up @@ -1102,6 +1105,19 @@ def disabled_new_usr_local_relocation_formulae
)
end

# Array with PyPI packages that should be used as dependencies
sig { overridable.returns(T::Array[String]) }
def pypi_dependencies_formulae
@pypi_dependencies_formulae ||= T.let(
if (synced_file = path/HOMEBREW_TAP_PYPI_DEPENDENCIES_FORMULAE).file?
JSON.parse(synced_file.read)
else
[]
end,
T.nilable(T::Array[String]),
)
end

sig { returns(T::Boolean) }
def should_report_analytics?
installed? && !private?
Expand Down
97 changes: 95 additions & 2 deletions Library/Homebrew/test/formula_auditor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ def formula_auditor(name, text, options = {})

if options.key? :tap_audit_exceptions
tap = Tap.fetch("test/tap")
allow(tap).to receive(:audit_exceptions).and_return(options[:tap_audit_exceptions])
allow(tap).to receive_messages(
audit_exceptions: options[:tap_audit_exceptions],
pypi_dependencies_formulae: options[:pypi_dependencies_formulae] || [],
)
allow(formula).to receive(:tap).and_return(tap)
options.delete :tap_audit_exceptions
end
Expand Down Expand Up @@ -580,7 +583,9 @@ class Foo < Formula
end
end

describe "#audit_resource_name_matches_pypi_package_name_in_url" do
describe "#audit_pypi_resources" do
let(:pypi_dependencies_formulae) { ["bar", "baz"] }

it "reports a problem if the resource name does not match the python sdist name" do
fa = formula_auditor "foo", <<~RUBY
class Foo < Formula
Expand Down Expand Up @@ -618,6 +623,94 @@ class Foo < Formula
expect(fa.problems.first[:message])
.to match("`resource` name should be 'FooSomething' to match the PyPI package name")
end

it "reports a problem if the resource should be replaced with a dependency" do
fa = formula_auditor("foo", <<~RUBY, tap_audit_exceptions: {}, pypi_dependencies_formulae:)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
sha256 "abc123"
homepage "https://brew.sh"

resource "bar" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/bar-1.0.0.tar.gz"
sha256 "def456"
end

resource "baz" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/baz-1.0.0.tar.gz"
sha256 "ghi789"
end
end
RUBY

fa.audit_specs
expect(fa.problems.count).to eq(2)
expect(fa.problems.first[:message])
.to match("PyPI package should be replaced with `depends_on \"bar\"` " \
"and excluded using `pypi_package` method")
end

it "doesn't report a problem if there is an exception to a PyPI resource that should be a dependency" do
tap_audit_exceptions = { pypi_resources_allowlist: { "foo" => "bar baz" } }
fa = formula_auditor("foo", <<~RUBY, tap_audit_exceptions:, pypi_dependencies_formulae:)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
sha256 "abc123"
homepage "https://brew.sh"

resource "bar" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/bar-1.0.0.tar.gz"
sha256 "def456"
end

resource "baz" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/baz-1.0.0.tar.gz"
sha256 "ghi789"
end
end
RUBY

fa.audit_specs
expect(fa.problems).to be_empty
end

it "doesn't report a problem if formula is deprecated" do
fa = formula_auditor("foo", <<~RUBY, tap_audit_exceptions: {}, pypi_dependencies_formulae:)
class Foo < Formula
url "https://brew.sh/foo-1.0.tgz"
sha256 "abc123"
homepage "https://brew.sh"

deprecate! date: "2026-01-01", because: "some reasone"

resource "bar" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/bar-1.0.0.tar.gz"
sha256 "def456"
end

resource "baz" do
url "https://files.pythonhosted.org/packages/00/00/aaaa/baz-1.0.0.tar.gz"
sha256 "ghi789"
end
end
RUBY

fa.audit_specs
expect(fa.problems).to be_empty
end

it "doesn't audit PyPI package if it is not a resource" do
fa = formula_auditor("bar", <<~RUBY, tap_audit_exceptions: {}, pypi_dependencies_formulae:)
class Bar < Formula
url "https://files.pythonhosted.org/packages/00/00/aaaa/bar-1.0.0.tar.gz"
sha256 "abc123"
homepage "https://brew.sh"
end
RUBY

fa.audit_specs
expect(fa.problems).to be_empty
end
end

describe "#check_service_command" do
Expand Down
Loading