Skip to content

tools/clang-format.sh: cache clang-format in the common git dir#13302

Open
moonchen wants to merge 1 commit into
apache:masterfrom
moonchen:fix-clang-format-worktree-fmt-dir
Open

tools/clang-format.sh: cache clang-format in the common git dir#13302
moonchen wants to merge 1 commit into
apache:masterfrom
moonchen:fix-clang-format-worktree-fmt-dir

Conversation

@moonchen

Copy link
Copy Markdown
Contributor

Problem

In a linked git worktree, cmake --build build -t format and the pre-commit hook fail with No clang-format found. tools/clang-format.sh resolves its download/cache directory with git rev-parse --absolute-git-dir, which in a worktree is the per-worktree git dir (.git/worktrees/<name>), not the shared .git. clang-format is installed once under the common dir, so the worktree looks in the wrong, empty place.

Fix

Resolve the common git dir, which is shared by all worktrees — the same thing CMakeLists.txt already does to compute GIT_COMMON_DIR, so the script and CMake now agree on the cache location. --git-common-dir can return a relative path, so it is made absolute with cd … && pwd.

I deliberately avoided git rev-parse --path-format=absolute --git-common-dir: that exact form was removed in #11495 for a cmake 3.28 incompatibility (it needs git 2.31+). This restores the worktree support originally added in #11015 without reverting the #11495 fix.

Only clang-format.sh is affected — yapf.sh/cmake-format.sh now use uv and no longer cache under the git dir.

Testing (git 2.43.0, cmake 3.28.3)

  • Main checkout: pre-commit hook and format target behave as before.
  • Linked worktree, with the binary installed only under the common .git/fmt: the pre-commit hook now finds clang-format and commits succeed (previously No clang-format found) — verified end-to-end with the clangFormat.binary fallback removed.

Copilot AI review requested due to automatic review settings June 19, 2026 18:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes formatter installation/cache lookup under git worktree setups by updating tools/clang-format.sh to cache/download clang-format under the common Git directory (shared across linked worktrees), aligning the script’s behavior with how CMakeLists.txt computes GIT_COMMON_DIR.

Changes:

  • Switch clang-format cache root from git rev-parse --absolute-git-dir to an absolute git rev-parse --git-common-dir.
  • Update the sourced-script path logic (used by the pre-commit hook) to use the common Git dir cache location as well.

Comment thread tools/clang-format.sh Outdated
clang-format.sh located its download cache with `git rev-parse
--absolute-git-dir`, which in a linked worktree is the per-worktree git
dir (.git/worktrees/<name>) rather than the shared .git.  clang-format
is installed once under the common dir, so in a worktree the pre-commit
hook and the format target could not find it and failed with "No
clang-format found".

Resolve the common git dir instead, matching how CMakeLists.txt already
computes GIT_COMMON_DIR.  It is made absolute with `cd ... && pwd`
rather than `git rev-parse --path-format=absolute`, avoiding a
dependency on git 2.31+; that --path-format form was removed in apache#11495
over a cmake 3.28 incompatibility.

The no-argument default target now uses `git rev-parse --show-toplevel`
so a bare run inside a worktree formats that worktree, not the main
checkout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@moonchen moonchen force-pushed the fix-clang-format-worktree-fmt-dir branch from e6f633a to 65def48 Compare June 19, 2026 18:56
@moonchen

Copy link
Copy Markdown
Contributor Author

Testing

Tested the format workflows from a fresh clone, comparing master with this PR.

Does clang-format get found and run? — verified two ways: cache-dir resolution, and a real pre-commit commit.

Workflow master PR #13302
No worktree ✅ works ✅ works
Linked worktree No clang-format found ✅ works

clang-format is installed once under the common .git/fmt; on master a linked worktree looks in .git/worktrees/<name>/fmt instead and misses it.

Everything checked:

Check
git 2.43.0 (current)
git 2.30.2 (pre---path-format)
cmake 3.28.3 (the version named in #11495)
cmake format targets (clang-format-install, clang-format-src) run in a worktree
--install downloads to the common .git/fmt
worktree finds the --install download
no-path run formats the current worktree, not the main checkout (Copilot's note)

Addressing #11495: that PR removed git rev-parse --path-format=absolute --git-common-dir because it "didn't work in cmake 3.28." The real cause is the git version, not cmake — --path-format only exists in git 2.31+, and on older git (2.30.2) that command returns the literal string --path-format=absolute. This PR gets the same result (the absolute path to the shared git dir) without that flag, using cd "$(git rev-parse --git-common-dir)" && pwd — so it restores worktree support and works on git back to 2.5.

@moonchen moonchen self-assigned this Jun 19, 2026
@moonchen moonchen added this to the 11.0.0 milestone Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants