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

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)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants