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

[pmrowla]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

Exposes CLI flags from update-python-resources so they can be used in bump-formula-pr:

• brew update-python-resources --package-name -> brew bump-formula-pr --python-package-name
• --extra-packages -> --python-extra-packages
• --exclude-packages -> --python-exclude-packages

Original PR changes (outdated)

- Add support for HOMEBREW_PIPGRIP_ADDITIONAL_DEPENDENCIES which will be propagated into PIPGRIP_ADDITIONAL_DEPENDENCIES when calling pipgrip

pipgrip 0.8.0 adds support for the PIPGRIP_ADDITIONAL_DEPENDENCIES environment variable which can be used to pass additional dependency information into pipgrip. When using the homebrew dev tools (update-python-resources and bump-formula-pr) this cannot be propagated into pipgrip due to the env var filtering (unless you also set the hidden+deprecated HOMEBREW_NO_ENV_FILTERING env var).

This PR adds an equivalent HOMEBREW_PIPGRIP_ADDITIONAL_DEPENDENCIES env var so that it can be passed into pipgrip through the filtering.

As to why this is needed, basically pipgrip has problems resolving dependencies for DVC due to misbehaving/broken pypi packages (that we don't have control over). This env var can be used to provide hints for specific versions pipgrip should try in order to successfully resolve the dependencies. See:

• dvc 2.10.x fail to build iterative/homebrew-dvc#50
• Skip package versions when wheel fails to build ddelange/pipgrip#77

[MikeMcQuaid]

Thanks for the PR! I think I'd like to see this use e.g. new CLI flags rather than undocumented environment variables. Thoughts?

[Bo98]

What's the benefits of this over the already existing extra_packages feature?

extra_packages has the benefit to applying to everyone who wants to bump the same formula. This env does not.

[pmrowla]

Thanks for the PR! I think I'd like to see this use e.g. new CLI flags rather than undocumented environment variables. Thoughts?

I should note that this could be done through the --extra-packages flag in brew update-python-resources. The problem is that brew bump-formula-pr calls PyPI.update_python_resources directly and does not support any of the CLI flags used in the update-python-resources CLI command.

Adding support for all of the same flags for including/excluding python extras in the bump formula script would technically work for us. (see followup note on this in #13147 (comment))

What's the benefits of this over the already existing extra_packages feature?

I'd say that the main difference between this and the existing extra_packages feature is that I see extra_packages as the method for specifying extra python packages which are actually required by the homebrew formula. And this env var is for providing hints to pipgrip so that the pipgrip resolver works properly.

These packages aren't direct DVC requirements, they are basically dependencies of dependencies of dependencies. The required version ranges are also specified correctly in the dependency python packages. The issue is that pipgrip crashes when trying to resolve the particular combination of python packages that end up being used in DVC (see the linked pipgrip issue in the first PR comment for specific details).

From the DVC maintainer perspective, we see this specifically as a pipgrip issue (and not a DVC requirements/dependency issue), since pip is able to resolve dependencies for DVC without needing to pin or hint any additional package version information.

We would prefer not to pin these types of nested dependencies in the homebrew-core pypi_formula_mappings.json entry for DVC, since we have no control over the packages that require the problematic nested dependencies. (And needing to manually maintain nested dependency package versions in pypi_formula_mappings.json would seem like it defeats the purpose of having a dependency resolver in the first place)

extra_packages has the benefit to applying to everyone who wants to bump the same formula. This env does not.

This is a valid point. But given that anyone who wants to bump this formula right now will still end up needing to manually run pipgrip to see where it crashes, find the correct set of hints to feed it, and then edit pypi_formula_mappings.json themselves, I don't think the existing conveniency of the extra_packages feature applies in this case.

[pmrowla]

I guess from the DVC maintainer perspective, ideally we just want a solution that won't require manually updating pinned dependency versions in pypi_formula_mappings.json for each DVC release, since it means we can't easily automate the use of bump-formula-pr in CI.

Maybe the best solution here would be to expose the include/exclude extras CLI flags in bump-formula-pr, and then anyone making use of the pipgrip env var could just do it via

brew bump-formula-pr --extra-packages "$PIPGRIP_ADDITIONAL_DEPENDENCIES"

This still doesn't address the "make it easy for anyone to bump the given formula" problem, but I'm not sure there's a solution to that for this particular case w/DVC and pipgrip.

---

edit: Actually I just noticed that using update-python-resources --extra-packages will only search the packages provided via the CLI, and what we're really looking for is "use the existing list of extras from pypi_formula_mappings.json plus these additional hints", so it wouldn't be quite as straightforward as exposing the existing flags in bump-formula-pr

[MikeMcQuaid]

Adding support for all of the same flags for including/excluding python extras in the bump formula script would technically work for us. (see followup note on this in #13147 (comment))

Maybe the best solution here would be to expose the include/exclude extras CLI flags in bump-formula-pr, and then anyone making use of the pipgrip env var could just do it via

brew bump-formula-pr --extra-packages "$PIPGRIP_ADDITIONAL_DEPENDENCIES"

--extra-python-packages but: yes, this makes sense to me.

[pmrowla]

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)

[MikeMcQuaid]

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

[pmrowla]

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)

[pmrowla]

Updated this PR to expose the relevant CLI flags in bump-formula-pr as discussed (and removed the env var changes)

[MikeMcQuaid]

One question here. Code looks good!

[MikeMcQuaid]

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

[MikeMcQuaid]

Thanks so much for your first contribution! Without people like you submitting PRs we couldn't run this project. You rock, @pmrowla!

[pmrowla]

Looks like CI needs to be re-run? The failure is due to GHA rate limiting

GitHub::API::AuthenticationFailedError:
  GitHub API Error: You have exceeded a secondary rate limit. Please wait a few minutes before you try again.