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

Update ruff to version 0.5.5 #21235

Closed

Conversation

krishnan-chandra
Copy link
Contributor

@krishnan-chandra krishnan-chandra commented Jul 30, 2024

It's been about a month and a half since last update. I'll try to keep bumping the version on a relatively frequent, but hopefully not annoying interval.

@krishnan-chandra krishnan-chandra added category:internal CI, fixes for not-yet-released features, etc. backend: Python Python backend-related issues labels Jul 30, 2024
@krishnan-chandra
Copy link
Contributor Author

krishnan-chandra commented Jul 30, 2024

Looks like tests are failing because the Pip version was bumped alongside Pex and no longer supports Python 3.7. I can regenerate the lockfile with a lower version of Pip, but we'll probably run into this again.

Related issues: #21103 #20852

@huonw
Copy link
Contributor

huonw commented Jul 31, 2024

This prompted me to dust of a branch and put up #21237, thanks.

That will allow side-stepping the interpreter constraint issues, and also makes supporting many ruff versions simpler (i.e. there's less annoyance from bumping often, because users can pin easily).

Can you take a look and let me know what you think?

@krishnan-chandra
Copy link
Contributor Author

There have been many more updates to ruff since I opened this PR, so I'll just close this and make a new PR at a later time after #21237 is merged.

@krishnan-chandra krishnan-chandra deleted the upgrade-ruff-0.5.5 branch September 6, 2024 17:42
huonw added a commit that referenced this pull request Oct 30, 2024
This switches the Ruff subsystem to download and run it via the
artifacts published to GitHub releases.

Ruff is published to PyPI and thus runnable as a PEX, which is what
Pants currently does... but under the hood ruff is a single binary, and
the PyPI package is for convenience. The package leads to a bit of extra
overhead (e.g. have to build a `VenvPex` before being able to run it).
It is also fiddly to change the version of a Python tool, requiring
building a resolve and using `python_requirement`s.

By downloading from GitHub releases, we can:

- more easily support multiple ruff versions and allow users to pin to
one/switch between them with a `version = "..."` (this PR demonstrates
this, by including both 0.6.4 as in `main`, and 0.4.9 as in 2.23, i.e.
if someone upgrades to 2.24 and wants to stick with 0.4.9, they can just
add `version = "0.4.9"`, no need to fiddle with `known_versions`)
- eliminate any Python/pex overhead by invoking the binary directly
- side-step fiddle like interpreter constraints
(#21235 (comment))

Potential issues:

- If Ruff adds plugins via Python in future, maybe we will want it
installed in a venv so that the venv can include those plugins...
astral-sh/ruff#283 seems to be inconclusive
about the direction, so I'm inclined to not worry and deal with it in
future, if it happens.

This PR does:

- Switches the ruff subsystem to be a `TemplatedExternalTool` subclass,
not `PythonToolBase`
- Plumbs through the requisite changes, including: setting up some
`default_known_versions` for the `main` and 2.23 versions (0.6.4 and
0.4.9 respectively), changing how the tool is installed, removing
metadata about interpreter constraints that is no longer relveant or
computable
- Eases the upgrade by adding deprecated instances of the fields
provided by `PythonToolBase`: people may've customised the Ruff version
with `install_from_resolve`. If the field was just removed, running any
pants command will fail on start-up with `Invalid option
'install_from_resolve' under [ruff] in path/to/pants.toml`, which isn't
very helpful. By having the fields exist, we ensure the user gets a
descriptive warning about what to do. NB. the backend is labelled
experimental, so I haven't tried to preserve behaviour, just focused on
ensuring we can show a removal error message to them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants