Skip to content

Commit c445820

Browse files
committed
resource_auditor: audit PyPI resources that exist as dependencies
Some widely-used PyPI packages are available in Homebrew as dependencies for other Python-based formulae. We encourage their use either because they take a lot of time to build (f.e. `pydantic` or `scipy`) or we don't want to do hundreds of revision bumps when new security updates come out (f.e. `cryptography` or `certifi`). The problem I see with new contributors is that they don't know it. A lot of the time, they read the cookbook, create a Python-based formula, and it passes audit and tests. They did nothing wrong, but a maintainer still have to point out, "Hey, numpy takes a lot of time to build, and it exists as a formula, let's use it instead". I'd rather add an audit for such cases and make exceptions for formulae where it cannot be used I'd also take a look at [Python for Formula Authors](https://docs.brew.sh/Python-for-Formula-Authors) but it should be revised in another PR Signed-off-by: botantony <antonsm21@gmail.com>
1 parent 30f9c6f commit c445820

File tree

3 files changed

+87
-5
lines changed

3 files changed

+87
-5
lines changed

Library/Homebrew/formula_auditor.rb

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,14 @@ def get_repo_data(regex)
731731
def audit_specs
732732
problem "HEAD-only (no stable download)" if head_only?(formula) && @core_tap
733733

734+
allowed_pypi_packages = formula.tap&.audit_exception(:pypi_resources_allowlist, formula.name)
735+
allowed_pypi_packages = case allowed_pypi_packages
736+
when String
737+
allowed_pypi_packages.split(/\s+/i).to_set
738+
else
739+
Set.new
740+
end
741+
734742
%w[Stable HEAD].each do |name|
735743
spec_name = name.downcase.to_sym
736744
next unless (spec = formula.send(spec_name))
@@ -753,9 +761,15 @@ def audit_specs
753761
spec.resources.each_value do |resource|
754762
problem "Resource name should be different from the formula name" if resource.name == formula.name
755763

764+
except = if allowed_pypi_packages.include?(resource.name)
765+
@except.to_a + ["pypi_resources"]
766+
else
767+
@except
768+
end
769+
756770
ra = ResourceAuditor.new(
757771
resource, spec_name,
758-
online: @online, strict: @strict, only: @only, except: @except,
772+
online: @online, strict: @strict, only: @only, except:,
759773
use_homebrew_curl: resource.using == :homebrew_curl
760774
).audit
761775
ra.problems.each do |message|

Library/Homebrew/resource_auditor.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ module Homebrew
88
class ResourceAuditor
99
include Utils::Curl
1010

11+
DEPENDENCY_PACKAGES = Set.new(%w[certifi cffi cryptography numpy pillow pydantic rpds-py scipy torch]).freeze
12+
1113
attr_reader :name, :version, :checksum, :url, :mirrors, :using, :specs, :owner, :spec_name, :problems
1214

1315
def initialize(resource, spec_name, options = {})
@@ -108,7 +110,7 @@ def self.curl_deps
108110
end
109111
end
110112

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

@@ -124,9 +126,13 @@ def audit_resource_name_matches_pypi_package_name_in_url
124126

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

127-
return if name.casecmp(pypi_package_name).zero?
129+
if name.casecmp(pypi_package_name).nonzero?
130+
problem "`resource` name should be '#{pypi_package_name}' to match the PyPI package name"
131+
end
132+
133+
return if DEPENDENCY_PACKAGES.exclude?(pypi_package_name.downcase)
128134

129-
problem "`resource` name should be '#{pypi_package_name}' to match the PyPI package name"
135+
problem "PyPI package should be replaced with Homebrew dependency and excluded using `pypi_package` method"
130136
end
131137

132138
def audit_urls

Library/Homebrew/test/formula_auditor_spec.rb

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ class Foo < Formula
525525
end
526526
end
527527

528-
describe "#audit_resource_name_matches_pypi_package_name_in_url" do
528+
describe "#audit_pypi_resources" do
529529
it "reports a problem if the resource name does not match the python sdist name" do
530530
fa = formula_auditor "foo", <<~RUBY
531531
class Foo < Formula
@@ -563,6 +563,68 @@ class Foo < Formula
563563
expect(fa.problems.first[:message])
564564
.to match("`resource` name should be 'FooSomething' to match the PyPI package name")
565565
end
566+
567+
it "reports a problem if the resource should be replaced with a dependency" do
568+
fa = formula_auditor "foo", <<~RUBY
569+
class Foo < Formula
570+
url "https://brew.sh/foo-1.0.tgz"
571+
sha256 "abc123"
572+
homepage "https://brew.sh"
573+
574+
resource "cryptography" do
575+
url "https://files.pythonhosted.org/packages/00/00/aaaa/cryptography-1.0.0.tar.gz"
576+
sha256 "def456"
577+
end
578+
579+
resource "pydantic" do
580+
url "https://files.pythonhosted.org/packages/00/00/aaaa/pydantic-1.0.0.tar.gz"
581+
sha256 "ghi789"
582+
end
583+
end
584+
RUBY
585+
586+
fa.audit_specs
587+
expect(fa.problems.count).to eq(2)
588+
expect(fa.problems.first[:message])
589+
.to match("PyPI package should be replaced with Homebrew dependency and exlcuded using `pypi_package` method")
590+
end
591+
592+
it "doesn't report a problem if there is an exception to a PyPI resource that should be a dependency" do
593+
tap_audit_exceptions = { pypi_resources_allowlist: { "foo" => "cryptography pydantic" } }
594+
fa = formula_auditor("foo", <<~RUBY, tap_audit_exceptions:)
595+
class Foo < Formula
596+
url "https://brew.sh/foo-1.0.tgz"
597+
sha256 "abc123"
598+
homepage "https://brew.sh"
599+
600+
resource "cryptography" do
601+
url "https://files.pythonhosted.org/packages/60/04/aaaa/cryptography-1.0.0.tar.gz"
602+
sha256 "def456"
603+
end
604+
605+
resource "pydantic" do
606+
url "https://files.pythonhosted.org/packages/00/00/aaaa/pydantic-1.0.0.tar.gz"
607+
sha256 "ghi789"
608+
end
609+
end
610+
RUBY
611+
612+
fa.audit_specs
613+
expect(fa.problems).to be_empty
614+
end
615+
616+
it "doesn't audit PyPI package if it is not a resource" do
617+
fa = formula_auditor "cryptography", <<~RUBY
618+
class Cryptography < Formula
619+
url "https://files.pythonhosted.org/packages/60/04/aaaa/cryptography-1.0.0.tar.gz"
620+
sha256 "abc123"
621+
homepage "https://brew.sh"
622+
end
623+
RUBY
624+
625+
fa.audit_specs
626+
expect(fa.problems).to be_empty
627+
end
566628
end
567629

568630
describe "#check_service_command" do

0 commit comments

Comments
 (0)