Skip to content

COMP: Run kw commit-msg hooks via pre-commit-managed Python#6522

Open
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-precommit-worktree-python-hooks
Open

COMP: Run kw commit-msg hooks via pre-commit-managed Python#6522
hjmjohnson wants to merge 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-precommit-worktree-python-hooks

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

The kw-commit-msg and kw-prose-budget hooks ran via Utilities/Hooks/run-pixi-python.sh, which hardcoded the CWD-relative path ./.pixi/envs/pre-commit/bin/python. That pixi environment is per-worktree and only exists where pixi install -e pre-commit has run, so the hooks fail in fresh git worktrees built with a different pixi environment. This moves the two hooks onto pre-commit's own managed-environment mechanism (language: python) — the same one already used by gersemi/clang-format/black — and removes the launcher.

Why this is the right mechanism
  • pre-commit's managed envs live in the shared ~/.cache/pre-commit (keyed to the repo, reused across all worktrees), not in per-worktree .pixi/envs/.
  • Both kw-commit-msg.py and kw-prose-budget.py import only the standard library (os, re, subprocess, sys, pathlib), so no additional_dependencies are needed.
  • The launcher (run-pixi-python.sh, added 2025-06-26) post-dated pre-commit adoption (2024-12-04); it reached for the pixi env instead of pre-commit's own env management.
Local verification

Reproduced the original failure in a fresh worktree, then with the pixi pre-commit env deleted confirmed pre-commit builds its own venv and both hooks pass:

[INFO] Installing environment for local.
kw commit-msg............................................................Passed
kw prose-budget (advisory)...............................................Passed

pre-commit run --all-files is green (gersemi, clang-format, black, all checks).

The kw-commit-msg and kw-prose-budget hooks invoked
Utilities/Hooks/run-pixi-python.sh, which hardcoded the CWD-relative
path ./.pixi/envs/pre-commit/bin/python. That pixi environment is
per-worktree and only exists where `pixi install -e pre-commit` has
run, so the hooks failed in fresh git worktrees that built with a
different pixi environment.

Use pre-commit's own managed Python environment (language: python),
matching the other repo hooks whose envs live in the shared
pre-commit cache. The scripts are stdlib-only, so no additional
dependencies are required. Remove the now-unused launcher.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class labels Jun 25, 2026
@hjmjohnson hjmjohnson marked this pull request as ready for review June 26, 2026 00:00
@hjmjohnson hjmjohnson requested a review from thewtex June 26, 2026 00:01
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR moves two commit-message hooks onto pre-commit-managed Python. The main changes are:

  • kw-commit-msg now runs with language: python.
  • kw-prose-budget now runs with language: python.
  • The pixi-specific Python launcher was removed.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
.pre-commit-config.yaml Updates the two commit-message hook entries to use pre-commit-managed Python while keeping the same script invocation shape.
Utilities/Hooks/run-pixi-python.sh Removes the pixi-specific launcher after the changed hook entries no longer reference it.

Reviews (1): Last reviewed commit: "COMP: Run kw commit-msg hooks via pre-co..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Python wrapping Python bindings for a class type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant