Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ruff analyze graph | head -n 10 panics #13442

Closed
tpgillam opened this issue Sep 21, 2024 · 4 comments · Fixed by #13485
Closed

ruff analyze graph | head -n 10 panics #13442

tpgillam opened this issue Sep 21, 2024 · 4 comments · Fixed by #13485
Assignees
Labels
bug Something isn't working

Comments

@tpgillam
Copy link
Contributor

tpgillam commented Sep 21, 2024

It seems like ruff analyze graph is sometimes unhappy when its output isn't fully consumed.

Tested with ruffs 0.6.6 and 0.6.7:

> uv run ruff analyze graph -q | wc -l
2382
> uv run ruff analyze graph -q | head -n 10

<snip 10 correct lines of output>

error: Ruff crashed. If you could open an issue at:

    https://github.com/astral-sh/ruff/issues/new?title=%5BPanic%5D

...quoting the executed command, along with the relevant file contents and `pyproject.toml` settings, we'd be very appreciative!

thread 'main' panicked at library/std/src/io/stdio.rs:1118:9:
failed printing to stdout: Broken pipe (os error 32)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

A full example to reproduce, starting in an otherwise empty directory, would be:

git clone https://github.com/Eomys/pyleecan.git --depth=1
uv init
uv add ruff==0.6.7
uv run ruff analyze graph | head -n 10

edit: above is running on MacOS 14.6.1 . Although head is the GNU version from homebrew's coreutils

@MichaReiser
Copy link
Member

That's probably due to a println usage. We should use writeln instead and propagate the error when the pipe breaks

@MichaReiser MichaReiser added the bug Something isn't working label Sep 21, 2024
@pixelb
Copy link

pixelb commented Sep 22, 2024

Just a suggestion to not propagate pipe "errors", as this is not really an error in the normal sense
https://www.pixelbeat.org/programming/sigpipe_handling.html

@charliermarsh
Copy link
Member

I got it.

@BurntSushi
Copy link
Member

This is how ripgrep handles pipe errors: https://github.com/BurntSushi/ripgrep/blob/bf63fe8f258afc09bae6caa48f0ae35eaf115005/crates/core/main.rs#L55-L61

There are other approaches, but this is my favorite because:

  • It doesn't require any platform specific APIs.
  • It doesn't require inspecting or dealing with errors specially every time writeln! is called. You just propagate it like normal and only treat it differently at the top-level of the program in one place.
  • It matches the expected behavior: a pipe error should make the program stop working and exit gracefully.

But yes, basically any println! automatically introduces a potential bug of this variety.

charliermarsh added a commit that referenced this issue Sep 23, 2024
## Summary

I think we should also make the change that @BurntSushi recommended in
the linked issue, but this gets rid of the panic.

See: #13483

See: #13442

## Test Plan

```
warning: `ruff analyze graph` is experimental and may change without warning
{
  "/Users/crmarsh/workspace/django/django/__init__.py": [
    "/Users/crmarsh/workspace/django/django/apps/__init__.py",
    "/Users/crmarsh/workspace/django/django/conf/__init__.py",
    "/Users/crmarsh/workspace/django/django/urls/__init__.py",
    "/Users/crmarsh/workspace/django/django/utils/log.py",
    "/Users/crmarsh/workspace/django/django/utils/version.py"
  ],
  "/Users/crmarsh/workspace/django/django/__main__.py": [
    "/Users/crmarsh/workspace/django/django/core/management/__init__.py"
ruff failed
  Cause: Broken pipe (os error 32)
```
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants