-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
pypi: do not use formula name as PyPI package name #8732
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: do not use formula name as PyPI package name #8732
Conversation
9cf48b5
to
345e6a0
Compare
I'm wondering if it might make more sense to remove the fallback to the formula name instead of having a new blocklist: https://github.com/Homebrew/brew/blob/master/Library/Homebrew/utils/pypi.rb#L73-L75 @Rylan12 what do you think? Are there any/many cases where the primary url is a non-PyPI url but we want to update the resources based on pipgrip output for the Python module? |
I'm not totally opposed to removing it. It means that we won't catch cases where the Have we discussed adding a message or label to the PR that If we add something like this, I would feel much better about removing the fallback. I agree, though, having two different exclude lists for this feels wrong. TL;DR: I have no sense of how many formulae are available on pypi and don't use the pypi url. If it's pretty small, not a problem (maybe these should be switched to use the pypi url anyway). Separately, it might be good to add a message to PRs that indicates whether resources were checked automatically. |
This seems like a good idea 👍🏻 |
In that case, I'd say we should remove the fallback instead of adding a second list. We can monitor PRs to see how many don't get auto-updated when they should and add it back if there are many instances. |
About 1,445 formulae match a PyPI package name: https://gist.github.com/SeekingMeaning/d440ec4496795049f6f576d9f7760218 Of these, 127 have a stable url referencing Adding the two numbers together, 208 (14.4%) of the 1,445 formulae matching a PyPI package name are using PyPI. Of all formulae in In summary, 259 (5%) of the 5,221 formulae in |
345e6a0
to
d6c056d
Compare
@SeekingMeaning is this table correct? If not, can you correct it?
To clarify: ✅ and ❌ mean yes and no rather than good and bad Here's a summary of how each category is handled:Pinging @Homebrew/core for thoughts on how all of the categories will be handled Category 1: Auto updates Category 2: No longer auto-updates but Category 3: No longer checks for new resources. This will likely be a minor problem because if the formulae start needing resources in the future we will notice because the formula won't build. At that point, the stable url could be updated to a Category 4: I don't think this category can exist... 🤔 Nothing should happen, though. Category 5: These cannot be auto-updated. Category 6: Nothing should happen. |
Accurate description ✅ Special case: this category includes
There's probably a possibility that some of these aren't on PyPI — there might be cases where the formula happens to have the same name as an unrelated PyPI package and the actual PyPI package has a different name or does not exist Special case: this category includes
Most of these are probably not PyPI packages
These are PyPI packages — the package name is different from the formula name (ex: the formula
This is a combination of categories 2 and 4 — some packages might not be on PyPI, and those that are (if any) have a different name from the formula name
Some of these use brew's List of formulae for categories 1, 2, 4, and 5:
|
Thanks for the clarification. I've updated the table to (I think) better match your descriptions. The bottom line is, formulae should use a As before, there is a blocklist that handles the exceptions for formulae in categories 1 or 4 that need special attention (9 formulae in total, I believe). |
Looks good!
I forget which formula, but there is at least one package that does not provide source code on PyPI — I'm not sure what the best way to handle this would be (One approach would be to add another field or two to the
|
Can we just add it to the blocklist?
The only reason I can see not to allow them is to force the contributor to create a PR manually (being on the blocklist makes |
From my perspective, it's appropriate to discourage folks from creating low-effort version bump PRs for |
Yes, that would probably be simplest
Nevermind, I wasn't thinking 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given there have been no major opinions either way, I think this can move forward.
Just one minor change that would be nice but won't block.
Also, I think that xonsh
can be removed from the blocklist. I think keeping the ansible
formulae around is a good idea but I don't think xonsh
has resources updated nearly as often. Again, not a blocker.
Library/Homebrew/utils/pypi.rb
Outdated
|
||
odie <<~EOS | ||
Could not infer PyPI package name from URL: | ||
#{formula.stable.url} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#{formula.stable.url} | |
#{Formatter.url(formula.stable.url)} |
d6c056d
to
cf37ff6
Compare
brew style
with your changes locally?brew tests
with your changes locally?This fixes
brew bump-formula-pr
forcdk8s
,juju
, andpulumi
as they aren't Python packages and don't use any PyPI resourcesBefore:
After: