From af6f728eb42f4715f8365f4598ba3e5ef92121aa Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Sun, 4 Jun 2023 23:31:40 -0400 Subject: [PATCH] utils, test: rewrite PyPI::Package This rewrites the `Package` class from the ground up to better accomodate non-PyPI URLs. The existing APIs are largely preserved, but with clearer invariants around when they can or can't be used (e.g., `#pypi_info`). Signed-off-by: William Woodruff --- Library/Homebrew/test/utils/pypi_spec.rb | 86 +++++++--- Library/Homebrew/utils/pypi.rb | 192 ++++++++++++----------- 2 files changed, 166 insertions(+), 112 deletions(-) diff --git a/Library/Homebrew/test/utils/pypi_spec.rb b/Library/Homebrew/test/utils/pypi_spec.rb index ad0657c8a7..33cf00348b 100644 --- a/Library/Homebrew/test/utils/pypi_spec.rb +++ b/Library/Homebrew/test/utils/pypi_spec.rb @@ -3,14 +3,17 @@ require "utils/pypi" describe PyPI do - let(:package_url) do + let(:pypi_package_url) do "https://files.pythonhosted.org/packages/b0/3f/2e1dad67eb172b6443b5eb37eb885a054a55cfd733393071499514140282/" \ "snakemake-5.29.0.tar.gz" end - let(:old_package_url) do + let(:old_pypi_package_url) do "https://files.pythonhosted.org/packages/6f/c4/da52bfdd6168ea46a0fe2b7c983b6c34c377a8733ec177cc00b197a96a9f/" \ "snakemake-5.28.0.tar.gz" end + let(:non_pypi_package_url) do + "https://github.com/pypa/pip-audit/releases/download/v2.5.6/v2.5.6.tar.gz" + end describe PyPI::Package do let(:package_checksum) { "47417307d08ecb0707b3b29effc933bd63d8c8e3ab15509c62b685b7614c6568" } @@ -22,7 +25,8 @@ describe PyPI do let(:package_with_extra) { described_class.new("snakemake[foo]") } let(:package_with_extra_and_version) { described_class.new("snakemake[foo]==5.28.0") } let(:package_with_different_capitalization) { described_class.new("SNAKEMAKE") } - let(:package_from_url) { described_class.new(package_url, is_url: true) } + let(:package_from_pypi_url) { described_class.new(pypi_package_url, is_url: true) } + let(:package_from_non_pypi_url) { described_class.new(non_pypi_package_url, is_url: true) } let(:other_package) { described_class.new("virtualenv==20.2.0") } describe "initialize" do @@ -66,12 +70,50 @@ describe PyPI do expect(described_class.new("foo[bar,baz]==1.2.3").version).to eq "1.2.3" end - it "initializes name from url" do - expect(described_class.new(package_url, is_url: true).name).to eq "snakemake" + it "initializes name from PyPI url" do + expect(described_class.new(pypi_package_url, is_url: true).name).to eq "snakemake" end - it "initializes version from url" do - expect(described_class.new(package_url, is_url: true).version).to eq "5.29.0" + it "initializes version from PyPI url" do + expect(described_class.new(pypi_package_url, is_url: true).version).to eq "5.29.0" + end + end + + describe ".version=" do + it "sets for package names" do + package = described_class.new("snakemake==5.28.0") + expect(package.version).to eq "5.28.0" + + package.version = "5.29.0" + expect(package.version).to eq "5.29.0" + end + + it "sets for PyPI package URLs" do + package = described_class.new(old_pypi_package_url, is_url: true) + expect(package.version).to eq "5.28.0" + + package.version = "5.29.0" + expect(package.version).to eq "5.29.0" + end + + it "fails for non-PYPI package URLs" do + package = described_class.new(non_pypi_package_url, is_url: true) + + expect {package.version = "1.2.3" }.to raise_error(ArgumentError) + end + end + + describe ".valid_pypi_package?" do + it "is true for package names" do + expect(package.valid_pypi_package?).to be true + end + + it "is true for PyPI URLs" do + expect(package_from_pypi_url.valid_pypi_package?).to be true + end + + it "is false for non-PyPI URLs" do + expect(package_from_non_pypi_url.valid_pypi_package?).to be false end end @@ -81,7 +123,8 @@ describe PyPI do end it "gets pypi info from a package name and specified version" do - expect(package.pypi_info(version: "5.29.0")).to eq ["snakemake", package_url, package_checksum, "5.29.0"] + expect(package.pypi_info(new_version: "5.29.0")).to eq ["snakemake", pypi_package_url, package_checksum, + "5.29.0"] end it "gets pypi info from a package name with extra" do @@ -89,26 +132,27 @@ describe PyPI do end it "gets pypi info from a package name and version" do - expect(package_with_version.pypi_info).to eq ["snakemake", old_package_url, old_package_checksum, "5.28.0"] + expect(package_with_version.pypi_info).to eq ["snakemake", old_pypi_package_url, old_package_checksum, + "5.28.0"] end it "gets pypi info from a package name with overridden version" do - expected_result = ["snakemake", package_url, package_checksum, "5.29.0"] - expect(package_with_version.pypi_info(version: "5.29.0")).to eq expected_result + expected_result = ["snakemake", pypi_package_url, package_checksum, "5.29.0"] + expect(package_with_version.pypi_info(new_version: "5.29.0")).to eq expected_result end it "gets pypi info from a package name, extras, and version" do - expected_result = ["snakemake", old_package_url, old_package_checksum, "5.28.0"] + expected_result = ["snakemake", old_pypi_package_url, old_package_checksum, "5.28.0"] expect(package_with_extra_and_version.pypi_info).to eq expected_result end it "gets pypi info from a url" do - expect(package_from_url.pypi_info).to eq ["snakemake", package_url, package_checksum, "5.29.0"] + expect(package_from_pypi_url.pypi_info).to eq ["snakemake", pypi_package_url, package_checksum, "5.29.0"] end it "gets pypi info from a url with overridden version" do - expected_result = ["snakemake", old_package_url, old_package_checksum, "5.28.0"] - expect(package_from_url.pypi_info(version: "5.28.0")).to eq expected_result + expected_result = ["snakemake", old_pypi_package_url, old_package_checksum, "5.28.0"] + expect(package_from_pypi_url.pypi_info(new_version: "5.28.0")).to eq expected_result end end @@ -130,7 +174,7 @@ describe PyPI do end it "returns string representation of package from url" do - expect(package_from_url.to_s).to eq "snakemake==5.29.0" + expect(package_from_pypi_url.to_s).to eq "snakemake==5.29.0" end end @@ -169,19 +213,15 @@ describe PyPI do describe "update_pypi_url", :needs_network do it "updates url to new version" do - expect(described_class.update_pypi_url(old_package_url, "5.29.0")).to eq package_url + expect(described_class.update_pypi_url(old_pypi_package_url, "5.29.0")).to eq pypi_package_url end it "returns nil for invalid versions" do - expect(described_class.update_pypi_url(old_package_url, "0.0.0")).to be_nil - end - - it "returns nil for nonexistent urls" do - expect(described_class.update_pypi_url("https://brew.sh/foo-1.0.tgz", "1.1")).to be_nil + expect(described_class.update_pypi_url(old_pypi_package_url, "0.0.0")).to be_nil end it "returns nil for non-pypi urls" do - expect(described_class.update_pypi_url("https://github.com/pypa/pip-audit/releases/download/v2.5.6/v2.5.6.tar.gz", "1.1")).to be_nil + expect(described_class.update_pypi_url(non_pypi_package_url, "1.1")).to be_nil end end end diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index aeb3e52940..28c257930e 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -8,80 +8,59 @@ module PyPI PYTHONHOSTED_URL_PREFIX = "https://files.pythonhosted.org/packages/" private_constant :PYTHONHOSTED_URL_PREFIX - # Represents a Python package. # This package can be a PyPI package (either by name/version or PyPI distribution URL), # or it can be a non-PyPI URL. # @api private class Package - attr_accessor :name, :extras, :version - sig { params(package_string: String, is_url: T::Boolean).void } def initialize(package_string, is_url: false) @pypi_info = nil - @from_pypi = true + @package_string = package_string + @is_url = is_url + @is_pypi_url = package_string.start_with? PYTHONHOSTED_URL_PREFIX + end - if is_url - if package_string.start_with?(PYTHONHOSTED_URL_PREFIX) - match = File.basename(package_string).match(/^(.+)-([a-z\d.]+?)(?:.tar.gz|.zip)$/) + sig { returns(String) } + def name + @name ||= basic_metadata[0] + end - raise ArgumentError, "Package should be a valid PyPI URL" if match.blank? + sig { returns(T::Array[T.nilable(String)]) } + def extras + @extras ||= basic_metadata[1] + end - @name = PyPI.normalize_python_package(match[1]) - @version = match[2] - else - ensure_formula_installed!("python") + sig { returns(String) } + def version + @version ||= basic_metadata[2] + end - # The URL might be a source distribution hosted somewhere; - # try and use `pip install -q --no-deps --dry-run --report ...` to get its - # name and version. - # Note that this is different from the (similar) `pip install --report` we - # do below, in that it uses `--no-deps` because we only care about resolving - # this specific URL's project metadata. - command = - [Formula["python"].bin/"python3", "-m", "pip", "install", "-q", "--no-deps", - "--dry-run", "--ignore-installed", "--report", "/dev/stdout", package_string] - pip_output = Utils.popen_read({ "PIP_REQUIRE_VIRTUALENV" => "false" }, *command) - unless $CHILD_STATUS.success? - raise ArgumentError, <<~EOS - Unable to determine dependencies for "#{package_string}" because of a failure when running - `#{command.join(" ")}`. - EOS - end + sig { params(new_version: String).void } + def version=(new_version) + raise ArgumentError, "can't update version for non-PyPI packages" unless valid_pypi_package? - metadata = JSON.parse(pip_output)["install"].first["metadata"] - @name = PyPI.normalize_python_package metadata["name"] - @version = metadata["version"] - @from_pypi = false - end + @version = new_version + end - return - end - - if package_string.include? "==" - @name, @version = package_string.split("==") - else - @name = package_string - end - - return unless (match = T.must(@name).match(/^(.*?)\[(.+)\]$/)) - - @name = match[1] - @extras = T.must(match[2]).split "," + sig { returns(T::Boolean) } + def valid_pypi_package? + @is_pypi_url || !@is_url end # Get name, URL, SHA-256 checksum, and latest version for a given package. # This only works for packages from PyPI or from a PyPI URL; packages # derived from non-PyPI URLs will produce `nil` here. - sig { params(version: T.nilable(T.any(String, Version))).returns(T.nilable(T::Array[String])) } - def pypi_info(version: nil) - return @pypi_info if @pypi_info.present? && version.blank? + sig { params(new_version: T.nilable(T.any(String, Version))).returns(T.nilable(T::Array[String])) } + def pypi_info(new_version: nil) + return unless valid_pypi_package? + return @pypi_info if @pypi_info.present? && new_version.blank? - version ||= @version - metadata_url = if version.present? - "https://pypi.org/pypi/#{@name}/#{version}/json" + new_version ||= version + metadata_url = if new_version.present? + "https://pypi.org/pypi/#{name}/#{new_version}/json" else - "https://pypi.org/pypi/#{@name}/json" + "https://pypi.org/pypi/#{name}/json" end out, _, status = curl_output metadata_url, "--location", "--fail" @@ -102,24 +81,22 @@ module PyPI ] end - sig { returns(T::Boolean) } - def valid_pypi_package? - return false unless @from_pypi - info = pypi_info - info.present? && info.is_a?(Array) - end - sig { returns(String) } def to_s - out = @name - out += "[#{@extras.join(",")}]" if @extras.present? - out += "==#{@version}" if @version.present? - out + if valid_pypi_package? + out = name + out += "[#{extras.join(",")}]" if extras.present? + out += "==#{version}" if version.present? + out + else + @package_string + end end sig { params(other: Package).returns(T::Boolean) } def same_package?(other) - T.must(@name.tr("_", "-").casecmp(other.name.tr("_", "-"))).zero? + # These names are pre-normalized, so we can compare them directly. + name == other.name end # Compare only names so we can use .include? and .uniq on a Package array @@ -131,12 +108,62 @@ module PyPI sig { returns(Integer) } def hash - @name.tr("_", "-").downcase.hash + name.hash end sig { params(other: Package).returns(T.nilable(Integer)) } def <=>(other) - @name <=> other.name + name <=> other.name + end + + private + + # Returns [name, [extras], version] for this package. + def basic_metadata + @basic_metadata ||= if @is_pypi_url + match = File.basename(@package_string).match(/^(.+)-([a-z\d.]+?)(?:.tar.gz|.zip)$/) + raise ArgumentError, "Package should be a valid PyPI URL" if match.blank? + + [PyPI.normalize_python_package(match[1]), [], match[2]] + elsif @is_url + ensure_formula_installed!("python") + + # The URL might be a source distribution hosted somewhere; + # try and use `pip install -q --no-deps --dry-run --report ...` to get its + # name and version. + # Note that this is different from the (similar) `pip install --report` we + # do below, in that it uses `--no-deps` because we only care about resolving + # this specific URL's project metadata. + command = + [Formula["python"].bin/"python3", "-m", "pip", "install", "-q", "--no-deps", + "--dry-run", "--ignore-installed", "--report", "/dev/stdout", @package_string] + pip_output = Utils.popen_read({ "PIP_REQUIRE_VIRTUALENV" => "false" }, *command) + unless $CHILD_STATUS.success? + raise ArgumentError, <<~EOS + Unable to determine metadata for "#{@package_string}" because of a failure when running + `#{command.join(" ")}`. + EOS + end + + metadata = JSON.parse(pip_output)["install"].first["metadata"] + [PyPI.normalize_python_package(metadata["name"]), [], metadata["version"]] + else + if @package_string.include? "==" + name, version = @package_string.split("==") + else + name = @package_string + version = nil + end + + if (match = T.must(name).match(/^(.*?)\[(.+)\]$/)) + name = match[1] + extras = T.must(match[2]).split "," + + [PyPI.normalize_python_package(name), extras, version] + else + [PyPI.normalize_python_package(name), [], version] + end + end end end @@ -146,7 +173,7 @@ module PyPI return unless package.valid_pypi_package? - _, url = package.pypi_info(version: version) + _, url = package.pypi_info(new_version: version) url rescue ArgumentError nil @@ -191,30 +218,17 @@ module PyPI main_package = if package_name.present? Package.new(package_name) else - begin - Package.new(formula.stable.url, is_url: true) - rescue ArgumentError - nil + Package.new(formula.stable.url, is_url: true) + end + + if version.present? + if main_package.valid_pypi_package? + main_package.version = version + else + odie "The main package is not a PyPI package. Please update its URL manually." end end - if main_package.blank? - return if ignore_non_pypi_packages - - odie <<~EOS - Could not infer PyPI package name from URL: - #{Formatter.url(formula.stable.url)} - EOS - end - - unless main_package.valid_pypi_package? - return if ignore_non_pypi_packages - - odie "\"#{main_package}\" is not available on PyPI." - end - - main_package.version = version if version.present? - extra_packages = (extra_packages || []).map { |p| Package.new p } exclude_packages = (exclude_packages || []).map { |p| Package.new p } exclude_packages += %W[#{main_package.name} argparse pip setuptools wsgiref].map { |p| Package.new p }