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
Add additional directories with register.py to pants bin deps (Cherry pick of #19848) #19855
Conversation
I compared the output of `find . -name register.py` and the dependencies of the `src/python/pants/bin:plugins` target: this identified several backends that seemingly weren't being packaged into the Pants binary, and thus not available for users, similar to #19836. - `pants.backend.experimental.openapi.codegen.java` - `pants.backend.experimental.python.framework.django` - `pants.backend.experimental.swift` - `pants.backend.python.providers.experimental.pyenv.custom_install` Two of these (Django and Rust) also had minor issues the caused errors when being loaded, which this PR resolves. There were also two that have been upgraded from experimental to stable, that were depended upon via the old/deprecated experimental name, but not the stable name: - `pants.backend.python.lint.autoflake` - `pants.backend.python.lint.pyupgrade` This doesn't try to solve the problem of keeping this up to date. I can think of two approaches (definitely for a separate PR): - #17301, with a test that reads the `BUILD` file and compares the dependencies to a `find` for `register.py` files - a target that exposes the output of `pants peek ::` as a file, with a test that reads that file and does the appropriate validation
I'm not so sure about cherry-picking this back to be a patch release. |
Oh wait, I guess you ran this specifically on the 2.17.x branch? That makes sense. Nevermind me. Late morning 😅 |
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.
There ought to be a deprecation warning to the user if they enable the experimental version when there is a stable version available, so that we can drop the experimental one in a future release.
I always wanted to make a |
Yeah, both pants/src/python/pants/backend/experimental/python/lint/autoflake/register.py Lines 12 to 15 in 87522c7
pants/src/python/pants/backend/experimental/python/lint/pyupgrade/register.py Lines 13 to 16 in 87522c7
|
I compared the output of
find . -name register.py
and the dependencies of thesrc/python/pants/bin:plugins
target: this identified several backends that seemingly weren't being packaged into the Pants binary, and thus not available for users, similar to #19836.pants.backend.experimental.openapi.codegen.java
pants.backend.experimental.python.framework.django
pants.backend.experimental.swift
pants.backend.python.providers.experimental.pyenv.custom_install
Django also had minor issues the caused errors when being loaded, which this PR resolves.
There were also two that have been upgraded from experimental to stable, that were depended upon via the old/deprecated experimental name, but not the stable name:
pants.backend.python.lint.autoflake
pants.backend.python.lint.pyupgrade
This doesn't try to solve the problem of keeping this up to date. I can think of two approaches (definitely for a separate PR):
BUILD
file and compares the dependencies to afind
forregister.py
filespants peek ::
as a file, with a test that reads that file and does the appropriate validation(This is a manually tailored cherry pick of #19848 to be appropriate for 2.17.)