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.

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

PyPI: Handle non-pythonhosted formula URLs #15617

Conversation

woodruffw
Copy link
Member

  • 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?

This is take two on #15511.

From the original:

This is another quality of life improvement to our handling of Python packages, unlocked by the switch to pip install --report --dry-run from a few weeks ago: pip install can accept arbitrary distribution URLs, meaning that we no longer require the top-level to be a pythonhosted URL in order to auto-bump its dependencies using update-python-resources.

This should result in automation for a handful of formulae that previously required manual resource updating. More indirectly, it improves brew-pip-audit's ability to bulk audit Python packages present in Homebrew formulae, resulting in more automated vulnerability fix PRs as well.

if main_package.valid_pypi_package?
main_package.version = version
else
return if ignore_non_pypi_packages
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only change between this PR and #15511: some callers like bump-formula-pr expect to be able to call update_python_resources! on arbitrary formulae and have it exit silently.

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Member Author

CC @p-linnane in particular: I believe I've covered the error mode you noticed, but I'm not super familiar with the various bump workflows these days 🙂

Emphasize that we're failing because the user tried to update
a non-PyPI package by version, when only PyPI packages can
be updated by version. Other Python packages need to be updated
by a full URL change.

Signed-off-by: William Woodruff <william@yossarian.net>
Copy link
Member

@p-linnane p-linnane left a comment

Choose a reason for hiding this comment

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

Verified that the error I was seeing last time has been resolved. LGTM, thanks @woodruffw!

@woodruffw
Copy link
Member Author

CI failures here appear to be flakes.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks good, thanks again @woodruffw!

@MikeMcQuaid MikeMcQuaid merged commit ccad81b into Homebrew:master Jul 4, 2023
24 checks passed
@woodruffw woodruffw deleted the ww/update-resources-handle-non-pypi-urls branch July 4, 2023 14:29
@woodruffw
Copy link
Member Author

Thank you both! Fingers crossed on breakage this time 🙂

@github-actions github-actions bot added the outdated PR was locked due to age label Aug 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants