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

CI Python formatting is confusing #135942

Closed
ehuss opened this issue Jan 23, 2025 · 4 comments · Fixed by #135950
Closed

CI Python formatting is confusing #135942

ehuss opened this issue Jan 23, 2025 · 4 comments · Fixed by #135950
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@ehuss
Copy link
Contributor

ehuss commented Jan 23, 2025

When there is a python formatting error in CI, it prints the following confusing message:

python formatting does not match! Printing diff:
error: unexpected argument '--check' found

  tip: to pass '--check' as a value, use '-- --check'

Usage: ruff check <FILES|--fix|--no-fix|--unsafe-fixes|--no-unsafe-fixes|--show-source|--no-show-source|--show-fixes|--no-show-fixes|--diff|--watch|--fix-only|--no-fix-only|--ignore-noqa|--output-format <OUTPUT_FORMAT>|--output-file <OUTPUT_FILE>|--target-version <TARGET_VERSION>|--preview|--no-preview|--select <RULE_CODE>|--ignore <RULE_CODE>|--extend-select <RULE_CODE>|--extend-ignore <RULE_CODE>|--per-file-ignores <PER_FILE_IGNORES>|--extend-per-file-ignores <EXTEND_PER_FILE_IGNORES>|--exclude <FILE_PATTERN>|--extend-exclude <FILE_PATTERN>|--fixable <RULE_CODE>|--unfixable <RULE_CODE>|--extend-fixable <RULE_CODE>|--extend-unfixable <RULE_CODE>|--respect-gitignore|--no-respect-gitignore|--force-exclude|--no-force-exclude|--line-length <LINE_LENGTH>|--dummy-variable-rgx <DUMMY_VARIABLE_RGX>|--no-cache|--cache-dir <CACHE_DIR>|--stdin-filename <STDIN_FILENAME>|--extension <EXTENSION>|--exit-zero|--exit-non-zero-on-fix|--statistics|--add-noqa|--show-files|--show-settings|--ecosystem-ci>

Some issues:

  • The diff is obviously not displayed.
  • There's no information on how to check this locally.
  • I can't find any documentation in the rustc-dev-guide about this.
  • Setting this up locally seems hard:
    • Dig through CI scripts until you find the magic incantation
    • ./x test tidy --extra-checks=py returns an error about virtualenv not installed
    • The error message is wrong for my environment, it says python3.11 but I have python3.12
    • python3 -m pip install virtualenv returns an error about an externally-managed-environment
    • brew install virtualenv seemed to do something
    • Wait, no, tidy is right about python3.11, but that's not what I use. python3.11 -m pip install virtualenv does something
    • ./x test tidy --extra-checks=py fails with something about ruff not being installed
    • brew install ruff seemed to do something
    • ./x test tidy --extra-checks=py now seems to work 🎉 but it does not show me the diff 😦
    • ./x test tidy --extra-checks=py:fmt --bless updated the file
@ehuss ehuss added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Jan 23, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 23, 2025
@onur-ozkan
Copy link
Member

cc @Kobzol

@onur-ozkan onur-ozkan removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 23, 2025
@Kobzol
Copy link
Contributor

Kobzol commented Jan 23, 2025

Could you please send a link to the workflow where the formatting error was reported? Locally it shows me this:

checking python file formatting
Would reformat: src/ci/github-actions/ci.py
rerun tidy with `--extra-checks=py:fmt --bless` to reformat Python code

@Kobzol
Copy link
Contributor

Kobzol commented Jan 23, 2025

Ah, TIDY_PRINT_DIFF=1 was needed to replicate. I will investigate and send a PR with a fix.

@Kobzol
Copy link
Contributor

Kobzol commented Jan 23, 2025

#135950 should fix the mentioned issues. I documented the Python checks in the rustc dev guide, fixed the diff problem, and also improved the creation of virtual environments (and its error messages :) ).

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 24, 2025
…=onur-ozkan

Tidy Python improvements

Fixes display of Python formatting diffs in tidy, and refactors the code to make it simpler and more robust. Also documents Python formatting and linting in the Rustc dev guide.

Fixes: rust-lang#135942

r? `@onur-ozkan`
@bors bors closed this as completed in c459b17 Jan 24, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 24, 2025
Rollup merge of rust-lang#135950 - Kobzol:tidy-python-improvements, r=onur-ozkan

Tidy Python improvements

Fixes display of Python formatting diffs in tidy, and refactors the code to make it simpler and more robust. Also documents Python formatting and linting in the Rustc dev guide.

Fixes: rust-lang#135942

r? `@onur-ozkan`
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this issue Jan 27, 2025
Tidy Python improvements

Fixes display of Python formatting diffs in tidy, and refactors the code to make it simpler and more robust. Also documents Python formatting and linting in the Rustc dev guide.

Fixes: rust-lang/rust#135942

r? `@onur-ozkan`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants