Fixes a regression in the ecosystem checks from
https://github.com/astral-sh/ruff/pull/9687 which was causing them to
run for multiple hours due to the size of the output.
We need the concise format for comparisons.
We should probably update the ecosystem checks to actually diff the full
output in the future because that'd be nice.
# Conflicts:
# python/ruff-ecosystem/ruff_ecosystem/projects.py
Updated implementation of https://github.com/astral-sh/ruff/pull/7369
which was left out in the cold.
This was motivated again following changes in #9691 and #9689 where we
could not test the changes without actually deprecating or removing
rules.
---
Follow-up to discussion in https://github.com/astral-sh/ruff/pull/7210
Moves integration tests from using rules that are transitively in
nursery / preview groups to dedicated test rules that only exist during
development. These rules always raise violations (they do not require
specific file behavior). The rules are not available in production or in
the documentation.
Uses features instead of `cfg(test)` for cross-crate support per
https://github.com/rust-lang/cargo/issues/8379
Uses our existing configuration overrides to unset the
`required-version` option in ecosystem projects during checks.
The downside to this approach, is we will now update the config file of
_every_ project (with a config file). This roughly normalizes the configuration file, as we
don't preserve comments and such. We could instead do a more targeted
approach applying this override to projects which we know use this
setting 🤷♀️
This saves a handful of milliseconds on Windows and even more on other
platforms when running `python -m ruff`. On non-Windows systems the
process is replaced directly (impossible on Windows unfortunately).
```
❯ docker run --rm python:3.11 bash -c "for i in {1..15}; do python -m timeit -n 1 -r 1 'from pathlib import Path'; done"
1 loop, best of 1: 25.7 msec per loop
1 loop, best of 1: 3.07 msec per loop
1 loop, best of 1: 3.16 msec per loop
1 loop, best of 1: 3.06 msec per loop
1 loop, best of 1: 3.32 msec per loop
1 loop, best of 1: 3.93 msec per loop
1 loop, best of 1: 3.26 msec per loop
1 loop, best of 1: 3.73 msec per loop
1 loop, best of 1: 3.1 msec per loop
1 loop, best of 1: 3.29 msec per loop
1 loop, best of 1: 3.12 msec per loop
1 loop, best of 1: 3.05 msec per loop
1 loop, best of 1: 3.04 msec per loop
1 loop, best of 1: 3.19 msec per loop
1 loop, best of 1: 3.04 msec per loop
❯ docker run --rm python:3.11 bash -c "for i in {1..15}; do python -m timeit -n 1 -r 1 'import subprocess'; done"
1 loop, best of 1: 31.2 msec per loop
1 loop, best of 1: 3.75 msec per loop
1 loop, best of 1: 4.71 msec per loop
1 loop, best of 1: 3.88 msec per loop
1 loop, best of 1: 4.08 msec per loop
1 loop, best of 1: 4.35 msec per loop
1 loop, best of 1: 3.94 msec per loop
1 loop, best of 1: 4.06 msec per loop
1 loop, best of 1: 3.88 msec per loop
1 loop, best of 1: 3.85 msec per loop
1 loop, best of 1: 3.84 msec per loop
1 loop, best of 1: 4.01 msec per loop
1 loop, best of 1: 4.21 msec per loop
1 loop, best of 1: 4.07 msec per loop
1 loop, best of 1: 4.11 msec per loop
❯ python -m timeit -n 1 -r 1 "from pathlib import Path"
1 loop, best of 1: 5.25 msec per loop
❯ python -m timeit -n 1 -r 1 "import subprocess"
1 loop, best of 1: 7.61 msec per loop
```
Adds the ability to override `ruff.toml` or `pyproject.toml` settings
per-project during ecosystem checks.
Exploring this as a fix for the `setuptools` project error.
Also useful for including Jupyter Notebooks in the ecosystem checks, see
#9293
Note the remaining `sphinx` project error is resolved in #9294
Failing due to
> error: Failed to read tests/roots/test-pycode/cp_1251_coded.py: stream
did not contain valid UTF-8
Unclear to me if ignoring is the correct response.
Instead of displaying the total completed project count in the "changed"
section of a header, we now separately calculated the changed and
unchanged count to make the header message nice and clear.
e.g.
> ℹ️ ecosystem check **detected format changes**. (+1772 -1859 lines in
239 files in 26 projects; 6 project errors; 9 projects unchanged)
and
> ℹ️ ecosystem check **detected linter changes**. (+4598 -5023
violations, +0 -40 fixes in 13 projects; 4 project errors; 24 projects
unchanged)
Previously, it would have included the unchanged count in the first
project count.
Extends https://github.com/astral-sh/ruff/pull/8416 activating the
`black-and-ruff` and `black-then-ruff` formatter comparison modes for
ecosystem checks allowing us to compare changes to Black across the
ecosystem.
Previously, the ecosystem checks formatted with the baseline then
formatted again with `--diff` to get the changed files.
Now, the ecosystem checks support a new mode where we:
- Format with the baseline
- Commit the changes
- Reset to the target ref
- Format again
- Check the diff from the baseline commit
This effectively tests Ruff changes on unformatted code rather than
changes in previously formatted code (unless, of course, the project is
already using Ruff).
While this mode is the new default, I've retained the old one for local
checks. The mode can be toggled with `--format-comparison <type>`.
Includes some more aggressive resetting of the GitHub repositories when
cached.
Here, I've also stubbed comparison modes in which `black` is used as the
baseline. While these do nothing here, #8419 adds support.
I tested this with the commit from #8216 and ecosystem changes appear
https://gist.github.com/zanieb/a982ec8c392939043613267474471a6e
Until https://github.com/astral-sh/ruff/issues/8076 is ready, it seems
beneficial to get feedback on preview mode changes.
Tested locally, updated logs to output the flags passed to `ruff` and
verified `--preview` is used.
Fixes bug where `total_affected_rules` is empty, a division by zero
error can occur if there are only errors and no rule changes. Calculates
the maximum display per rule with the calculated project maximum as the
upper bound instead of 50, this should show more rule variety when
project maximums are lower.
This commit was meant to be in #8223 but I missed it.
Closes#7239
- Refactors `scripts/check_ecosystem.py` into a new Python project at
`python/ruff-ecosystem`
- Includes
[documentation](https://github.com/astral-sh/ruff/blob/zanie/ecosystem-format/python/ruff-ecosystem/README.md)
now
- Provides a `ruff-ecosystem` CLI
- Fixes bug where `ruff check` report included "fixable" summary line
- Adds truncation to `ruff check` reports
- Otherwise we often won't see the `ruff format` reports
- The truncation uses some very simple heuristics and could be improved
in the future
- Identifies diagnostic changes that occur just because a violation's
fix available changes
- We still show the diff for the line because it's could matter _where_
this changes, but we could improve this
- Similarly, we could improve detection of diagnostic changes where just
the message changes
- Adds support for JSON ecosystem check output
- I added this primarily for development purposes
- If there are no changes, only errors while processing projects, we
display a different summary message
- When caching repositories, we now checkout the requested ref
- Adds `ruff format` reports, which format with the baseline then the
use `format --diff` to generate a report
- Runs all CI jobs when the CI workflow is changed
## Known problems
- Since we must format the project to get a baseline, the permalink line
numbers do not exactly correspond to the correct range
- This looks... hard. I tried using `git diff` and some wonky hunk
matching to recover the original line numbers but it doesn't seem worth
it. I think we should probably commit the formatted changes to a fork or
something if we want great results here. Consequently, I've just used
the start line instead of a range for now.
- I don't love the comment structure — it'd be nice, perhaps, to have
separate headings for the linter and formatter.
- However, the `pr-comment` workflow is an absolute pain to change
because it runs _separately_ from this pull request so I if I want to
make edits to it I can only test it via manual workflow dispatch.
- Lines are not printed "as we go" which means they're all held in
memory, presumably this would be a problem for large-scale ecosystem
checks
- We are encountering a hard limit with the maximum comment length
supported by GitHub. We will need to move the bulk of the report
elsewhere.
## Future work
- Update `ruff-ecosystem` to support non-default projects and
`check_ecosystem_all.py` behavior
- Remove existing ecosystem check scripts
- Add preview mode toggle (#8076)
- Add a toggle for truncation
- Add hints for quick reproduction of runs locally
- Consider parsing JSON output of Ruff instead of using regex to parse
the text output
- Links to project repositories should use the commit hash we checked
against
- When caching repositories, we should pull the latest changes for the
ref
- Sort check diffs by path and rule code only (changes in messages
should not change order)
- Update check diffs to distinguish between new violations and changes
in messages
- Add "fix" diffs
- Remove existing formatter similarity reports
- On release pull request, compare to the previous tag instead
---------
Co-authored-by: konsti <konstin@mailbox.org>
## Summary
From the [subprocess
docs](https://docs.python.org/3/library/subprocess.html#subprocess.Popen):
> Changed in version 3.6: args parameter accepts a path-like object if
shell is False and a sequence containing path-like objects on POSIX.
>
> Changed in version 3.8: args parameter accepts a path-like object if
shell is False and a sequence containing bytes and path-like objects on
Windows.
We want to support python 3.7 on windows, so we need to convert the
`Path` into a `str`
Fixes#5077
## Summary
Previously, in a powershell on windows when using `python -m ruff`
instead of `ruff` a call such as `python -m ruff "a b.py"` would fail
because the space would be split into two arguments.
The python docs
[recommend](https://docs.python.org/3/library/os.html#os.spawnv) using
subprocess instead of os.spawn variants, which does fix the problem.
## Test Plan
I manually confirmed that the problem previously occurred and now
doesn't anymore. This only happens in a very specific environment
(maturin build, windows, powershell), so i could try adding a step on CI
for it but i don't think it's worth it.
```
(.venv) PS C:\Users\Konstantin\PycharmProjects\ruff> python -m ruff "a b.py"
warning: Detected debug build without --no-cache.
error: Failed to lint a: The system cannot find the file specified. (os error 2)
error: Failed to lint b.py: The system cannot find the file specified. (os error 2)
a:1:1: E902 The system cannot find the file specified. (os error 2)
b.py:1:1: E902 The system cannot find the file specified. (os error 2)
Found 2 errors.
(.venv) PS C:\Users\Konstantin\PycharmProjects\ruff> python -m ruff "a b.py"
warning: Detected debug build without --no-cache.
a b.py:2:5: F841 [*] Local variable `x` is assigned to but never used
Found 1 error.
[*] 1 potentially fixable with the --fix option.
```
Prefer the version from a currently active virtualenv over a version
from `pip install --user`. Add the .exe extension on Windows, and
find the path for `pip install --user` correctly on Windows.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
