From 2de59902fdfb06003f82d1488855c267894c1940 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 07:07:25 +0200 Subject: [PATCH 1/2] fix: keep formatting hooks from staging unrelated changes --- AGENTS.md | 3 +- scripts/format | 34 ++++-------- scripts/git_hooks/pre-commit/0_format | 42 +++++++-------- scripts/tests/test_format.py | 78 +++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 46 deletions(-) create mode 100644 scripts/tests/test_format.py diff --git a/AGENTS.md b/AGENTS.md index adc629a9b..6d39f62ef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,7 +135,8 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file to HTML ## Conventions - Formatting: clang-format per `.clang-format`, clang-tidy per `.clang-tidy`. - Run `scripts/format`, or install the hook with `scripts/setup`. CI enforces + Run `scripts/format`; the hook installed by `scripts/setup` checks staged + content without modifying files or staging additional changes. CI enforces both. - C++23 with three ceilings. Check a new facility against all three by building an object file, because `-fsyntax-only` misses codegen bugs. diff --git a/scripts/format b/scripts/format index 97692941d..e024c5a28 100755 --- a/scripts/format +++ b/scripts/format @@ -1,26 +1,14 @@ #!/usr/bin/env bash +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")/.." CLANG_FORMAT=${CLANG_FORMAT:-clang-format} - -echo "Running clang-format on all .hpp/.cpp and .h/.mm files in the project" -echo "Using clang-format: ${CLANG_FORMAT}" - -${CLANG_FORMAT} --version - -function format_file() { - echo "Formatting $1" - ${CLANG_FORMAT} --style=file -i $1 -} -function format_folder() { - export -f format_file - export CLANG_FORMAT - find $1 -type f \( -iname \*.hpp -o -iname \*.cpp -o -iname \*.h -o -iname \*.mm \) -exec bash -c 'format_file "${1}"' -- {} \; -} - -format_folder src -format_folder cli -format_folder test/src -format_folder python/src -format_folder jni/src -format_folder apple/src -format_folder apple/include +"$CLANG_FORMAT" --version + +find src cli test/src python/src jni/src apple/src apple/include wasm/src \ + -type f \( -iname '*.hpp' -o -iname '*.cpp' -o -iname '*.cc' \ + -o -iname '*.h' -o -iname '*.mm' \) -print0 | + while IFS= read -r -d '' file; do + echo "Formatting $file" + "$CLANG_FORMAT" --style=file -i "$file" + done diff --git a/scripts/git_hooks/pre-commit/0_format b/scripts/git_hooks/pre-commit/0_format index 5b10cf0a6..1b2ac798e 100755 --- a/scripts/git_hooks/pre-commit/0_format +++ b/scripts/git_hooks/pre-commit/0_format @@ -1,25 +1,23 @@ #!/usr/bin/env bash -{ -format_file() { - local file="${1}" - if [ -f $file ]; then - clang-format -i -style=file ${1} - fi -} +set -euo pipefail -case "${1}" in - --about) - echo "Runs formatters on any files that are going to be checked in." - ;; - *) - # make newlines the only separator - IFS=$'\n' +if [[ ${1:-} == --about ]]; then + echo "Checks formatting of staged C++ and Objective-C++ files." + exit 0 +fi - echo "running scripts/scripts/format.sh on all staged files" - for file in `git diff-index --cached --name-only --diff-filter=ACMRT HEAD | grep -iE '\.(cpp|cc|h|hpp|mm)$'` ; do - format_file "${file}" - git add "${file}" - done - ;; -esac -} +cd "$(git rev-parse --show-toplevel)" +CLANG_FORMAT=${CLANG_FORMAT:-clang-format} +shopt -s nocasematch + +git diff --cached --name-only --diff-filter=ACMRT -z | + while IFS= read -r -d '' file; do + case "$file" in + *.cpp|*.cc|*.h|*.hpp|*.mm) + mode=$(git --literal-pathspecs ls-files --stage -- "$file") + [[ $mode == 120000\ * ]] && continue + git show ":$file" | + "$CLANG_FORMAT" --style=file --assume-filename="$file" --dry-run --Werror + ;; + esac + done diff --git a/scripts/tests/test_format.py b/scripts/tests/test_format.py new file mode 100644 index 000000000..96ecf2083 --- /dev/null +++ b/scripts/tests/test_format.py @@ -0,0 +1,78 @@ +import os +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +FORMATTER = shutil.which(os.environ.get("CLANG_FORMAT", "clang-format")) + + +@unittest.skipUnless(FORMATTER, "clang-format is required") +class FormatHookTest(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory(prefix="odr format ") + self.addCleanup(self.temp.cleanup) + self.repo = Path(self.temp.name) + self.run_command("git", "init", "-q") + (self.repo / ".clang-format").write_text("BasedOnStyle: LLVM\n") + + def run_command(self, *args, **kwargs): + return subprocess.run(args, cwd=self.repo, capture_output=True, + check=True, **kwargs) + + def test_partial_staging_and_unusual_paths_remain_unchanged(self): + self.run_command("git", "add", ".clang-format") + self.run_command("git", "-c", "user.name=Review", "-c", + "user.email=review@example.invalid", "-c", + "core.hooksPath=/dev/null", "commit", "-qm", "initial") + for name in ("file.cpp", "-space and\nnewline.cpp"): + for staged, unstaged, succeeds in ( + ("int x = 1;\n", "int x=2;\n", True), + ("int x=1;\n", "int x = 2;\n", False), + ): + with self.subTest(name=name, succeeds=succeeds): + path = self.repo / name + path.write_text(staged) + self.run_command("git", "add", "--", name) + path.write_text(unstaged) + index = (self.repo / ".git/index").read_bytes() + result = self.run_hook() + self.assertEqual(result.returncode == 0, succeeds, result.stderr) + self.assertEqual((self.repo / ".git/index").read_bytes(), index) + self.assertEqual(path.read_text(), unstaged) + self.run_command("git", "rm", "--cached", "-f", "--", name) + + def test_checks_initial_commit(self): + (self.repo / "file.cpp").write_text("int x=1;\n") + self.run_command("git", "add", "file.cpp") + self.assertNotEqual(self.run_hook().returncode, 0) + + def run_hook(self): + return subprocess.run( + [ROOT / "scripts/git_hooks/pre-commit/0_format"], + cwd=self.repo, capture_output=True, + env={**os.environ, "CLANG_FORMAT": FORMATTER}, + ) + + +class FormatScriptTest(unittest.TestCase): + def test_formatter_failures_stop_the_script(self): + with tempfile.TemporaryDirectory(prefix="odr format ") as directory: + root = Path(directory) + (root / "scripts").mkdir() + shutil.copy2(ROOT / "scripts/format", root / "scripts/format") + for name in ("src", "cli", "test/src", "python/src", "jni/src", + "apple/src", "apple/include", "wasm/src"): + (root / name).mkdir(parents=True) + (root / "wasm/src/file with spaces.cpp").write_text("int x;\n") + formatter = root / "fake formatter" + formatter.write_text('#!/bin/sh\n[ "$1" = --version ] && exit 0\nexit 7\n') + formatter.chmod(0o755) + result = subprocess.run( + [root / "scripts/format"], cwd=directory, capture_output=True, + env={**os.environ, "CLANG_FORMAT": str(formatter)}, + ) + self.assertEqual(result.returncode, 7, result.stderr) + self.assertIn(b"wasm/src/file with spaces.cpp", result.stdout) From 71a68f5185d1cedd23129023b888e77662e4d371 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 19:02:36 +0200 Subject: [PATCH 2/2] fix: format the staged content instead of refusing the commit The hook now formats the staged blob of each C++ file and writes the result back to the index, so a commit with unformatted code still goes through, as before. It formats the working file too only when that file has no unstaged edits, so unstaged changes are never staged. The hook reads the blob with git cat-file, and a failing formatter stops the commit with the index unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc --- AGENTS.md | 5 ++- scripts/git_hooks/pre-commit/0_format | 26 ++++++++++---- scripts/tests/test_format.py | 49 ++++++++++++++++++++------- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6d39f62ef..d444d1f47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,9 +135,8 @@ cmake --build cmake-build-relwithdebinfo --target translate # CLI: file to HTML ## Conventions - Formatting: clang-format per `.clang-format`, clang-tidy per `.clang-tidy`. - Run `scripts/format`; the hook installed by `scripts/setup` checks staged - content without modifying files or staging additional changes. CI enforces - both. + Run `scripts/format`; the hook installed by `scripts/setup` formats only the + staged content and never stages other changes. CI enforces both. - C++23 with three ceilings. Check a new facility against all three by building an object file, because `-fsyntax-only` misses codegen bugs. - The standard library is capped by emsdk 3.1.73 (libc++ 18.1). Available: diff --git a/scripts/git_hooks/pre-commit/0_format b/scripts/git_hooks/pre-commit/0_format index 1b2ac798e..86f3a493c 100755 --- a/scripts/git_hooks/pre-commit/0_format +++ b/scripts/git_hooks/pre-commit/0_format @@ -2,7 +2,7 @@ set -euo pipefail if [[ ${1:-} == --about ]]; then - echo "Checks formatting of staged C++ and Objective-C++ files." + echo "Formats staged C++ and Objective-C++ files without staging other changes." exit 0 fi @@ -13,11 +13,23 @@ shopt -s nocasematch git diff --cached --name-only --diff-filter=ACMRT -z | while IFS= read -r -d '' file; do case "$file" in - *.cpp|*.cc|*.h|*.hpp|*.mm) - mode=$(git --literal-pathspecs ls-files --stage -- "$file") - [[ $mode == 120000\ * ]] && continue - git show ":$file" | - "$CLANG_FORMAT" --style=file --assume-filename="$file" --dry-run --Werror - ;; + *.cpp|*.cc|*.h|*.hpp|*.mm) ;; + *) continue ;; esac + read -r mode staged _ < <(git --literal-pathspecs ls-files --stage -- "$file") + [[ $mode == 120000 ]] && continue + formatted=$(git cat-file blob "$staged" | + "$CLANG_FORMAT" --style=file --assume-filename="$file" | + git hash-object -w --stdin) + [[ $formatted == "$staged" ]] && continue + # unstaged edits stay as they are; a file without any follows the index + if git --literal-pathspecs diff --quiet -- "$file"; then + follow=true + else + follow=false + fi + git update-index --cacheinfo "$mode,$formatted,$file" + if $follow; then + git checkout-index --force -- "$file" + fi done diff --git a/scripts/tests/test_format.py b/scripts/tests/test_format.py index 96ecf2083..929e18612 100644 --- a/scripts/tests/test_format.py +++ b/scripts/tests/test_format.py @@ -22,38 +22,63 @@ def run_command(self, *args, **kwargs): return subprocess.run(args, cwd=self.repo, capture_output=True, check=True, **kwargs) - def test_partial_staging_and_unusual_paths_remain_unchanged(self): + def commit_initial(self): self.run_command("git", "add", ".clang-format") self.run_command("git", "-c", "user.name=Review", "-c", "user.email=review@example.invalid", "-c", "core.hooksPath=/dev/null", "commit", "-qm", "initial") + + def staged(self, name): + return self.run_command("git", "show", f":{name}").stdout.decode() + + def test_formats_only_the_staged_content(self): + self.commit_initial() for name in ("file.cpp", "-space and\nnewline.cpp"): - for staged, unstaged, succeeds in ( - ("int x = 1;\n", "int x=2;\n", True), - ("int x=1;\n", "int x = 2;\n", False), + for staged, unstaged, formatted in ( + ("int x=1;\n", "int x=2;\n", "int x = 1;\n"), + ("int x = 1;\n", "int x=2;\n", "int x = 1;\n"), ): - with self.subTest(name=name, succeeds=succeeds): + with self.subTest(name=name, staged=staged): path = self.repo / name path.write_text(staged) self.run_command("git", "add", "--", name) path.write_text(unstaged) - index = (self.repo / ".git/index").read_bytes() result = self.run_hook() - self.assertEqual(result.returncode == 0, succeeds, result.stderr) - self.assertEqual((self.repo / ".git/index").read_bytes(), index) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.staged(name), formatted) self.assertEqual(path.read_text(), unstaged) self.run_command("git", "rm", "--cached", "-f", "--", name) - def test_checks_initial_commit(self): + def test_formats_a_file_without_unstaged_changes_in_place(self): + self.commit_initial() + path = self.repo / "file.cpp" + path.write_text("int x=1;\n") + self.run_command("git", "add", "file.cpp") + self.assertEqual(self.run_hook().returncode, 0) + self.assertEqual(self.staged("file.cpp"), "int x = 1;\n") + self.assertEqual(path.read_text(), "int x = 1;\n") + + def test_formats_the_initial_commit(self): + (self.repo / "file.cpp").write_text("int x=1;\n") + self.run_command("git", "add", "file.cpp") + self.assertEqual(self.run_hook().returncode, 0) + self.assertEqual(self.staged("file.cpp"), "int x = 1;\n") + + def test_a_formatter_failure_stops_the_commit(self): (self.repo / "file.cpp").write_text("int x=1;\n") self.run_command("git", "add", "file.cpp") - self.assertNotEqual(self.run_hook().returncode, 0) + index = (self.repo / ".git/index").read_bytes() + formatter = self.repo / "failing formatter" + formatter.write_text("#!/bin/sh\nexit 7\n") + formatter.chmod(0o755) + self.assertNotEqual(self.run_hook(str(formatter)).returncode, 0) + self.assertEqual((self.repo / ".git/index").read_bytes(), index) - def run_hook(self): + def run_hook(self, formatter=FORMATTER): return subprocess.run( [ROOT / "scripts/git_hooks/pre-commit/0_format"], cwd=self.repo, capture_output=True, - env={**os.environ, "CLANG_FORMAT": FORMATTER}, + env={**os.environ, "CLANG_FORMAT": formatter}, )