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

GH-109408: Move the Python file whitespace check from patchcheck to pre-commit #109891

Merged
merged 5 commits into from
Oct 10, 2023

Conversation

AA-Turner
Copy link
Member

@AA-Turner AA-Turner commented Sep 26, 2023

@hugovk hugovk enabled auto-merge (squash) October 10, 2023 08:46
@hugovk
Copy link
Member

hugovk commented Oct 10, 2023

Thanks!

@hugovk hugovk merged commit 08ec4a1 into python:main Oct 10, 2023
20 checks passed
@hugovk hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 bug and security fixes labels Oct 10, 2023
@miss-islington
Copy link
Contributor

Thanks @AA-Turner for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @AA-Turner for the PR, and @hugovk for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @AA-Turner and @hugovk, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 08ec4a1dbf66383303de9ce5cb55b2b437ef92c0 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 10, 2023
…k to pre-commit (pythonGH-109891)

(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2023

GH-110633 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Oct 10, 2023
AlexWaygood pushed a commit that referenced this pull request Oct 10, 2023
…ck to pre-commit (GH-109891) (#110633)

GH-109408: Move the Python file whitespace check from patchcheck to pre-commit (GH-109891)
(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@hugovk
Copy link
Member

hugovk commented Oct 10, 2023

@AA-Turner Please could you take care of the backports in #109890 and here?

AA-Turner added a commit to AA-Turner/cpython that referenced this pull request Oct 10, 2023
…tchcheck to pre-commit (pythonGH-109891)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>.
(cherry picked from commit 08ec4a1)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2023

GH-110641 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 10, 2023
@hugovk
Copy link
Member

hugovk commented Oct 10, 2023

I've found a problem with this (and the C one) when running on macOS, pre-commit cannot find an executable called python:

pre-commit run --all-files
Run Ruff on Lib/test/....................................................Passed
Run Ruff on Argument Clinic..............................................Passed
check toml...............................................................Passed
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
Check Python file whitespace.............................................Failed
- hook id: python-file-whitespace
- exit code: 1

Executable `python` not found

Check C file whitespace..................................................Failed
- hook id: c-file-whitespace
- exit code: 1

Executable `python` not found

Sphinx lint..............................................................Passed
Check hooks apply to the repository......................................Passed
Check for useless excludes...............................................Passed

It works with python3, but that's not going to work with Windows.

Is there a portable solution?

One option could be like https://github.com/pre-commit/pre-commit-hooks/blob/main/.pre-commit-hooks.yaml which has entries such as:

-   id: check-added-large-files
    name: check for added large files
    description: prevents giant files from being committed.
    entry: check-added-large-files
    language: python
    stages: [commit, push, manual]

Which uses a file like https://github.com/pre-commit/pre-commit-hooks/blob/main/pre_commit_hooks/check_added_large_files.py

@hugovk
Copy link
Member

hugovk commented Oct 10, 2023

(So let's hold off on the remaining backports until this is fixed.)

@AA-Turner
Copy link
Member Author

The pre-commit website (implicitly) suggested using python for system languages, I hadn't realised it wouldn't work on Macs.

The drawback with language: python is that it seems there's no option to run outwith a venv, so we'd suffer an unnecessary slowdown.

A

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
…k to pre-commit (python#109891)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants