Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump-formula-pr: expose update-python-resources CLI flags #13147

Merged
merged 1 commit into from
Apr 22, 2022
Merged

bump-formula-pr: expose update-python-resources CLI flags #13147

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 14 additions & 2 deletions Library/Homebrew/dev-cmd/bump-formula-pr.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,13 @@ def bump_formula_pr_args
"or specified <version>."
switch "-f", "--force",
description: "Ignore duplicate open PRs. Remove all mirrors if `--mirror` was not specified."
flag "--python-package-name=",
description: "Use the specified <package-name> when finding Python resources for <formula>. "\
"If no package name is specified, it will be inferred from the formula's stable URL."
comma_array "--python-extra-packages=",
description: "Include these additional Python packages when finding resources."
comma_array "--python-exclude-packages=",
description: "Exclude these Python packages when finding resources."
Comment on lines +81 to +87
Copy link
Author

@pmrowla pmrowla Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the preferred flag ordering is here (specifically regarding whether the new args need to be inserted somewhere else in the args list)

Also went with --python-<flag> naming instead of the original suggested --extra-python-packages for consistency (python in the middle works for include/exclude but not --package-name)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: do you personally need all three of these arguments?

Copy link
Author

@pmrowla pmrowla Apr 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because PyPI.update_python_resources only loads the contents of pypi_formula_mappings.json if none of these flags are specified via the CLI.

brew/Library/Homebrew/utils/pypi.rb

Lines 141 to 143 in ce5cb9b

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?

So in order to get the behavior I'm looking for (to build DVC with additional --python-extra-packages hints to be passed into pipgrip), I have to also specify the package name + excludes via the CLI (even though they are listed in pypi_formula_mappings.json)


conflicts "--dry-run", "--write-only"
conflicts "--dry-run", "--write"
Expand Down Expand Up @@ -329,8 +336,13 @@ def bump_formula_pr
end

unless args.dry_run?
resources_checked = PyPI.update_python_resources! formula, version: new_formula_version,
silent: args.quiet?, ignore_non_pypi_packages: true
resources_checked = PyPI.update_python_resources! formula,
version: new_formula_version,
package_name: args.python_package_name,
extra_packages: args.python_extra_packages,
exclude_packages: args.python_exclude_packages,
silent: args.quiet?,
ignore_non_pypi_packages: true
end

run_audit(formula, alias_rename, old_contents, args: args)
Expand Down