From 8e670995f8907f3b47069def7181dccc33fe2f06 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Wed, 18 Nov 2020 02:25:55 -0500 Subject: [PATCH 1/7] migrate automatic python resource list to Homebrew/core --- .../dev-cmd/update-python-resources.rb | 16 ++- Library/Homebrew/utils/pypi.rb | 122 ++++++++++++------ 2 files changed, 97 insertions(+), 41 deletions(-) diff --git a/Library/Homebrew/dev-cmd/update-python-resources.rb b/Library/Homebrew/dev-cmd/update-python-resources.rb index ee3cd4be19..736268b875 100644 --- a/Library/Homebrew/dev-cmd/update-python-resources.rb +++ b/Library/Homebrew/dev-cmd/update-python-resources.rb @@ -26,6 +26,14 @@ module Homebrew flag "--version=", description: "Use the specified when finding resources for . "\ "If no version is specified, the current version for will be used." + flag "--package-name=", + depends_on: "--version", + description: "Use the specified when finding resources for . "\ + "If no package name is specified, it will be inferred from the formula's stable URL." + comma_array "--extra-packages=", + description: "Include these additional packages when finding resources." + comma_array "--exclude-packages=", + description: "Exclude these packages when finding resources." min_named :formula end @@ -35,7 +43,13 @@ module Homebrew args = update_python_resources_args.parse args.named.to_formulae.each do |formula| - PyPI.update_python_resources! formula, args.version, print_only: args.print_only?, silent: args.silent?, + PyPI.update_python_resources! formula, + version: args.version, + package_name: args.package_name, + extra_packages: args.extra_packages, + exclude_packages: args.exclude_packages, + print_only: args.print_only?, + silent: args.silent?, ignore_non_pypi_packages: args.ignore_non_pypi_packages? end end diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index 6f91f31b57..292f827ab0 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -10,18 +10,6 @@ module PyPI PYTHONHOSTED_URL_PREFIX = "https://files.pythonhosted.org/packages/" private_constant :PYTHONHOSTED_URL_PREFIX - AUTOMATIC_RESOURCE_UPDATE_BLOCKLIST = %w[ - ansible - ansible@2.8 - cloudformation-cli - diffoscope - dxpy - ipython - molecule - salt - ].freeze - private_constant :AUTOMATIC_RESOURCE_UPDATE_BLOCKLIST - @pipgrip_installed = nil def url_to_pypi_package_name(url) @@ -40,7 +28,11 @@ module PyPI # Get name, URL and SHA-256 checksum for a given PyPI package. def get_pypi_info(package, version) - metadata_url = "https://pypi.org/pypi/#{package}/#{version}/json" + metadata_url = if version.present? + "https://pypi.org/pypi/#{package}/#{version}/json" + else + "https://pypi.org/pypi/#{package}/json" + end out, _, status = curl_output metadata_url, "--location" return unless status.success? @@ -58,17 +50,38 @@ module PyPI end # Return true if resources were checked (even if no change). - def update_python_resources!(formula, version = nil, print_only: false, silent: false, - ignore_non_pypi_packages: false) + def update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil, exclude_packages: nil, + print_only: false, silent: false, ignore_non_pypi_packages: false) - if !print_only && AUTOMATIC_RESOURCE_UPDATE_BLOCKLIST.include?(formula.full_name) - odie "The resources for \"#{formula.name}\" need special attention. Please update them manually." - return + auto_update_list = formula.tap.audit_exceptions[:automatic_resource_update_list] + if package_name.blank? && extra_packages.blank? && !print_only && + auto_update_list.present? && auto_update_list.key?(formula.full_name) + + list_entry = auto_update_list[formula.full_name] + case list_entry + when false + odie "The resources for \"#{formula.name}\" need special attention. Please update them manually." + when String + package_name = list_entry + when Hash + package_name = list_entry["package_name"] + extra_packages = list_entry["extra_packages"] + exclude_packages = list_entry["exclude_packages"] + end end - pypi_name = url_to_pypi_package_name formula.stable.url + package_name ||= url_to_pypi_package_name formula.stable.url + version ||= formula.version + extra_packages ||= [] + exclude_packages ||= [] - if pypi_name.nil? + # opoo "package_name: #{package_name}" + # opoo "version: #{version}" + # opoo "extra_packages: #{extra_packages}" + # opoo "exclude_packages: #{exclude_packages}" + # odie "" + + if package_name.nil? return if ignore_non_pypi_packages odie <<~EOS @@ -77,11 +90,24 @@ module PyPI EOS end - version ||= formula.version + input_package_names = { package_name => version } + extra_packages.each do |extra| + extra_name, extra_version = extra.split "==" - if get_pypi_info(pypi_name, version).blank? - odie "\"#{pypi_name}\" at version #{version} is not available on PyPI." unless ignore_non_pypi_packages - return + if input_package_names.key?(extra_name) && input_package_names[extra_name] != extra_version + odie "Conflicting versions specified for the `#{extra_name}` package: "\ + "#{input_package_names[extra_name]}, #{extra_version}" + end + + input_package_names[extra_name] = extra_version + end + + input_package_names.each do |name, package_version| + name = name.split("[").first + next if get_pypi_info(name, package_version).present? + + version_string = " at version #{package_version}" if package_version.present? + odie "\"#{name}\"#{version_string} is not available on PyPI." unless ignore_non_pypi_packages end non_pypi_resources = formula.resources.reject do |resource| @@ -95,27 +121,43 @@ module PyPI @pipgrip_installed ||= Formula["pipgrip"].any_version_installed? odie '"pipgrip" must be installed (`brew install pipgrip`)' unless @pipgrip_installed - ohai "Retrieving PyPI dependencies for \"#{pypi_name}==#{version}\"..." if !print_only && !silent - pipgrip_output = Utils.popen_read Formula["pipgrip"].bin/"pipgrip", "--json", "--no-cache-dir", - "#{pypi_name}==#{version}" - unless $CHILD_STATUS.success? - odie <<~EOS - Unable to determine dependencies for \"#{pypi_name}\" because of a failure when running - `pipgrip --json --no-cache-dir #{pypi_name}==#{version}`. - Please update the resources for \"#{formula.name}\" manually. - EOS - end + found_packages = {} + input_package_names.each do |name, package_version| + pypi_package_string = if package_version.present? + "#{name}==#{package_version}" + else + name + end - packages = JSON.parse(pipgrip_output).sort.to_h + ohai "Retrieving PyPI dependencies for \"#{pypi_package_string}\"..." if !print_only && !silent + pipgrip_output = Utils.popen_read Formula["pipgrip"].bin/"pipgrip", "--json", "--no-cache-dir", + pypi_package_string + unless $CHILD_STATUS.success? + odie <<~EOS + Unable to determine dependencies for \"#{name}\" because of a failure when running + `pipgrip --json --no-cache-dir #{pypi_package_string}`. + Please update the resources for \"#{formula.name}\" manually. + EOS + end + + found_packages.merge!(JSON.parse(pipgrip_output).sort.to_h) do |conflicting_package, old_version, new_version| + next old_version if old_version == new_version + + odie "Conflicting versions found for the `#{conflicting_package}` resource: #{old_version}, #{new_version}" + end + end # Remove extra packages that may be included in pipgrip output - exclude_list = %W[#{pypi_name.downcase} argparse pip setuptools wheel wsgiref] - packages.delete_if do |package| - exclude_list.include? package - end + exclude_list = %W[#{package_name.downcase} argparse pip setuptools wheel wsgiref] + found_packages.delete_if { |package| exclude_list.include? package } new_resource_blocks = "" - packages.each do |package, package_version| + found_packages.each do |package, package_version| + if exclude_packages.include? package + ohai "Excluding \"#{package}==#{package_version}\"" if !print_only && !silent + next + end + ohai "Getting PyPI info for \"#{package}==#{package_version}\"" if !print_only && !silent name, url, checksum = get_pypi_info package, package_version # Fail if unable to find name, url or checksum for any resource From b1d907291b7150b7a243c9a62ee22f3e9c0f2770 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Wed, 18 Nov 2020 02:48:05 -0500 Subject: [PATCH 2/7] update man pages --- docs/Manpage.md | 6 ++++++ manpages/brew.1 | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/docs/Manpage.md b/docs/Manpage.md index 014d4413dd..291381b289 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -1352,6 +1352,12 @@ Update versions for PyPI resource blocks in *`formula`*. Don't fail if *`formula`* is not a PyPI package. * `--version`: Use the specified *`version`* when finding resources for *`formula`*. If no version is specified, the current version for *`formula`* will be used. +* `--package-name`: + Use the specified *`package-name`* when finding resources for *`formula`*. If no package name is specified, it will be inferred from the formula's stable URL. +* `--extra-packages`: + Include these additional packages when finding resources. +* `--exclude-packages`: + Exclude these packages when finding resources. ### `update-test` [*`options`*] diff --git a/manpages/brew.1 b/manpages/brew.1 index 8fa25bf1ae..e700050a57 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -1884,6 +1884,18 @@ Don\'t fail if \fIformula\fR is not a PyPI package\. \fB\-\-version\fR Use the specified \fIversion\fR when finding resources for \fIformula\fR\. If no version is specified, the current version for \fIformula\fR will be used\. . +.TP +\fB\-\-package\-name\fR +Use the specified \fIpackage\-name\fR when finding resources for \fIformula\fR\. If no package name is specified, it will be inferred from the formula\'s stable URL\. +. +.TP +\fB\-\-extra\-packages\fR +Include these additional packages when finding resources\. +. +.TP +\fB\-\-exclude\-packages\fR +Exclude these packages when finding resources\. +. .SS "\fBupdate\-test\fR [\fIoptions\fR]" Run a test of \fBbrew update\fR with a new repository clone\. If no options are passed, use \fBorigin/master\fR as the start commit\. . From 51a1b7c9e1b39e19cc93cb9e40017218a1d2d8cd Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 20 Nov 2020 00:55:21 -0500 Subject: [PATCH 3/7] move pypi list to tap formula_lists directory --- .../dev-cmd/update-python-resources.rb | 1 - Library/Homebrew/tap.rb | 49 ++++++++++++----- Library/Homebrew/tap_auditor.rb | 55 +++++++++++-------- Library/Homebrew/utils/pypi.rb | 27 ++++----- 4 files changed, 76 insertions(+), 56 deletions(-) diff --git a/Library/Homebrew/dev-cmd/update-python-resources.rb b/Library/Homebrew/dev-cmd/update-python-resources.rb index 736268b875..936dad5aba 100644 --- a/Library/Homebrew/dev-cmd/update-python-resources.rb +++ b/Library/Homebrew/dev-cmd/update-python-resources.rb @@ -27,7 +27,6 @@ module Homebrew description: "Use the specified when finding resources for . "\ "If no version is specified, the current version for will be used." flag "--package-name=", - depends_on: "--version", description: "Use the specified when finding resources for . "\ "If no package name is specified, it will be inferred from the formula's stable URL." comma_array "--extra-packages=", diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index d4a33d8ab7..02e60e15cb 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -21,11 +21,13 @@ class Tap HOMEBREW_TAP_FORMULA_RENAMES_FILE = "formula_renames.json" HOMEBREW_TAP_MIGRATIONS_FILE = "tap_migrations.json" HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR = "audit_exceptions" + HOMEBREW_TAP_FORMULA_LISTS_DIR = "formula_lists" HOMEBREW_TAP_JSON_FILES = %W[ #{HOMEBREW_TAP_FORMULA_RENAMES_FILE} #{HOMEBREW_TAP_MIGRATIONS_FILE} #{HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR}/*.json + #{HOMEBREW_TAP_FORMULA_LISTS_DIR}/*.json ].freeze def self.fetch(*args) @@ -112,6 +114,7 @@ class Tap @formula_renames = nil @tap_migrations = nil @audit_exceptions = nil + @formula_lists = nil @config = nil remove_instance_variable(:@private) if instance_variable_defined?(:@private) end @@ -560,22 +563,12 @@ class Tap # Hash with audit exceptions def audit_exceptions - @audit_exceptions = {} + @audit_exceptions = read_formula_list_directory HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR + end - Pathname.glob(path/HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR/"*").each do |exception_file| - list_name = exception_file.basename.to_s.chomp(".json").to_sym - list_contents = begin - JSON.parse exception_file.read - rescue JSON::ParserError - opoo "#{exception_file} contains invalid JSON" - end - - next if list_contents.nil? - - @audit_exceptions[list_name] = list_contents - end - - @audit_exceptions + # Hash with formula lists + def formula_lists + @formula_lists = read_formula_list_directory HOMEBREW_TAP_FORMULA_LISTS_DIR end def ==(other) @@ -636,6 +629,25 @@ class Tap end end end + + def read_formula_list_directory(directory) + list = {} + + Pathname.glob(path/directory/"*").each do |exception_file| + list_name = exception_file.basename.to_s.chomp(".json").to_sym + list_contents = begin + JSON.parse exception_file.read + rescue JSON::ParserError + opoo "#{exception_file} contains invalid JSON" + end + + next if list_contents.nil? + + list[list_name] = list_contents + end + + list + end end # A specialized {Tap} class for the core formulae. @@ -739,6 +751,13 @@ class CoreTap < Tap end end + def formula_lists + @formula_lists ||= begin + self.class.ensure_installed! + super + end + end + # @private def formula_file_to_name(file) file.basename(".rb").to_s diff --git a/Library/Homebrew/tap_auditor.rb b/Library/Homebrew/tap_auditor.rb index d8294815f2..17be78d11c 100644 --- a/Library/Homebrew/tap_auditor.rb +++ b/Library/Homebrew/tap_auditor.rb @@ -15,13 +15,14 @@ module Homebrew @name = tap.name @path = tap.path @tap_audit_exceptions = tap.audit_exceptions + @tap_formula_lists = tap.formula_lists @problems = [] end sig { void } def audit audit_json_files - audit_tap_audit_exceptions + audit_tap_formula_lists end sig { void } @@ -35,32 +36,38 @@ module Homebrew end sig { void } - def audit_tap_audit_exceptions - @tap_audit_exceptions.each do |list_name, formula_names| - unless [Hash, Array].include? formula_names.class + def audit_tap_formula_lists + tap_lists = { + audit_exceptions: @tap_audit_exceptions, + formula_lists: @tap_formula_lists, + } + tap_lists.each do |list_directory, list| + list.each do |list_name, formula_names| + unless [Hash, Array].include? formula_names.class + problem <<~EOS + #{list_directory}/#{list_name}.json should contain a JSON array + of formula names or a JSON object mapping formula names to values + EOS + next + end + + formula_names = formula_names.keys if formula_names.is_a? Hash + + invalid_formulae = [] + formula_names.each do |name| + invalid_formulae << name if Formula[name].tap != @name + rescue FormulaUnavailableError + invalid_formulae << name + end + + next if invalid_formulae.empty? + problem <<~EOS - audit_exceptions/#{list_name}.json should contain a JSON array - of formula names or a JSON object mapping formula names to values + #{list_directory}/#{list_name}.json references + formulae that are not found in the #{@name} tap. + Invalid formulae: #{invalid_formulae.join(", ")} EOS - next end - - formula_names = formula_names.keys if formula_names.is_a? Hash - - invalid_formulae = [] - formula_names.each do |name| - invalid_formulae << name if Formula[name].tap != @name - rescue FormulaUnavailableError - invalid_formulae << name - end - - next if invalid_formulae.empty? - - problem <<~EOS - audit_exceptions/#{list_name}.json references - formulae that are not found in the #{@name} tap. - Invalid formulae: #{invalid_formulae.join(", ")} - EOS end end diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index 292f827ab0..ad75aa6f0e 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -26,8 +26,9 @@ module PyPI url end - # Get name, URL and SHA-256 checksum for a given PyPI package. - def get_pypi_info(package, version) + # Get name, URL, SHA-256 checksum, and latest version for a given PyPI package. + def get_pypi_info(package, version = nil) + package = package.split("[").first metadata_url = if version.present? "https://pypi.org/pypi/#{package}/#{version}/json" else @@ -46,15 +47,15 @@ module PyPI sdist = json["urls"].find { |url| url["packagetype"] == "sdist" } return json["info"]["name"] if sdist.nil? - [json["info"]["name"], sdist["url"], sdist["digests"]["sha256"]] + [json["info"]["name"], sdist["url"], sdist["digests"]["sha256"], json["info"]["version"]] end # Return true if resources were checked (even if no change). def update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil, exclude_packages: nil, print_only: false, silent: false, ignore_non_pypi_packages: false) - auto_update_list = formula.tap.audit_exceptions[:automatic_resource_update_list] - if package_name.blank? && extra_packages.blank? && !print_only && + auto_update_list = formula.tap.formula_lists[:pypi_automatic_resource_update_list] + if package_name.blank? && extra_packages.blank? && exclude_packages.blank? && !print_only && auto_update_list.present? && auto_update_list.key?(formula.full_name) list_entry = auto_update_list[formula.full_name] @@ -70,18 +71,12 @@ module PyPI end end + version ||= formula.version if package_name.blank? package_name ||= url_to_pypi_package_name formula.stable.url - version ||= formula.version extra_packages ||= [] exclude_packages ||= [] - # opoo "package_name: #{package_name}" - # opoo "version: #{version}" - # opoo "extra_packages: #{extra_packages}" - # opoo "exclude_packages: #{exclude_packages}" - # odie "" - - if package_name.nil? + if package_name.blank? return if ignore_non_pypi_packages odie <<~EOS @@ -140,7 +135,7 @@ module PyPI EOS end - found_packages.merge!(JSON.parse(pipgrip_output).sort.to_h) do |conflicting_package, old_version, new_version| + found_packages.merge!(JSON.parse(pipgrip_output).to_h) do |conflicting_package, old_version, new_version| next old_version if old_version == new_version odie "Conflicting versions found for the `#{conflicting_package}` resource: #{old_version}, #{new_version}" @@ -148,11 +143,11 @@ module PyPI end # Remove extra packages that may be included in pipgrip output - exclude_list = %W[#{package_name.downcase} argparse pip setuptools wheel wsgiref] + exclude_list = %W[#{package_name.split("[").first.downcase} argparse pip setuptools wheel wsgiref] found_packages.delete_if { |package| exclude_list.include? package } new_resource_blocks = "" - found_packages.each do |package, package_version| + found_packages.sort.each do |package, package_version| if exclude_packages.include? package ohai "Excluding \"#{package}==#{package_version}\"" if !print_only && !silent next From fac2bd8843f6d959e2566a872d1863abf52d6b62 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 20 Nov 2020 01:55:34 -0500 Subject: [PATCH 4/7] use auto update list when --print-only is passed --- Library/Homebrew/utils/pypi.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index ad75aa6f0e..1822951775 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -55,13 +55,15 @@ module PyPI print_only: false, silent: false, ignore_non_pypi_packages: false) auto_update_list = formula.tap.formula_lists[:pypi_automatic_resource_update_list] - if package_name.blank? && extra_packages.blank? && exclude_packages.blank? && !print_only && - auto_update_list.present? && auto_update_list.key?(formula.full_name) + if auto_update_list.present? && auto_update_list.key?(formula.full_name) && + package_name.blank? && extra_packages.blank? && exclude_packages.blank? list_entry = auto_update_list[formula.full_name] case list_entry when false - odie "The resources for \"#{formula.name}\" need special attention. Please update them manually." + unless print_only + odie "The resources for \"#{formula.name}\" need special attention. Please update them manually." + end when String package_name = list_entry when Hash From 20de58b5ae473939e9a39d4e79c3a6f7601d6bf1 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 20 Nov 2020 09:27:42 -0500 Subject: [PATCH 5/7] bump-formula-pr: use new update_python_resources! signature --- Library/Homebrew/dev-cmd/bump-formula-pr.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/dev-cmd/bump-formula-pr.rb b/Library/Homebrew/dev-cmd/bump-formula-pr.rb index 4e648874f2..dffc4dcdf3 100644 --- a/Library/Homebrew/dev-cmd/bump-formula-pr.rb +++ b/Library/Homebrew/dev-cmd/bump-formula-pr.rb @@ -336,7 +336,7 @@ module Homebrew end unless args.dry_run? - resources_checked = PyPI.update_python_resources! formula, new_formula_version, + resources_checked = PyPI.update_python_resources! formula, version: new_formula_version, silent: args.quiet?, ignore_non_pypi_packages: true end From ee47b863c49ee151fd491f5e687460198ae256fe Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 20 Nov 2020 18:14:45 -0500 Subject: [PATCH 6/7] move mapping from formula_lists to pypi_formula_mappings --- Library/Homebrew/tap.rb | 41 ++++++++++------- Library/Homebrew/tap_auditor.rb | 81 +++++++++++++++++---------------- Library/Homebrew/utils/pypi.rb | 2 +- 3 files changed, 69 insertions(+), 55 deletions(-) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index 02e60e15cb..0022617249 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -21,13 +21,13 @@ class Tap HOMEBREW_TAP_FORMULA_RENAMES_FILE = "formula_renames.json" HOMEBREW_TAP_MIGRATIONS_FILE = "tap_migrations.json" HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR = "audit_exceptions" - HOMEBREW_TAP_FORMULA_LISTS_DIR = "formula_lists" + HOMEBREW_TAP_PYPI_FORMULA_MAPPINGS = "pypi_formula_mappings.json" HOMEBREW_TAP_JSON_FILES = %W[ #{HOMEBREW_TAP_FORMULA_RENAMES_FILE} #{HOMEBREW_TAP_MIGRATIONS_FILE} #{HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR}/*.json - #{HOMEBREW_TAP_FORMULA_LISTS_DIR}/*.json + #{HOMEBREW_TAP_PYPI_FORMULA_MAPPINGS} ].freeze def self.fetch(*args) @@ -114,7 +114,7 @@ class Tap @formula_renames = nil @tap_migrations = nil @audit_exceptions = nil - @formula_lists = nil + @pypi_formula_mappings = nil @config = nil remove_instance_variable(:@private) if instance_variable_defined?(:@private) end @@ -562,13 +562,15 @@ class Tap end # Hash with audit exceptions + sig { returns(Hash) } def audit_exceptions - @audit_exceptions = read_formula_list_directory HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR + @audit_exceptions = read_formula_list_directory "#{HOMEBREW_TAP_AUDIT_EXCEPTIONS_DIR}/*" end - # Hash with formula lists - def formula_lists - @formula_lists = read_formula_list_directory HOMEBREW_TAP_FORMULA_LISTS_DIR + # Hash with pypi formula mappings + sig { returns(Hash) } + def pypi_formula_mappings + @pypi_formula_mappings = read_formula_list path/HOMEBREW_TAP_PYPI_FORMULA_MAPPINGS end def ==(other) @@ -630,18 +632,25 @@ class Tap end end + sig { params(file: Pathname).returns(Hash) } + def read_formula_list(file) + JSON.parse file.read + rescue JSON::ParserError + opoo "#{file} contains invalid JSON" + {} + rescue Errno::ENOENT + {} + end + + sig { params(directory: String).returns(Hash) } def read_formula_list_directory(directory) list = {} - Pathname.glob(path/directory/"*").each do |exception_file| + Pathname.glob(path/directory).each do |exception_file| list_name = exception_file.basename.to_s.chomp(".json").to_sym - list_contents = begin - JSON.parse exception_file.read - rescue JSON::ParserError - opoo "#{exception_file} contains invalid JSON" - end + list_contents = read_formula_list exception_file - next if list_contents.nil? + next if list_contents.blank? list[list_name] = list_contents end @@ -751,8 +760,8 @@ class CoreTap < Tap end end - def formula_lists - @formula_lists ||= begin + def pypi_formula_mappings + @pypi_formula_mappings ||= begin self.class.ensure_installed! super end diff --git a/Library/Homebrew/tap_auditor.rb b/Library/Homebrew/tap_auditor.rb index 17be78d11c..5936980919 100644 --- a/Library/Homebrew/tap_auditor.rb +++ b/Library/Homebrew/tap_auditor.rb @@ -8,15 +8,15 @@ module Homebrew class TapAuditor extend T::Sig - attr_reader :name, :path, :tap_audit_exceptions, :problems + attr_reader :name, :path, :tap_audit_exceptions, :tap_pypi_formula_mappings, :problems sig { params(tap: Tap, strict: T.nilable(T::Boolean)).void } def initialize(tap, strict:) - @name = tap.name - @path = tap.path - @tap_audit_exceptions = tap.audit_exceptions - @tap_formula_lists = tap.formula_lists - @problems = [] + @name = tap.name + @path = tap.path + @tap_audit_exceptions = tap.audit_exceptions + @tap_pypi_formula_mappings = tap.pypi_formula_mappings + @problems = [] end sig { void } @@ -37,43 +37,48 @@ module Homebrew sig { void } def audit_tap_formula_lists - tap_lists = { - audit_exceptions: @tap_audit_exceptions, - formula_lists: @tap_formula_lists, - } - tap_lists.each do |list_directory, list| - list.each do |list_name, formula_names| - unless [Hash, Array].include? formula_names.class - problem <<~EOS - #{list_directory}/#{list_name}.json should contain a JSON array - of formula names or a JSON object mapping formula names to values - EOS - next - end - - formula_names = formula_names.keys if formula_names.is_a? Hash - - invalid_formulae = [] - formula_names.each do |name| - invalid_formulae << name if Formula[name].tap != @name - rescue FormulaUnavailableError - invalid_formulae << name - end - - next if invalid_formulae.empty? - - problem <<~EOS - #{list_directory}/#{list_name}.json references - formulae that are not found in the #{@name} tap. - Invalid formulae: #{invalid_formulae.join(", ")} - EOS - end - end + check_formula_list_directory "audit_exceptions", @tap_audit_exceptions + check_formula_list "pypi_formula_mappings", @tap_pypi_formula_mappings end sig { params(message: String).void } def problem(message) @problems << ({ message: message, location: nil }) end + + private + + sig { params(list_file: String, list: T.untyped).void } + def check_formula_list(list_file, list) + unless [Hash, Array].include? list.class + problem <<~EOS + #{list_file}.json should contain a JSON array + of formula names or a JSON object mapping formula names to values + EOS + return + end + + invalid_formulae = [] + list.each do |name, _| + invalid_formulae << name if Formula[name].tap != @name + rescue FormulaUnavailableError + invalid_formulae << name + end + + return if invalid_formulae.empty? + + problem <<~EOS + #{list_file}.json references + formulae that are not found in the #{@name} tap. + Invalid formulae: #{invalid_formulae.join(", ")} + EOS + end + + sig { params(directory_name: String, lists: Hash).void } + def check_formula_list_directory(directory_name, lists) + lists.each do |list_name, list| + check_formula_list "#{directory_name}/#{list_name}", list + end + end end end diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index 1822951775..4c15a03140 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -54,7 +54,7 @@ module PyPI def update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil, exclude_packages: nil, print_only: false, silent: false, ignore_non_pypi_packages: false) - auto_update_list = formula.tap.formula_lists[:pypi_automatic_resource_update_list] + auto_update_list = formula.tap.pypi_formula_mappings if auto_update_list.present? && auto_update_list.key?(formula.full_name) && package_name.blank? && extra_packages.blank? && exclude_packages.blank? From b2e25d12aa00ff3c78c23087dc265f29c72c35b1 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sun, 22 Nov 2020 15:23:43 -0500 Subject: [PATCH 7/7] utils/pypi: refactor package handling --- Library/Homebrew/test/utils/pypi_spec.rb | 171 ++++++++++++++++ Library/Homebrew/utils/pypi.rb | 240 +++++++++++++++-------- 2 files changed, 334 insertions(+), 77 deletions(-) create mode 100644 Library/Homebrew/test/utils/pypi_spec.rb diff --git a/Library/Homebrew/test/utils/pypi_spec.rb b/Library/Homebrew/test/utils/pypi_spec.rb new file mode 100644 index 0000000000..021a2215c8 --- /dev/null +++ b/Library/Homebrew/test/utils/pypi_spec.rb @@ -0,0 +1,171 @@ +# typed: false +# frozen_string_literal: true + +require "utils/pypi" + +describe PyPI do + let(:package_url) do + "https://files.pythonhosted.org/packages/b0/3f/2e1dad67eb172b6443b5eb37eb885a054a55cfd733393071499514140282/"\ + "snakemake-5.29.0.tar.gz" + end + let(:old_package_url) do + "https://files.pythonhosted.org/packages/6f/c4/da52bfdd6168ea46a0fe2b7c983b6c34c377a8733ec177cc00b197a96a9f/"\ + "snakemake-5.28.0.tar.gz" + end + + describe PyPI::Package do + let(:package_checksum) { "47417307d08ecb0707b3b29effc933bd63d8c8e3ab15509c62b685b7614c6568" } + let(:old_package_checksum) { "2367ce91baf7f8fa7738d33aff9670ffdf5410bbac49aeb209f73b45a3425046" } + + let(:package) { described_class.new("snakemake") } + let(:package_with_version) { described_class.new("snakemake==5.28.0") } + let(:package_with_different_version) { described_class.new("snakemake==5.29.0") } + 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_from_url) { described_class.new(package_url, is_url: true) } + let(:other_package) { described_class.new("virtualenv==20.2.0") } + + describe "initialize" do + it "initializes name" do + expect(described_class.new("foo").name).to eq "foo" + end + + it "initializes name with extra" do + expect(described_class.new("foo[bar]").name).to eq "foo" + end + + it "initializes extra" do + expect(described_class.new("foo[bar]").extras).to eq ["bar"] + end + + it "initializes multiple extras" do + expect(described_class.new("foo[bar,baz]").extras).to eq ["bar", "baz"] + end + + it "initializes name with version" do + expect(described_class.new("foo==1.2.3").name).to eq "foo" + end + + it "initializes version" do + expect(described_class.new("foo==1.2.3").version).to eq "1.2.3" + end + + it "initializes extra with version" do + expect(described_class.new("foo[bar]==1.2.3").extras).to eq ["bar"] + end + + it "initializes multiple extras with version" do + expect(described_class.new("foo[bar,baz]==1.2.3").extras).to eq ["bar", "baz"] + end + + it "initializes version with extra" do + expect(described_class.new("foo[bar]==1.2.3").version).to eq "1.2.3" + end + + it "initializes version with multiple extras" 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" + end + + it "initializes version from url" do + expect(described_class.new(package_url, is_url: true).version).to eq "5.29.0" + end + end + + describe ".pypi_info", :needs_network do + it "gets pypi info from a package name" do + expect(package.pypi_info.first).to eq "snakemake" + 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"] + end + + it "gets pypi info from a package name with extra" do + expect(package_with_extra.pypi_info.first).to eq "snakemake" + 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"] + end + + it "gets pypi info from a package name with overriden 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 + 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"] + 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"] + end + + it "gets pypi info from a url with overriden 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 + end + end + + describe ".to_s" do + it "returns string representation of package name" do + expect(package.to_s).to eq "snakemake" + end + + it "returns string representation of package with version" do + expect(package_with_version.to_s).to eq "snakemake==5.28.0" + end + + it "returns string representation of package with extra" do + expect(package_with_extra.to_s).to eq "snakemake[foo]" + end + + it "returns string representation of package with extra and version" do + expect(package_with_extra_and_version.to_s).to eq "snakemake[foo]==5.28.0" + end + + it "returns string representation of package from url" do + expect(package_from_url.to_s).to eq "snakemake==5.29.0" + end + end + + describe ".same_package?" do + it "returns false for different packages" do + expect(package.same_package?(other_package)).to eq false + end + + it "returns true for the same package" do + expect(package.same_package?(package_with_version)).to eq true + end + + it "returns true for the same package with different versions" do + expect(package_with_version.same_package?(package_with_different_version)).to eq true + end + end + + describe "<=>" do + it "returns -1" do + expect(package <=> other_package).to eq((-1)) + end + + it "returns 0" do + expect(package <=> package_with_version).to eq 0 + end + + it "returns 1" do + expect(other_package <=> package_with_extra_and_version).to eq 1 + end + end + end + + 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 + end + end +end diff --git a/Library/Homebrew/utils/pypi.rb b/Library/Homebrew/utils/pypi.rb index 4c15a03140..d69291b069 100644 --- a/Library/Homebrew/utils/pypi.rb +++ b/Library/Homebrew/utils/pypi.rb @@ -5,6 +5,8 @@ # # @api private module PyPI + extend T::Sig + module_function PYTHONHOSTED_URL_PREFIX = "https://files.pythonhosted.org/packages/" @@ -12,45 +14,119 @@ module PyPI @pipgrip_installed = nil - def url_to_pypi_package_name(url) - return unless url.start_with? PYTHONHOSTED_URL_PREFIX + # PyPI Package + # + # @api private + class Package + extend T::Sig - File.basename(url).match(/^(.+)-[a-z\d.]+$/)[1] + attr_accessor :name + attr_accessor :extras + attr_accessor :version + + sig { params(package_string: String, is_url: T::Boolean).void } + def initialize(package_string, is_url: false) + @pypi_info = nil + + if is_url + unless package_string.start_with?(PYTHONHOSTED_URL_PREFIX) && + match = File.basename(package_string).match(/^(.+)-([a-z\d.]+?)(?:.tar.gz|.zip)$/) + raise ArgumentError, "package should be a valid PyPI url" + end + + @name = match[1] + @version = match[2] + return + end + + @name = package_string + @name, @version = @name.split("==") if @name.include? "==" + + return unless match = @name.match(/^(.*?)\[(.+)\]$/) + + @name = match[1] + @extras = match[2].split "," + end + + # Get name, URL, SHA-256 checksum, and latest version for a given PyPI package. + 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? + + version ||= @version + metadata_url = if version.present? + "https://pypi.org/pypi/#{@name}/#{version}/json" + else + "https://pypi.org/pypi/#{@name}/json" + end + out, _, status = curl_output metadata_url, "--location" + + return unless status.success? + + begin + json = JSON.parse out + rescue JSON::ParserError + return + end + + sdist = json["urls"].find { |url| url["packagetype"] == "sdist" } + return json["info"]["name"] if sdist.nil? + + @pypi_info = [json["info"]["name"], sdist["url"], sdist["digests"]["sha256"], json["info"]["version"]] + end + + sig { returns(T::Boolean) } + def valid_pypi_package? + 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 + end + + sig { params(other: Package).returns(T::Boolean) } + def same_package?(other) + @name.tr("_", "-") == other.name.tr("_", "-") + end + + # Compare only names so we can use .include? on a Package array + sig { params(other: Package).returns(T::Boolean) } + def ==(other) + same_package?(other) + end + + sig { params(other: Package).returns(T.nilable(Integer)) } + def <=>(other) + @name <=> other.name + end end + sig { params(url: String, version: T.any(String, Version)).returns(T.nilable(String)) } def update_pypi_url(url, version) - package = url_to_pypi_package_name url - return if package.nil? + package = Package.new url, is_url: true - _, url = get_pypi_info(package, version) + _, url = package.pypi_info(version: version) url end - # Get name, URL, SHA-256 checksum, and latest version for a given PyPI package. - def get_pypi_info(package, version = nil) - package = package.split("[").first - metadata_url = if version.present? - "https://pypi.org/pypi/#{package}/#{version}/json" - else - "https://pypi.org/pypi/#{package}/json" - end - out, _, status = curl_output metadata_url, "--location" - - return unless status.success? - - begin - json = JSON.parse out - rescue JSON::ParserError - return - end - - sdist = json["urls"].find { |url| url["packagetype"] == "sdist" } - return json["info"]["name"] if sdist.nil? - - [json["info"]["name"], sdist["url"], sdist["digests"]["sha256"], json["info"]["version"]] - end - # Return true if resources were checked (even if no change). + sig do + params( + formula: Formula, + version: T.nilable(String), + package_name: T.nilable(String), + extra_packages: T.nilable(T::Array[String]), + exclude_packages: T.nilable(T::Array[String]), + print_only: T::Boolean, + silent: T::Boolean, + ignore_non_pypi_packages: T::Boolean, + ).returns(T.nilable(T::Boolean)) + end def update_python_resources!(formula, version: nil, package_name: nil, extra_packages: nil, exclude_packages: nil, print_only: false, silent: false, ignore_non_pypi_packages: false) @@ -73,12 +149,17 @@ module PyPI end end - version ||= formula.version if package_name.blank? - package_name ||= url_to_pypi_package_name formula.stable.url - extra_packages ||= [] - exclude_packages ||= [] + main_package = if package_name.present? + Package.new(package_name) + else + begin + Package.new(formula.stable.url, is_url: true) + rescue ArgumentError + nil + end + end - if package_name.blank? + if main_package.blank? return if ignore_non_pypi_packages odie <<~EOS @@ -87,76 +168,81 @@ module PyPI EOS end - input_package_names = { package_name => version } - extra_packages.each do |extra| - extra_name, extra_version = extra.split "==" + unless main_package.valid_pypi_package? + return if ignore_non_pypi_packages - if input_package_names.key?(extra_name) && input_package_names[extra_name] != extra_version - odie "Conflicting versions specified for the `#{extra_name}` package: "\ - "#{input_package_names[extra_name]}, #{extra_version}" + 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 } + + input_packages = [main_package] + extra_packages.each do |extra_package| + if !extra_package.valid_pypi_package? && !ignore_non_pypi_packages + odie "\"#{extra_package}\" is not available on PyPI." end - input_package_names[extra_name] = extra_version + input_packages.each do |existing_package| + if existing_package.same_package?(extra_package) && existing_package.version != extra_package.version + odie "Conflicting versions specified for the `#{extra_package.name}` package: "\ + "#{existing_package.version}, #{extra_package.version}" + end + end + + input_packages << extra_package unless input_packages.include? extra_package end - input_package_names.each do |name, package_version| - name = name.split("[").first - next if get_pypi_info(name, package_version).present? - - version_string = " at version #{package_version}" if package_version.present? - odie "\"#{name}\"#{version_string} is not available on PyPI." unless ignore_non_pypi_packages - end - - non_pypi_resources = formula.resources.reject do |resource| - resource.url.start_with? PYTHONHOSTED_URL_PREFIX - end - - if non_pypi_resources.present? && !print_only - odie "\"#{formula.name}\" contains non-PyPI resources. Please update the resources manually." + formula.resources.each do |resource| + if !print_only && !resource.url.start_with?(PYTHONHOSTED_URL_PREFIX) + odie "\"#{formula.name}\" contains non-PyPI resources. Please update the resources manually." + end end @pipgrip_installed ||= Formula["pipgrip"].any_version_installed? odie '"pipgrip" must be installed (`brew install pipgrip`)' unless @pipgrip_installed - found_packages = {} - input_package_names.each do |name, package_version| - pypi_package_string = if package_version.present? - "#{name}==#{package_version}" - else - name - end - - ohai "Retrieving PyPI dependencies for \"#{pypi_package_string}\"..." if !print_only && !silent - pipgrip_output = Utils.popen_read Formula["pipgrip"].bin/"pipgrip", "--json", "--no-cache-dir", - pypi_package_string + found_packages = [] + input_packages.each do |package| + ohai "Retrieving PyPI dependencies for \"#{package}\"..." if !print_only && !silent + pipgrip_output = Utils.popen_read Formula["pipgrip"].bin/"pipgrip", "--json", "--no-cache-dir", package.to_s unless $CHILD_STATUS.success? odie <<~EOS - Unable to determine dependencies for \"#{name}\" because of a failure when running - `pipgrip --json --no-cache-dir #{pypi_package_string}`. + Unable to determine dependencies for \"#{package}\" because of a failure when running + `pipgrip --json --no-cache-dir #{package}`. Please update the resources for \"#{formula.name}\" manually. EOS end - found_packages.merge!(JSON.parse(pipgrip_output).to_h) do |conflicting_package, old_version, new_version| - next old_version if old_version == new_version + JSON.parse(pipgrip_output).to_h.each do |new_name, new_version| + new_package = Package.new("#{new_name}==#{new_version}") - odie "Conflicting versions found for the `#{conflicting_package}` resource: #{old_version}, #{new_version}" + found_packages.each do |existing_package| + if existing_package.same_package?(new_package) && existing_package.version != new_package.version + odie "Conflicting versions found for the `#{new_package.name}` resource: "\ + "#{existing_package.version}, #{new_package.version}" + end + end + + found_packages << new_package unless found_packages.include? new_package end end # Remove extra packages that may be included in pipgrip output - exclude_list = %W[#{package_name.split("[").first.downcase} argparse pip setuptools wheel wsgiref] + exclude_list = %W[#{main_package.name} argparse pip setuptools wheel wsgiref].map { |p| Package.new p } found_packages.delete_if { |package| exclude_list.include? package } new_resource_blocks = "" - found_packages.sort.each do |package, package_version| + found_packages.sort.each do |package| if exclude_packages.include? package - ohai "Excluding \"#{package}==#{package_version}\"" if !print_only && !silent + ohai "Excluding \"#{package}\"" if !print_only && !silent next end - ohai "Getting PyPI info for \"#{package}==#{package_version}\"" if !print_only && !silent - name, url, checksum = get_pypi_info package, package_version + ohai "Getting PyPI info for \"#{package}\"" if !print_only && !silent + name, url, checksum = package.pypi_info # Fail if unable to find name, url or checksum for any resource if name.blank? odie "Unable to resolve some dependencies. Please update the resources for \"#{formula.name}\" manually."