diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000..8ee9715 --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +# +# Rejects a commit whose staged content is not formatted the way CI expects. +# +# Installed by pointing core.hooksPath at this directory, which the CMake +# configure step does automatically (-DINSTALL_GIT_HOOKS=OFF opts out). Unlike +# .git/hooks, this directory is version controlled, so the hook arrives with a +# clone instead of relying on everyone remembering to copy it in. +# +# All the logic lives in tools/format.sh, which CI and the CMake targets also +# call. A hook with its own copy of the rules is a hook that eventually +# disagrees with CI. +# +# Bypass with `git commit --no-verify` when you genuinely need to. + +set -euo pipefail + +repo_root="$(git rev-parse --show-toplevel)" +formatter="${repo_root}/tools/format.sh" + +# Not an error: a branch predating the script, or a partial checkout. Refusing +# to commit because the checker is missing would be worse than not checking. +if [[ ! -x "${formatter}" ]]; then + exit 0 +fi + +exec "${formatter}" --staged diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 65572ae..14695df 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,16 +34,8 @@ jobs: # Pin to clang-format-18 to match the version in the dev container # (Dockerfile aliases clang-format -> clang-format-18). An unpinned # clang-format can disagree with local formatting. - - name: Check C++ formatting - run: | - sudo apt-get update && sudo apt-get install -y clang-format-18 - - if ! find src \( -name '*.cpp' -o -name '*.hpp' \) -print0 \ - | xargs -r0 clang-format-18 --dry-run --Werror; then - echo "::error::C++ formatting errors. Fix with:" - echo " find src \( -name '*.cpp' -o -name '*.hpp' \) | xargs clang-format -i" - exit 1 - fi + - name: Install clang-format + run: sudo apt-get update && sudo apt-get install -y clang-format-18 # uv resolves ruff/mypy/Python from py/host-emulator/uv.lock and # .python-version, so CI runs the exact versions the container does. @@ -53,19 +45,21 @@ jobs: enable-cache: true cache-dependency-glob: py/host-emulator/uv.lock - - name: Lint and type-check Python - working-directory: py/host-emulator + # The same script the pre-commit hook and the `format-check` CMake target + # run. Sharing one implementation is the point: a local check that has + # drifted from CI is worse than no local check, because it is trusted. + - name: Check formatting run: | - if ! uv run --frozen ruff check .; then - echo "::error::Python lint errors. Fix with: uv run ruff check --fix ." - exit 1 - fi - - if ! uv run --frozen ruff format --check .; then - echo "::error::Python formatting errors. Fix with: uv run ruff format ." + if ! tools/format.sh --check; then + echo "::error::Formatting errors. Fix with: tools/format.sh --fix" exit 1 fi + # Not part of format.sh: type checking is not a formatting concern, and is + # too slow to belong in a commit hook. + - name: Type-check Python + working-directory: py/host-emulator + run: | if ! uv run --frozen mypy src; then echo "::error::Python type errors. Reproduce with: uv run mypy src" exit 1 diff --git a/CLAUDE.md b/CLAUDE.md index bfbaf2b..a7d82c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,8 +25,19 @@ ctest --preset=host-debug -R host_emulator_test cd py/host-emulator && uv run pytest tests/test_blinky.py -v \ --blinky=../../build/host/bin/Debug/blinky -# Python lint / type-check -cd py/host-emulator && uv run ruff check . && uv run mypy src +# Formatting (C++ and Python). CI, the pre-commit hook, and these targets all +# call tools/format.sh, so they cannot disagree about what "formatted" means. +tools/format.sh --fix # Reformat in place +tools/format.sh --check # Verify, exactly as CI does +cmake --build build/host --target format # Same, via CMake +cmake --build build/host --target format-check + +# The pre-commit hook is installed by the CMake configure step, which points +# core.hooksPath at .githooks. Opt out with -DINSTALL_GIT_HOOKS=OFF; bypass a +# single commit with `git commit --no-verify`. + +# Python type-check (not covered by format.sh - types are not formatting) +cd py/host-emulator && uv run mypy src # Cross-compile for ARM - not yet functional. Presets and toolchain files exist, # but src/libs/mcu/CMakeLists.txt does add_subdirectory(${EMBEDDED_CPP_MCU}) and diff --git a/CMakeLists.txt b/CMakeLists.txt index 63e1cf2..69fc388 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -115,6 +115,11 @@ if(CMAKE_PRESET STREQUAL "arm-cm7") FetchContent_MakeAvailable(stm32cubef7) endif() +# `format` / `format-check` targets, and the pre-commit hook. Included for every +# preset: formatting is not host-specific, and a cross-compiling developer +# should get the same guard rails. +include("${PROJECT_SOURCE_DIR}/cmake/format.cmake") + clang_tidy("-header-filter=${CMAKE_CURRENT_SOURCE_DIR}/src/.*}") add_subdirectory(src) add_subdirectory(test) diff --git a/cmake/format.cmake b/cmake/format.cmake new file mode 100644 index 0000000..8de2356 --- /dev/null +++ b/cmake/format.cmake @@ -0,0 +1,62 @@ +# Formatting targets, and installation of the pre-commit hook. +# +# Both delegate to tools/format.sh rather than reimplementing the commands, so +# `cmake --build ... --target format-check`, the pre-commit hook, and CI are +# guaranteed to agree. Anything that duplicates the rules eventually disagrees +# with CI, which is the failure mode this file exists to prevent. + +set(FORMAT_SCRIPT "${CMAKE_SOURCE_DIR}/tools/format.sh") + +if(NOT EXISTS "${FORMAT_SCRIPT}") + message(WARNING "tools/format.sh not found; format targets unavailable") + return() +endif() + +add_custom_target(format + COMMAND "${FORMAT_SCRIPT}" --fix + WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" + COMMENT "Reformatting C++ and Python sources in place" + USES_TERMINAL + VERBATIM +) + +add_custom_target(format-check + COMMAND "${FORMAT_SCRIPT}" --check + WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" + COMMENT "Checking formatting (same checks as CI)" + USES_TERMINAL + VERBATIM +) + +# Install the pre-commit hook by pointing core.hooksPath at the version +# controlled .githooks directory. +# +# This writes to the developer's local git config, which is why it is announced +# rather than done silently, and why it is opt-out. It is skipped outside a git +# work tree so that tarball builds and CI checkouts are unaffected. +option(INSTALL_GIT_HOOKS "Point core.hooksPath at .githooks during configure" ON) + +if(INSTALL_GIT_HOOKS AND EXISTS "${CMAKE_SOURCE_DIR}/.git") + find_program(GIT_EXECUTABLE git) + if(GIT_EXECUTABLE) + execute_process( + COMMAND "${GIT_EXECUTABLE}" config --get core.hooksPath + WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" + OUTPUT_VARIABLE current_hooks_path + OUTPUT_STRIP_TRAILING_WHITESPACE + ERROR_QUIET + ) + if(NOT current_hooks_path STREQUAL ".githooks") + execute_process( + COMMAND "${GIT_EXECUTABLE}" config core.hooksPath .githooks + WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}" + RESULT_VARIABLE hooks_result + ERROR_QUIET + ) + if(hooks_result EQUAL 0) + message(STATUS "Set git core.hooksPath to .githooks (pre-commit format check)") + message(STATUS " Opt out with -DINSTALL_GIT_HOOKS=OFF; bypass once with git commit --no-verify") + endif() + endif() + endif() +endif() diff --git a/tools/format.sh b/tools/format.sh new file mode 100755 index 0000000..14fe7ab --- /dev/null +++ b/tools/format.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# +# Single source of truth for formatting. CI, the CMake `format`/`format-check` +# targets, and the pre-commit hook all call this script, so the three cannot +# drift apart -- which is the whole point. A local "it's formatted" that CI +# disagrees with is worse than no check at all. +# +# Usage: +# tools/format.sh --check Verify the whole tree. What CI runs. +# tools/format.sh --fix Reformat in place, and apply safe lint fixes. +# tools/format.sh --staged Verify only staged content. What the hook runs. +# +# Covers what can be fixed automatically: clang-format for C++, ruff format and +# ruff's fixable lint rules for Python. Type checking (mypy) is deliberately not +# here -- it is not a formatting concern and is too slow for a commit hook. + +set -uo pipefail + +readonly REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +readonly PY_DIR="${REPO_ROOT}/py/host-emulator" + +# Pinned to match .github/workflows/ci.yml and the dev container, which aliases +# clang-format -> clang-format-18. Versions disagree about formatting, so an +# unpinned binary produces exactly the local/CI split this script exists to +# prevent. +readonly REQUIRED_CLANG_FORMAT_MAJOR=18 + +red() { printf '\033[31m%s\033[0m\n' "$*"; } +green() { printf '\033[32m%s\033[0m\n' "$*"; } +bold() { printf '\033[1m%s\033[0m\n' "$*"; } + +find_clang_format() { + local candidate + for candidate in "clang-format-${REQUIRED_CLANG_FORMAT_MAJOR}" clang-format; do + if command -v "${candidate}" >/dev/null 2>&1; then + local version + version="$("${candidate}" --version | grep -oE '[0-9]+' | head -1)" + if [[ "${version}" != "${REQUIRED_CLANG_FORMAT_MAJOR}" ]]; then + red "warning: ${candidate} is version ${version}, expected ${REQUIRED_CLANG_FORMAT_MAJOR}." >&2 + red " Formatting may disagree with CI." >&2 + fi + echo "${candidate}" + return 0 + fi + done + red "error: clang-format not found (want clang-format-${REQUIRED_CLANG_FORMAT_MAJOR})" >&2 + return 1 +} + +cpp_sources() { + find "${REPO_ROOT}/src" \( -name '*.cpp' -o -name '*.hpp' \) -print0 +} + +check_all() { + local clang_format status=0 + clang_format="$(find_clang_format)" || return 1 + + bold "Checking C++ formatting (${clang_format})" + if ! cpp_sources | xargs -r0 "${clang_format}" --dry-run --Werror; then + red "C++ formatting errors. Fix with: tools/format.sh --fix" + status=1 + fi + + bold "Checking Python formatting and lint (ruff)" + if ! (cd "${PY_DIR}" && uv run --frozen ruff format --check .); then + red "Python formatting errors. Fix with: tools/format.sh --fix" + status=1 + fi + if ! (cd "${PY_DIR}" && uv run --frozen ruff check .); then + red "Python lint errors. Fix with: tools/format.sh --fix" + status=1 + fi + + [[ ${status} -eq 0 ]] && green "All formatting checks passed." + return ${status} +} + +fix_all() { + local clang_format + clang_format="$(find_clang_format)" || return 1 + + bold "Formatting C++ (${clang_format})" + cpp_sources | xargs -r0 "${clang_format}" -i + + bold "Formatting Python and applying safe lint fixes (ruff)" + (cd "${PY_DIR}" && uv run --frozen ruff format .) + # --fix applies only rules ruff considers safe; anything else still needs a + # human, and --check will still fail on it. + (cd "${PY_DIR}" && uv run --frozen ruff check --fix .) + + green "Done. Review the changes before committing." +} + +# Checks the *staged* content, not the working tree. A file can be staged in one +# state and edited further on disk; committing the staged version while checking +# the file on disk would pass a commit that CI then rejects. +check_staged() { + local clang_format status=0 file staged_files + staged_files="$(git diff --cached --name-only --diff-filter=ACMR)" + [[ -z "${staged_files}" ]] && return 0 + + local cpp_failed=() py_failed=() + + while IFS= read -r file; do + [[ -z "${file}" ]] && continue + case "${file}" in + src/*.cpp|src/*.hpp|src/**/*.cpp|src/**/*.hpp) + if [[ -z "${clang_format:-}" ]]; then + clang_format="$(find_clang_format)" || return 1 + fi + if ! git show ":${file}" \ + | "${clang_format}" --assume-filename="${file}" --dry-run --Werror \ + >/dev/null 2>&1; then + cpp_failed+=("${file}") + status=1 + fi + ;; + *.py) + if ! git show ":${file}" \ + | (cd "${PY_DIR}" && uv run --frozen ruff format --check \ + --stdin-filename "${file}" -) >/dev/null 2>&1; then + py_failed+=("${file}") + status=1 + elif ! git show ":${file}" \ + | (cd "${PY_DIR}" && uv run --frozen ruff check \ + --stdin-filename "${file}" -) >/dev/null 2>&1; then + py_failed+=("${file}") + status=1 + fi + ;; + esac + done <<< "${staged_files}" + + if [[ ${status} -ne 0 ]]; then + red "Formatting problems in staged files:" + for file in "${cpp_failed[@]:-}" "${py_failed[@]:-}"; do + [[ -n "${file}" ]] && printf ' %s\n' "${file}" + done + printf '\n' + printf ' Fix: tools/format.sh --fix && git add -u\n' + printf ' Bypass: git commit --no-verify\n' + fi + return ${status} +} + +case "${1:---check}" in + --check) check_all ;; + --fix) fix_all ;; + --staged) check_staged ;; + *) + red "usage: $0 [--check|--fix|--staged]" >&2 + exit 2 + ;; +esac