-
Notifications
You must be signed in to change notification settings - Fork 1.6k
RFC [ruff][ext-lint] 3: external lint interpreter using PyO3 #21415
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
base: main
Are you sure you want to change the base?
RFC [ruff][ext-lint] 3: external lint interpreter using PyO3 #21415
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
| } | ||
| }) | ||
| } | ||
| #[cfg(not(Py_GIL_DISABLED))] |
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.
This is a holdover from trying to support both gil and non-gil builds. The current state is that only non-GIL really works, so will clean this up.
|
| SOURCE_DIR="Python-${PYTHON_VERSION}" | ||
| DOWNLOAD_URL="https://www.python.org/ftp/python/${PYTHON_VERSION}/${TARBALL}" | ||
|
|
||
| echo "Downloading ${DOWNLOAD_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.
Can't we use pbs instead of building Python manually?
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.
That'd probably make more sense. Ddoes PBS have configs that include freethreading as a flag already?
|
Thank you, this is great. Thanks for taking the time to hack on a prototype. I haven't read the code in detail, so forgive me if any of my feedback or questions should be obvious from the code.
Do you have any numbers you can share. How does the performance of this branch compare to:
My biggest concern (other than performance) are:
|
|
@MichaReiser Thanks for the initial look! I'll keep cleaning this up a little, since it (this PR in particular) is a bit rougher than the Performance stuffI can post some flamegraphs that compare running G004 (with AST and general API stuff
I can put up more code to show what this might look like, but it'll take me a few days, most likely. What I have cooking currently is:
...but let me put up the code once it's semi-presentable. +1 on versioning the API. I actually had a note to add that to the TOML (linters specify which version of the API they were built again). Integration/Distribution
I don't think I know enough about the whole ecosystem to be helpful here. At minimum, doing Aside from that, from my/our point of view, we really want each rule to have some tests, so ideally Ruff would have specific options for that. Other languages
I think that makes sense. We can chat about what are the best options to try. I have code lying around from when I tried RHAI (1 month ago) and RustPython (a few weeks ago). |
cea3810 to
57f73c9
Compare
|
Rebase; some cleanup:
|
Summary
Note
This is a stacked PR; the two base commits are PRs #21413 and #21412 . The
[ruff][ext-lint] set up linter runtimecommit is the actual delta for this PR.This PR brings in the PyO3 dependencies and sets up per-Rayon-thread, per-distinct-config environments to actually run external linters. The build requires
--features ext-lint; without that it does not include the PyO3 dependency.The AST that gets ‘projected’ to the individual external linters is still quite minimal, but it gets us an end-to-end example (a G004-like “don’t log eagerly interpolated strings” rule).
There are a bunch of constraints here that restrict what we can do:
The easiest way to get to a working state is to build PyO3 >= 0.23 against a CPython build with freethreading support. In that case, it’s well-defined for multiple Rust threads to call into the same interpreter and execute code, so long as each OS thread is ‘attached.’
The main downside is that we have limited isolation; we do some module name mangling, but a misbehaving rule could modify Python-side global state that would affect other rules. On the plus side, performance for this implementation is (surprisingly) good.
Python interface: For now, we send over a very barebones node object, and require external rules to implement
check_stmtorcheck_exprthat take a node and a context object as input. The goal was to get to a somewhat-minimal running example; this interface will need more work.Deployment: this requires building against a freethreading CPython (3.13 with
--disable-gilor 3.14). To avoid making that a prerequisite for every Ruff build, theext-lintfeature is disabled by default. I added some scripts for doing a CPython 3.13--disable-gilbuild; see the test plan.Test Plan