◐ Shell
reader mode source ↗
Skip to content

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

Merged
hugovk merged 5 commits into
python:mainfrom
AA-Turner:patchcheck/python-file-whitespace
Oct 10, 2023
Merged

GH-109408: Move the Python file whitespace check from patchcheck to pre-commit#109891
hugovk merged 5 commits into
python:mainfrom
AA-Turner:patchcheck/python-file-whitespace

Conversation

@AA-Turner

@AA-Turner AA-Turner commented Sep 26, 2023 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

@bedevere-app bedevere-app Bot mentioned this pull request Sep 26, 2023
Closed
This was referenced Sep 26, 2023
Closed
Merged
@hugovk hugovk enabled auto-merge (squash) October 10, 2023 08:46
@hugovk

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

Thanks!

@hugovk hugovk merged commit 08ec4a1 into python:main Oct 10, 2023
@hugovk hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 10, 2023
@miss-islington

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
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

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
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

miss-islington commented Oct 10, 2023

Copy link
Copy Markdown
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
@AA-Turner @hugovk @miss-islington
pythonGH-109408: Move the Python file whitespace check from patchchec… …
cbf7a0b 
…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

bedevere-app Bot commented Oct 10, 2023

Copy link
Copy Markdown

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 only security fixes label Oct 10, 2023
AlexWaygood pushed a commit that referenced this pull request Oct 10, 2023
@miss-islington @AA-Turner @hugovk
[3.12] GH-109408: Move the Python file whitespace check from patchche… …
9485765 
…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

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

@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
@AA-Turner
[3.11] pythonGH-109408: Move the Python file whitespace check from pa… …
26ccc48 
…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

bedevere-app Bot commented Oct 10, 2023

Copy link
Copy Markdown

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

hugovk commented Oct 10, 2023

Copy link
Copy Markdown
Member

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

hugovk commented Oct 10, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

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

@AA-Turner

AA-Turner commented Oct 10, 2023

Copy link
Copy Markdown
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

@AlexWaygood AlexWaygood mentioned this pull request Oct 10, 2023
Closed
This was referenced Oct 10, 2023
Closed
Merged
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@AA-Turner @hugovk @Glyphack
pythonGH-109408: Move the Python file whitespace check from patchchec… …
2d6f00d 
…k to pre-commit (python#109891)

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
@AlexWaygood AlexWaygood mentioned this pull request Oct 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk approved these changes

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees

@hugovk hugovk

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AA-Turner @hugovk @miss-islington @AlexWaygood