From 5d2ae98d0cdd83c6276b1e293ac3fe74327f804d Mon Sep 17 00:00:00 2001 From: Issy Long Date: Sun, 3 Sep 2023 00:18:15 +0100 Subject: [PATCH 1/6] Add an audit for mismatched Python resource and PyPi package names - Issue 14537. - When people manually add or modify PyPI resources the `Resource#name` sometimes ends up out-of-sync with the PyPI package name. --- Library/Homebrew/resource_auditor.rb | 9 +++++++++ Library/Homebrew/test/dev-cmd/audit_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index f149e19f48..05f76d0103 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -100,6 +100,15 @@ module Homebrew end end + def audit_resource_name_matches_pypi_package_name_in_url + return unless url.match?(%r{^https?://files\.pythonhosted\.org/packages/}) + + pypi_package_name = url.split("/").last.split(/-\d+\.\d+./).first.tr("_", "-") + return if name.casecmp(pypi_package_name).zero? + + problem "resource name should be `#{pypi_package_name}` to closer match the PyPI package name" + end + def audit_urls urls = [url] + mirrors diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 3e0156aa43..413f8916ec 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -522,6 +522,27 @@ module Homebrew end end + describe "#audit_resource_name_matches_pypi_package_name_in_url" do + it "reports a problem if the resource name does not match the python package name" do + fa = formula_auditor "foo", <<~RUBY + class Foo < Formula + url "https://brew.sh/foo-1.0.tgz" + sha256 "abc123" + homepage "https://brew.sh" + + resource "Something" do + url "https://files.pythonhosted.org/packages/FooSomething-1.0.0.tar.gz" + sha256 "def456" + end + end + RUBY + + fa.audit_specs + expect(fa.problems.first[:message]) + .to match("resource name should be `FooSomething` to closer match the PyPI package name") + end + end + describe "#check_service_command" do specify "Not installed" do fa = formula_auditor "foo", <<~RUBY From 94d40615892c6f03ad6f8917b9fce1175f7fde9a Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 6 Sep 2023 23:16:25 +0100 Subject: [PATCH 2/6] Improve PyPI package name audit wording --- Library/Homebrew/resource_auditor.rb | 2 +- Library/Homebrew/test/dev-cmd/audit_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index 05f76d0103..68f1f90594 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -106,7 +106,7 @@ module Homebrew pypi_package_name = url.split("/").last.split(/-\d+\.\d+./).first.tr("_", "-") return if name.casecmp(pypi_package_name).zero? - problem "resource name should be `#{pypi_package_name}` to closer match the PyPI package name" + problem "resource name should be `#{pypi_package_name}` to match the PyPI package name" end def audit_urls diff --git a/Library/Homebrew/test/dev-cmd/audit_spec.rb b/Library/Homebrew/test/dev-cmd/audit_spec.rb index 413f8916ec..b5b3a93701 100644 --- a/Library/Homebrew/test/dev-cmd/audit_spec.rb +++ b/Library/Homebrew/test/dev-cmd/audit_spec.rb @@ -539,7 +539,7 @@ module Homebrew fa.audit_specs expect(fa.problems.first[:message]) - .to match("resource name should be `FooSomething` to closer match the PyPI package name") + .to match("resource name should be `FooSomething` to match the PyPI package name") end end From bb44d66e79fcbfc0c90ec47414ac48fd6d2d7004 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 6 Sep 2023 23:28:23 +0100 Subject: [PATCH 3/6] Python package names can have more characters in than just `_` and `-` --- Library/Homebrew/resource_auditor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index 68f1f90594..3bf4cc912a 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -103,7 +103,7 @@ module Homebrew def audit_resource_name_matches_pypi_package_name_in_url return unless url.match?(%r{^https?://files\.pythonhosted\.org/packages/}) - pypi_package_name = url.split("/").last.split(/-\d+\.\d+./).first.tr("_", "-") + pypi_package_name = url.split("/").last.split(/[-.]\d+?./).first.gsub(/[_.]/, "-") return if name.casecmp(pypi_package_name).zero? problem "resource name should be `#{pypi_package_name}` to match the PyPI package name" From 08f58ab5f73e9e884cab30d68ad41295fa8f5d53 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Wed, 6 Sep 2023 23:29:06 +0100 Subject: [PATCH 4/6] Skip when the resource name is the same as the formula name - Otherwise we get an audit failure in, for example, the `twine-pypi` formula for the package name from its `url` that's actually `twine`. - For this we only should track `resource "name"` blocks. --- Library/Homebrew/resource_auditor.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index 3bf4cc912a..1f9b1c5a13 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -102,6 +102,7 @@ module Homebrew def audit_resource_name_matches_pypi_package_name_in_url 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. pypi_package_name = url.split("/").last.split(/[-.]\d+?./).first.gsub(/[_.]/, "-") return if name.casecmp(pypi_package_name).zero? From bf163013d92c75703cb4bc508da5b8c83365402c Mon Sep 17 00:00:00 2001 From: Issy Long Date: Tue, 12 Sep 2023 00:30:21 +0100 Subject: [PATCH 5/6] Use a regex instead of splitting the URL on `/` etc --- Library/Homebrew/resource_auditor.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index 1f9b1c5a13..acfa38fc2a 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -104,7 +104,8 @@ module Homebrew 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. - pypi_package_name = url.split("/").last.split(/[-.]\d+?./).first.gsub(/[_.]/, "-") + url =~ %r{/(?[^/]+)-} + pypi_package_name = Regexp.last_match(:package_name).gsub(/[_.]/, "-") return if name.casecmp(pypi_package_name).zero? problem "resource name should be `#{pypi_package_name}` to match the PyPI package name" From e7c4d7ebeb9a4a160dda790ca6bc555be29918bb Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 13 Sep 2023 08:51:54 +0100 Subject: [PATCH 6/6] resource_auditor: handle potential nil case. --- Library/Homebrew/resource_auditor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/resource_auditor.rb b/Library/Homebrew/resource_auditor.rb index acfa38fc2a..a4d14946c0 100644 --- a/Library/Homebrew/resource_auditor.rb +++ b/Library/Homebrew/resource_auditor.rb @@ -105,7 +105,7 @@ module Homebrew return if name == owner.name # Skip the top-level package name as we only care about `resource "foo"` blocks. url =~ %r{/(?[^/]+)-} - pypi_package_name = Regexp.last_match(:package_name).gsub(/[_.]/, "-") + pypi_package_name = Regexp.last_match(:package_name).to_s.gsub(/[_.]/, "-") return if name.casecmp(pypi_package_name).zero? problem "resource name should be `#{pypi_package_name}` to match the PyPI package name"