diff --git a/AGENTS.md b/AGENTS.md index adc629a9b..d444d1f47 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,8 +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 - 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/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..86f3a493c 100755 --- a/scripts/git_hooks/pre-commit/0_format +++ b/scripts/git_hooks/pre-commit/0_format @@ -1,25 +1,35 @@ #!/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 "Formats staged C++ and Objective-C++ files without staging other changes." + 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) ;; + *) 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 new file mode 100644 index 000000000..929e18612 --- /dev/null +++ b/scripts/tests/test_format.py @@ -0,0 +1,103 @@ +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 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, 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, staged=staged): + path = self.repo / name + path.write_text(staged) + self.run_command("git", "add", "--", name) + path.write_text(unstaged) + result = self.run_hook() + 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_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") + 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, 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}, + ) + + +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)