From 3d8aad16cb487424619086f28ce93dbf77e36a9a Mon Sep 17 00:00:00 2001 From: Growl Date: Tue, 12 May 2026 17:31:42 +0800 Subject: [PATCH 1/4] Add an alternative Precommit workflow --- .github/workflows/precommit.yml | 53 ++++ .github/workflows/test.yml | 105 +++++--- tools/precommit/Dockerfile | 16 ++ tools/precommit/README.md | 104 ++++++++ tools/precommit/abacus_clang_format.yaml | 49 ++++ tools/precommit/check_file_properties.py | 302 ++++++++++++++++++++++ tools/precommit/clang_format_wrapper.sh | 28 +++ tools/precommit/cloudbuild.yaml | 31 +++ tools/precommit/deploy.sh | 23 ++ tools/precommit/format_makefile.py | 43 ++++ tools/precommit/install_requirements.sh | 32 +++ tools/precommit/precommit.py | 308 +++++++++++++++++++++++ tools/precommit/precommit_server.py | 98 ++++++++ tools/precommit/requirements.txt | 30 +++ tools/precommit/start_local_server.sh | 11 + 15 files changed, 1193 insertions(+), 40 deletions(-) create mode 100644 .github/workflows/precommit.yml create mode 100644 tools/precommit/Dockerfile create mode 100644 tools/precommit/README.md create mode 100644 tools/precommit/abacus_clang_format.yaml create mode 100755 tools/precommit/check_file_properties.py create mode 100755 tools/precommit/clang_format_wrapper.sh create mode 100644 tools/precommit/cloudbuild.yaml create mode 100755 tools/precommit/deploy.sh create mode 100755 tools/precommit/format_makefile.py create mode 100755 tools/precommit/install_requirements.sh create mode 100755 tools/precommit/precommit.py create mode 100755 tools/precommit/precommit_server.py create mode 100644 tools/precommit/requirements.txt create mode 100755 tools/precommit/start_local_server.sh diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml new file mode 100644 index 00000000000..ce597fd0186 --- /dev/null +++ b/.github/workflows/precommit.yml @@ -0,0 +1,53 @@ +name: Precommit + +on: + workflow_dispatch: + pull_request: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Precommit + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v6 + with: + fetch-depth: 0 + # We will handle submodules manually after fixing ownership + submodules: 'false' + + - name: Update submodules + run: | + git config --global --add safe.directory "$GITHUB_WORKSPACE" + git submodule update --init --recursive + + - name: Configure precommit server + run: | + cd tools/precommit + ./start_local_server.sh & + + - name: Wait for precommit server + run: | + for i in {1..300}; do + if curl -fs -o /dev/null http://127.0.0.1:8080/ 2>/dev/null; then + echo "precommit server is ready" + exit 0 + fi + sleep 1 + done + + echo "precommit server did not become ready" >&2 + echo "==== precommit server log ====" >&2 + cat "$RUNNER_TEMP/precommit-server.log" >&2 || true + exit 1 + + - name: Run precommit + env: + ABACUS_PRECOMMIT_SERVER: "http://127.0.0.1:8080" + run: | + ./tools/precommit/precommit.py --no-cache + diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a21d54b2a82..baedd4dfc3b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,214 +32,239 @@ jobs: - name: Install CI tools run: | sudo apt-get update - sudo apt-get install -y ccache ca-certificates python-is-python3 python3-pip - sudo pip install clang-format clang-tidy + sudo apt-get install -y ccache ca-certificates python-is-python3 - name: Configure run: | cmake -B build -DBUILD_TESTING=ON -DENABLE_MLALGO=ON -DENABLE_LIBXC=ON -DENABLE_LIBRI=ON -DENABLE_GOOGLEBENCH=ON -DENABLE_RAPIDJSON=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DENABLE_FLOAT_FFTW=ON -# Temporarily removed because no one maintains this now. -# And it will break the CI test workflow. - -# - uses: pre-commit/action@v3.0.1 -# with: -# extra_args: -# --from-ref ${{ github.event.pull_request.base.sha }} -# --to-ref ${{ github.event.pull_request.head.sha }} -# continue-on-error: true -# - uses: pre-commit-ci/lite-action@v1.0.3 - - name: Build run: | cmake --build build -j8 cmake --install build + - name: Prepare ctest wrapper + run: | + cat > "$RUNNER_TEMP/run_ctest.sh" <<'EOF' + #!/usr/bin/env bash + set +e + + name="$1" + shift + + echo "::group::$name" + ctest --test-dir build -V --timeout 1700 "$@" + status=$? + echo "::endgroup::" + + if [ "$status" -ne 0 ]; then + echo "$name" >> "$RUNNER_TEMP/failed-tests.txt" + echo "::error title=$name failed::ctest exited with status $status" + fi + + exit 0 + EOF + + chmod +x "$RUNNER_TEMP/run_ctest.sh" + - name: Integrated Tests Preparation env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R integrated_test + "$RUNNER_TEMP/run_ctest.sh" "Integrated Tests Preparation" -R integrated_test - name: Module_Base Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_BASE + "$RUNNER_TEMP/run_ctest.sh" "Module_Base Unittests" -R MODULE_BASE - name: Module_IO Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_IO + "$RUNNER_TEMP/run_ctest.sh" "Module_IO Unittests" -R MODULE_IO - name: Module_HSolver Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_HSOLVER -E PERF_MODULE_HSOLVER_KERNELS + "$RUNNER_TEMP/run_ctest.sh" "Module_HSolver Unittests" -R MODULE_HSOLVER -E PERF_MODULE_HSOLVER_KERNELS - name: Module_Cell Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_CELL + "$RUNNER_TEMP/run_ctest.sh" "Module_Cell Unittests" -R MODULE_CELL - name: Module_MD Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_MD + "$RUNNER_TEMP/run_ctest.sh" "Module_MD Unittests" -R MODULE_MD - name: Module_Psi Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_PSI + "$RUNNER_TEMP/run_ctest.sh" "Module_Psi Unittests" -R MODULE_PSI - name: Module_RI Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_RI + "$RUNNER_TEMP/run_ctest.sh" "Module_RI Unittests" -R MODULE_RI - name: Module_Estate Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_ESTATE + "$RUNNER_TEMP/run_ctest.sh" "Module_Estate Unittests" -R MODULE_ESTATE - name: Module_Hamilt Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_HAMILT + "$RUNNER_TEMP/run_ctest.sh" "Module_Hamilt Unittests" -R MODULE_HAMILT - name: Module_PW Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_PW + "$RUNNER_TEMP/run_ctest.sh" "Module_PW Unittests" -R MODULE_PW - name: Module_LCAO Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_LCAO + "$RUNNER_TEMP/run_ctest.sh" "Module_LCAO Unittests" -R MODULE_LCAO - name: Module_AO Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_AO + "$RUNNER_TEMP/run_ctest.sh" "Module_AO Unittests" -R MODULE_AO - name: Module_NAO Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_NAO + "$RUNNER_TEMP/run_ctest.sh" "Module_NAO Unittests" -R MODULE_NAO - name: Module_RELAX Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_RELAX + "$RUNNER_TEMP/run_ctest.sh" "Module_RELAX Unittests" -R MODULE_RELAX - name: Module_LR Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R MODULE_LR + "$RUNNER_TEMP/run_ctest.sh" "Module_LR Unittests" -R MODULE_LR - name: 01_PW Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 01_PW + "$RUNNER_TEMP/run_ctest.sh" "01_PW Test" -R 01_PW - name: 02_NAO_Gamma Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 02_NAO_Gamma + "$RUNNER_TEMP/run_ctest.sh" "02_NAO_Gamma Test" -R 02_NAO_Gamma - name: 03_NAO_multik Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 03_NAO_multik + "$RUNNER_TEMP/run_ctest.sh" "03_NAO_multik Test" -R 03_NAO_multik - name: 04_FF Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 04_FF + "$RUNNER_TEMP/run_ctest.sh" "04_FF Test" -R 04_FF - name: 05_rtTDDFT Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 05_rtTDDFT + "$RUNNER_TEMP/run_ctest.sh" "05_rtTDDFT Test" -R 05_rtTDDFT - name: 06_SDFT Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 06_SDFT + "$RUNNER_TEMP/run_ctest.sh" "06_SDFT Test" -R 06_SDFT - name: 07_OFDFT Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 07_OFDFT + "$RUNNER_TEMP/run_ctest.sh" "07_OFDFT Test" -R 07_OFDFT - name: 08_EXX Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 08_EXX + "$RUNNER_TEMP/run_ctest.sh" "08_EXX Test" -R 08_EXX - name: 09_DeePKS Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 09_DeePKS + "$RUNNER_TEMP/run_ctest.sh" "09_DeePKS Test" -R 09_DeePKS - name: 10_others Test env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -R 10_others + "$RUNNER_TEMP/run_ctest.sh" "10_others Test" -R 10_others - name: Other Unittests env: GTEST_COLOR: 'yes' OMP_NUM_THREADS: '2' run: | - ctest --test-dir build -V --timeout 1700 -E 'integrate_test|01_PW|02_NAO_Gamma|03_NAO_multik|04_FF|05_rtTDDFT|06_SDFT|07_OFDFT|08_EXX|09_DeePKS|10_others|11_PW_GPU|12_NAO_Gamma_GPU|13_NAO_multik_GPU|15_rtTDDFT_GPU|16_SDFT_GPU|MODULE_BASE|MODULE_IO|MODULE_HSOLVER|MODULE_CELL|MODULE_MD|MODULE_PSI|MODULE_ESTATE|MODULE_RI|MODULE_HAMILT|MODULE_PW|MODULE_LCAO|MODULE_AO|MODULE_NAO|MODULE_RELAX|MODULE_LR' + "$RUNNER_TEMP/run_ctest.sh" "Other Unittests" -E 'integrate_test|01_PW|02_NAO_Gamma|03_NAO_multik|04_FF|05_rtTDDFT|06_SDFT|07_OFDFT|08_EXX|09_DeePKS|10_others|11_PW_GPU|12_NAO_Gamma_GPU|13_NAO_multik_GPU|15_rtTDDFT_GPU|16_SDFT_GPU|MODULE_BASE|MODULE_IO|MODULE_HSOLVER|MODULE_CELL|MODULE_MD|MODULE_PSI|MODULE_ESTATE|MODULE_RI|MODULE_HAMILT|MODULE_PW|MODULE_LCAO|MODULE_AO|MODULE_NAO|MODULE_RELAX|MODULE_LR' + + - name: Fail if any test group failed + if: always() + run: | + if [ -s "$RUNNER_TEMP/failed-tests.txt" ]; then + echo "" + echo "The following test groups failed:" + cat "$RUNNER_TEMP/failed-tests.txt" + echo "" + exit 1 + fi + + echo "All test groups passed." diff --git a/tools/precommit/Dockerfile b/tools/precommit/Dockerfile new file mode 100644 index 00000000000..2a8aab17861 --- /dev/null +++ b/tools/precommit/Dockerfile @@ -0,0 +1,16 @@ +FROM ubuntu:24.04 + +# Based on CP2K's tools/precommit/Dockerfile, adapted for ABACUS. + +WORKDIR /opt/abacus-precommit +COPY install_requirements.sh requirements.txt ./ +RUN ./install_requirements.sh +ENV PATH="/opt/venv/bin:/opt/abacus-precommit:$PATH" + +ARG REVISION +ENV REVISION=${REVISION} + +COPY . ./ +CMD ["gunicorn", "--bind=:8080", "--workers=1", "--threads=8", "--timeout=0", "precommit_server:app"] + +# EOF diff --git a/tools/precommit/README.md b/tools/precommit/README.md new file mode 100644 index 00000000000..d5a7db67bd2 --- /dev/null +++ b/tools/precommit/README.md @@ -0,0 +1,104 @@ +# ABACUS Precommit + +This is a CP2K-style precommit system adapted for ABACUS. The local driver is +`tools/precommit/precommit.py`; the formatting tools are provided by a small Flask/gunicorn +precommit server. + +The design intentionally follows CP2K's usage model: + +- one local driver script; +- `obj/precommit/cache.json` mtime cache; +- file-level processing; +- local backup before each tool modifies a file; +- default check-only mode; +- `--allow-modifications` to keep formatter changes; +- remote or local Docker server for external tools. + +## Install Git hook + +From the ABACUS repository root: + +```bash +ln -fs ../../tools/precommit/precommit.py .git/hooks/pre-commit +``` + +## Run checks + +```bash +./tools/precommit/precommit.py +``` + +The default mode does not modify source files. If a formatter would change a file, the driver +restores the original content and prints a unified diff. + +## Apply modifications + +```bash +./tools/precommit/precommit.py --allow-modifications +``` + +Short option: + +```bash +./tools/precommit/precommit.py -m +``` + +## Ignore cache + +```bash +./tools/precommit/precommit.py --no-cache +``` + +Short option: + +```bash +./tools/precommit/precommit.py -a +``` + +## Process selected files + +```bash +./tools/precommit/precommit.py source/module/foo.cpp source/module/foo.h +``` + +## Local server + +Until an ABACUS precommit server is deployed, run the server locally with Docker: + +```bash +cd tools/precommit +./start_local_server.sh +``` + +Then, in another terminal from the repository root: + +```bash +export ABACUS_PRECOMMIT_SERVER="http://127.0.0.1:8080" +./tools/precommit/precommit.py +``` + +## Tools + +The first ABACUS version keeps CP2K's lightweight file-level model and maps it to ABACUS' C++-first +source tree: + +- C/C++/CUDA/HIP/OpenCL: `clang-format` +- Python: `ast.parse` + `black` +- Shell: `shfmt` + `shellcheck` +- Markdown: `mdformat --wrap=100` +- CMake: `cmake-format -i` +- Makefile: local `format_makefile.py` +- all files: local `check_file_properties.py` + +`clang-tidy` and `compile_commands.json` are intentionally not part of this CP2K-style first +version. They are build-level static-analysis concerns, while this precommit driver is a lightweight +file-level formatting and convention gate. + +## Notes for ABACUS maintainers + +The only non-CP2K structural change is that C++ is first-class. CP2K rejects most C++ files; this +ABACUS version formats `.c`, `.cc`, `.cpp`, `.cxx`, `.h`, `.hh`, `.hpp`, `.hxx`, `.cu`, `.cuh`, +`.hip`, and `.cl` files. + +`check_file_properties.py` contains the ABACUS-specific convention checks. The banner/license policy +should be tightened once the exact ABACUS source header is agreed upon. diff --git a/tools/precommit/abacus_clang_format.yaml b/tools/precommit/abacus_clang_format.yaml new file mode 100644 index 00000000000..efc19b0b9fd --- /dev/null +++ b/tools/precommit/abacus_clang_format.yaml @@ -0,0 +1,49 @@ +--- +Language: Cpp +BasedOnStyle: LLVM + +AlwaysBreakTemplateDeclarations: Yes + +ColumnLimit: 120 +AllowAllArgumentsOnNextLine: false +AllowAllParametersOfDeclarationOnNextLine: false +BinPackArguments: false +BinPackParameters: false + +BreakBeforeTernaryOperators: true + +Cpp11BracedListStyle: true +FixNamespaceComments: true + +IncludeBlocks: Preserve +IncludeIsMainRegex: '([-_](test|unittest))?$' +IncludeIsMainSourceRegex: '' + +IndentWidth: 4 + +DerivePointerAlignment: false +PointerAlignment: Left + +ReflowComments: true +SortIncludes: false +SortUsingDeclarations: true + +# ABACUS follows the usual C/C++ convention: control statements use a space +# before parentheses (`if (...)`), but function calls/declarations do not +# (`foo(...)`). GNU style defaults to `Always`, which would rewrite calls as +# `foo (...)` and produce noisy, unidiomatic diffs. +SpaceBeforeParens: ControlStatements + +SpaceAfterCStyleCast: false +SpaceAfterLogicalNot: false +SpaceBeforeRangeBasedForLoopColon: false +SpaceInEmptyParentheses: false +SpacesInAngles: false +SpacesInCStyleCastParentheses: false +SpacesInContainerLiterals: false +SpacesInParentheses: false +SpacesInSquareBrackets: false + +Standard: c++17 +UseTab: Never +... diff --git a/tools/precommit/check_file_properties.py b/tools/precommit/check_file_properties.py new file mode 100755 index 00000000000..cb3f1e557fe --- /dev/null +++ b/tools/precommit/check_file_properties.py @@ -0,0 +1,302 @@ +#!/usr/bin/env python3 + +# Based on CP2K's tools/precommit/check_file_properties.py, adapted for ABACUS. + +import argparse +from functools import lru_cache +import os +import pathlib +import re +import sys +from typing import Iterable, List, Set, Tuple, TypeVar + +T = TypeVar("T") + +# We assume this script is in tools/precommit/. +ABACUS_DIR = pathlib.Path(__file__).resolve().parents[2] + +PORTABLE_FILENAME_RE = re.compile(r"^[a-zA-Z0-9._/#~=+-]*$") +OP_RE = re.compile(r"[\\|()!&><=*/+-]") +NUM_RE = re.compile(r"[0-9]+[ulUL]*") +CMAKE_OPTION_RE = re.compile(r"option\(\s*(\w+)", re.DOTALL) +MERGE_CONFLICT_RE = re.compile( + r"^(<<<<<<<(?: .*)?|\|\|\|\|\|\|\|(?: .*)?|=======$|>>>>>>>(?: .*)?)$", + re.MULTILINE, +) + +C_EXTENSIONS = ( + ".c", + ".cc", + ".cpp", + ".cxx", + ".h", + ".hh", + ".hpp", + ".hxx", + ".cu", + ".cuh", + ".hip", + ".cl", +) +HEADER_EXTENSIONS = (".h", ".hh", ".hpp", ".hxx", ".cuh") +TEXT_EXTENSIONS = C_EXTENSIONS + ( + ".cmake", + ".md", + ".py", + ".sh", + ".txt", + ".yml", + ".yaml", + ".json", +) +WINDOWS_SCRIPT_EXTENSIONS = (".bat", ".cmd") + +# Keep this list intentionally broad. Project-specific additions should be made +# here rather than suppressing warnings at call sites. +FLAG_EXCEPTIONS = ( + r"\$\{.+\}\$", + r"__.+__", + r"_M_.+", + r"_WIN32", + r"_OPENMP", + r"__cplusplus", + r"__GNUC__", + r"__GNUC_MINOR__", + r"__GNUC_PATCHLEVEL__", + r"__clang__", + r"__clang_major__", + r"__INTEL_COMPILER", + r"__INTEL_LLVM_COMPILER", + r"__NVCC__", + r"__CUDACC__", + r"__HIPCC__", + r"CUDA_VERSION", + r"HIP_VERSION", + r"NDEBUG", + r"M_PI", + r"EIGEN_.+", + r"ELPA_.+", + r"FFTW_.+", + r"MPI_.+", + r"MKL_.+", + r"LAPACK_.+", + r"BLAS_.+", + r"OPENBLAS_.+", + r"LIBINT.+", + r"LIBXC.+", + r"CEREAL_.+", + r"GTEST_.+", +) +FLAG_EXCEPTIONS_RE = re.compile(r"|".join(FLAG_EXCEPTIONS)) + + +@lru_cache(maxsize=None) +def get_cmake_sources() -> str: + files: List[pathlib.Path] = [] + root_cmake = ABACUS_DIR / "CMakeLists.txt" + if root_cmake.exists(): + files.append(root_cmake) + for directory in ("cmake", "toolchain", "tools"): + base = ABACUS_DIR / directory + if base.exists(): + files.extend(base.glob("**/*.cmake")) + files.extend(base.glob("**/CMakeLists.txt")) + return "\n".join(read_text_safely(fn) for fn in sorted(set(files))) + + +@lru_cache(maxsize=None) +def get_build_docs() -> str: + docs = ABACUS_DIR / "docs" + if not docs.exists(): + return "" + files = list(docs.glob("**/*.md")) + list(docs.glob("**/*.rst")) + return "\n".join(read_text_safely(fn) for fn in sorted(set(files))) + + +def read_text_safely(path: pathlib.Path) -> str: + try: + return path.read_text(encoding="utf8") + except Exception: + return "" + + +def check_file(path: pathlib.Path) -> List[str]: + """Check one file for ABACUS source-tree convention violations.""" + warnings: List[str] = [] + + fn_ext = path.suffix.lower() + abspath = path.resolve() + basefn = path.name + is_executable = os.access(abspath, os.X_OK) + + if not PORTABLE_FILENAME_RE.match(str(path)): + warnings += [f"Filename '{path}' not portable"] + + if not abspath.exists(): + return warnings + + raw_content = abspath.read_bytes() + if b"\0" in raw_content: + return warnings + + try: + content = raw_content.decode("utf8") + except UnicodeDecodeError: + if fn_ext in TEXT_EXTENSIONS or not fn_ext: + warnings += [f"{path}: is not valid UTF-8"] + return warnings + + if "\r\n" in content and fn_ext.lower() not in WINDOWS_SCRIPT_EXTENSIONS: + warnings += [f"{path}: contains DOS linebreaks"] + + if ( + fn_ext not in (".pot", ".patch", ".diff") + and basefn not in ("Makefile",) + and "\t" in content + ): + warnings += [f"{path}: contains tab character"] + + if MERGE_CONFLICT_RE.search(content): + warnings += [f"{path}: contains merge conflict marker"] + + # check shebang, matching CP2K's behavior for executable Python scripts + PY_SHEBANG = "#!/usr/bin/env python3" + if fn_ext == ".py" and is_executable and not content.startswith(f"{PY_SHEBANG}\n"): + warnings += [f"{path}: Wrong shebang, please use '{PY_SHEBANG}'"] + + # C++ headers should not introduce namespace pollution into downstream TUs. + # Be conservative: only reject a clearly global using-directive. Local + # using-directives inside inline/template functions are not ideal, but they + # do not leak into downstream translation units and should not be a first + # version hard failure. + if fn_ext in HEADER_EXTENSIONS: + for i, line in enumerate(content.splitlines(), start=1): + stripped = line.strip() + if line == stripped and stripped.startswith("using namespace "): + warnings += [ + f"{path}:{i} Do not use '{stripped}' at global scope in a header file" + ] + + # Find likely ABACUS build-controlled preprocessor condition flags and check + # whether ABACUS' CMake files know about them. Unlike CP2K, ABACUS has many + # C/C++ header guards and third-party compatibility macros, so this must not + # treat every uppercase token in #if/#ifdef/#ifndef as a project flag. + if fn_ext in C_EXTENSIONS: + flags = find_preprocessor_flags(content, fn_ext, basefn, path) + cmake_sources = get_cmake_sources() + for flag in sorted(flags): + if flag not in cmake_sources: + warnings += [f"{path}: Flag '{flag}' not mentioned in CMake files"] + + # Check CMake options against docs if docs exist. + if "cmake" in str(path).lower() or basefn == "CMakeLists.txt": + build_docs = get_build_docs() + if build_docs: + options = CMAKE_OPTION_RE.findall(content) + for opt in options: + if opt not in build_docs: + warnings += [f"{path}: CMake option {opt} not mentioned in docs"] + + return warnings + + +BUILD_FLAG_RE = re.compile( + r"^(ENABLE_.+|USE_.+|WITH_.+|ABACUS_.+|__UT_.+|__MPI|__CUDA|__ROCM|__EXX|__LCAO|__OPENMP)$" +) +HEADER_GUARD_RE = re.compile(r"^[A-Z0-9_]+_(H|HH|HPP|HXX|CUH)_?$") +SYSTEM_OR_COMPILER_MACRO_RE = re.compile(r"^__[A-Z0-9_]+$|^__[A-Z0-9_]+__$") + + +def find_preprocessor_flags( + content: str, fn_ext: str, basefn: str, path: pathlib.Path +) -> Set[str]: + flags: Set[str] = set() + line_continuation = False + include_guards = find_include_guards(content) + include_guard = basefn.upper().replace(".", "_") + + for line in content.splitlines(): + line = line.lstrip() + + if not line_continuation: + if not line or line[0] != "#": + continue + if line.split()[0] not in ("#if", "#ifdef", "#ifndef", "#elif"): + continue + + line = line.split("/*", 1)[0] + line = line.split("//", 1)[0] + line_continuation = line.rstrip().endswith("\\") + line = OP_RE.sub(" ", line) + line = line.replace("defined", " ") + + for word in line.split()[1:]: + if NUM_RE.match(word): + continue + if fn_ext in HEADER_EXTENSIONS and ( + word == include_guard + or word in include_guards + or HEADER_GUARD_RE.fullmatch(word) + ): + continue + if FLAG_EXCEPTIONS_RE.fullmatch(word): + continue + if SYSTEM_OR_COMPILER_MACRO_RE.fullmatch(word): + continue + if not BUILD_FLAG_RE.fullmatch(word): + continue + flags.add(word) + + return {flag for flag in flags if not FLAG_EXCEPTIONS_RE.fullmatch(flag)} + + +def find_include_guards(content: str) -> Set[str]: + """Return classic #ifndef/#define include guards found near file starts. + + This catches guards such as BASE_THIRD_PARTY_CUSOLVER_H_, not just the + basename-derived CUSOLVER_H used by the first draft. + """ + guards: Set[str] = set() + lines = content.splitlines()[:50] + for idx, line in enumerate(lines[:-1]): + m_ifndef = re.match(r"\s*#\s*ifndef\s+([A-Za-z_][A-Za-z0-9_]*)\b", line) + if not m_ifndef: + continue + guard = m_ifndef.group(1) + for next_line in lines[idx + 1 : idx + 6]: + m_define = re.match( + r"\s*#\s*define\s+([A-Za-z_][A-Za-z0-9_]*)\b", next_line + ) + if m_define: + if m_define.group(1) == guard: + guards.add(guard) + break + return guards + + +def pairwise(iterable: Iterable[T]) -> Iterable[Tuple[T, T]]: + """itertools.pairwise is not available before Python 3.10.""" + a, b = iter(iterable), iter(iterable) + next(b, None) + return zip(a, b) + + +# ====================================================================================== +if __name__ == "__main__": + parser = argparse.ArgumentParser( + description="Check the given FILENAME for ABACUS source-tree conventions" + ) + parser.add_argument("files", metavar="FILENAME", type=pathlib.Path, nargs="+") + args = parser.parse_args() + + all_warnings = [] + for fpath in args.files: + all_warnings += check_file(fpath) + + for warning in all_warnings: + print(warning) + + if all_warnings: + sys.exit(1) + +# EOF diff --git a/tools/precommit/clang_format_wrapper.sh b/tools/precommit/clang_format_wrapper.sh new file mode 100755 index 00000000000..57bd0533d8d --- /dev/null +++ b/tools/precommit/clang_format_wrapper.sh @@ -0,0 +1,28 @@ +#!/bin/bash + +# Based on CP2K's clang_format_wrapper.sh, adapted for ABACUS. + +if (($# != 1)); then + echo "Usage: clang_format_wrapper.sh " + exit 1 +fi + +STYLE_FILE="/opt/abacus-precommit/abacus_clang_format.yaml" +if [[ -f "${STYLE_FILE}" ]]; then + CLANG_FORMAT_STYLE="file:${STYLE_FILE}" +else + CLANG_FORMAT_STYLE="llvm" +fi + +# For some files clang-format can require too much memory. As a work-around we +# use the "clang-format off" marker to disable formatting. To actually reduce +# the memory usage this script hides the offending sections from clang-format. +if csplit --quiet --prefix="section" "$1" "/clang-format off/"; then + clang-format --style="${CLANG_FORMAT_STYLE}" -i section00 + cat section* > "$1" + rm section* +else + clang-format --style="${CLANG_FORMAT_STYLE}" -i "$1" +fi + +# EOF diff --git a/tools/precommit/cloudbuild.yaml b/tools/precommit/cloudbuild.yaml new file mode 100644 index 00000000000..e36f46d6895 --- /dev/null +++ b/tools/precommit/cloudbuild.yaml @@ -0,0 +1,31 @@ +# Based on CP2K's tools/precommit/cloudbuild.yaml, adapted for ABACUS. +# Adjust image name, service name, and region before enabling in production. + +substitutions: + _IMAGE_NAME: "us-central1-docker.pkg.dev/${PROJECT_ID}/misc/img_abacus_precommit" + _REGION: "us-central1" + _SERVICE: "abacus-precommit" + +steps: +- name: 'gcr.io/cloud-builders/docker' + args: ["build", "--build-arg", "REVISION=${SHORT_SHA}", "-t", "${_IMAGE_NAME}:${SHORT_SHA}", "./tools/precommit/"] + +- name: 'gcr.io/cloud-builders/docker' + args: ["push", "${_IMAGE_NAME}:${SHORT_SHA}"] + +- name: 'gcr.io/cloud-builders/docker' + args: ["tag", "${_IMAGE_NAME}:${SHORT_SHA}", "${_IMAGE_NAME}:latest"] + +- name: 'gcr.io/cloud-builders/docker' + args: ["push", "${_IMAGE_NAME}:latest"] + +- name: "gcr.io/cloud-builders/gcloud" + args: + - "run" + - "deploy" + - "${_SERVICE}" + - "--platform=managed" + - "--region=${_REGION}" + - "--image=${_IMAGE_NAME}:${SHORT_SHA}" + +# EOF diff --git a/tools/precommit/deploy.sh b/tools/precommit/deploy.sh new file mode 100755 index 00000000000..0c4e0acbb73 --- /dev/null +++ b/tools/precommit/deploy.sh @@ -0,0 +1,23 @@ +#!/bin/bash -e + +# Based on CP2K's tools/precommit/deploy.sh, adapted for ABACUS. +# Adjust PROJECT/REGISTRY/REGION before using for production deployment. + +set -x + +: "${ABACUS_PRECOMMIT_IMAGE:=abacus-precommit}" +: "${ABACUS_PRECOMMIT_REGISTRY:=gcr.io/abacus-project/img_abacus_precommit}" +: "${ABACUS_PRECOMMIT_REGION:=us-central1}" + +SHORT_SHA=$(git rev-parse --short HEAD) +docker build --build-arg "REVISION=${SHORT_SHA}" -t "${ABACUS_PRECOMMIT_IMAGE}" . + +docker tag "${ABACUS_PRECOMMIT_IMAGE}" "${ABACUS_PRECOMMIT_REGISTRY}:${SHORT_SHA}" +docker tag "${ABACUS_PRECOMMIT_IMAGE}" "${ABACUS_PRECOMMIT_REGISTRY}:latest" +docker push "${ABACUS_PRECOMMIT_REGISTRY}:${SHORT_SHA}" +docker push "${ABACUS_PRECOMMIT_REGISTRY}:latest" + +gcloud run deploy abacus-precommit --platform=managed --region="${ABACUS_PRECOMMIT_REGION}" \ + --image="${ABACUS_PRECOMMIT_REGISTRY}:${SHORT_SHA}" + +# EOF diff --git a/tools/precommit/format_makefile.py b/tools/precommit/format_makefile.py new file mode 100755 index 00000000000..39f880120d8 --- /dev/null +++ b/tools/precommit/format_makefile.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 + +# author: Ole Schuett + +import re +import sys +from pathlib import Path + + +def main() -> None: + if len(sys.argv) != 2: + print("Usage: format_makefile.py ") + sys.exit(1) + makefile = Path(sys.argv[1]) + + lines_out = [] + continuation = False + for line in makefile.read_text(encoding="utf8").split("\n"): + # Remove trailing whitespaces. + line = line.rstrip() + + # Detect continued lines. + prev_continuation = continuation + continuation = line.endswith("\\") + + # Continued lines are indented 8 spaces. + if prev_continuation: + lines_out.append(" " * 8 + line.strip()) + + # Tabbed lines are indented with excatly one tab. + elif line.startswith("\t"): + lines_out.append("\t" + line.strip()) + + # All other lines are not indented. + else: + lines_out.append(line.strip()) + + makefile.write_text("\n".join(lines_out), encoding="utf8") + + +main() + +# EOF diff --git a/tools/precommit/install_requirements.sh b/tools/precommit/install_requirements.sh new file mode 100755 index 00000000000..ffb641c90cc --- /dev/null +++ b/tools/precommit/install_requirements.sh @@ -0,0 +1,32 @@ +#!/bin/bash -e + +# Based on CP2K's tools/precommit/install_requirements.sh, adapted for ABACUS. + +export DEBIAN_FRONTEND=noninteractive DEBCONF_NONINTERACTIVE_SEEN=true +apt-get update -qq +apt-get install -qq --no-install-recommends \ + ca-certificates \ + clang-format \ + git \ + less \ + nano \ + python3 \ + python3-venv \ + python3-pip \ + python3-wheel \ + python3-setuptools \ + shellcheck \ + wget +rm -rf /var/lib/apt/lists/* + +python3 -m venv /opt/venv +export PATH="/opt/venv/bin:$PATH" +pip3 install --quiet -r requirements.txt + +# Install shfmt. +wget -q https://github.com/mvdan/sh/releases/download/v3.2.2/shfmt_v3.2.2_linux_amd64 +echo '3a32a69286a19491a81fcd854154f0d886c379ff28d99e32d5594490b8bbef4b shfmt_v3.2.2_linux_amd64' | sha256sum --check +chmod +x shfmt_v3.2.2_linux_amd64 +ln -s /opt/abacus-precommit/shfmt_v3.2.2_linux_amd64 /usr/bin/shfmt + +# EOF diff --git a/tools/precommit/precommit.py b/tools/precommit/precommit.py new file mode 100755 index 00000000000..bb33249a290 --- /dev/null +++ b/tools/precommit/precommit.py @@ -0,0 +1,308 @@ +#!/usr/bin/env python3 + +# Based on CP2K's tools/precommit/precommit.py, adapted for ABACUS. + +import ast +import argparse +import concurrent.futures +from difflib import unified_diff +from http.client import HTTPResponse +import json +import os +from pathlib import Path +import re +from subprocess import PIPE, STDOUT +import subprocess +import shutil +import sys +from time import sleep, time +from typing import cast, Iterator +from urllib.error import HTTPError +from urllib.request import Request, urlopen +import uuid + +SCRATCH_DIR = Path("./obj/precommit") +CACHE_FILE = SCRATCH_DIR / "cache.json" +SERVER = os.environ.get("ABACUS_PRECOMMIT_SERVER", "https://precommit.abacus.org") + +C_CPP_EXT_RE = re.compile(r".*\.(c|cc|cpp|cxx|h|hh|hpp|hxx|cu|cuh|hip|hip.hpp|cl)$") + + +# ====================================================================================== +def main() -> None: + parser = argparse.ArgumentParser( + description="Check source code for formatting and linter problems." + ) + parser.add_argument( + "-a", + "--no-cache", + action="store_true", + help="ignore the cache and check all files", + ) + parser.add_argument( + "-m", + "--allow-modifications", + action="store_true", + help="allow the tools to modify files", + ) + parser.add_argument( + "-j", + "--num_workers", + type=int, + default=min(16, (os.cpu_count() or 0) + 2), + help="number of parallel workers", + ) + parser.add_argument( + "--progressbar-wait", + metavar="SECONDS", + type=int, + default=1, + help="number seconds in between progressbar updates", + ) + parser.add_argument( + "-t", + "--timeout", + type=int, + default=10, + help="server wait timeout in seconds", + ) + parser.add_argument("files", metavar="FILE", nargs="*", help="files to process") + args = parser.parse_args() + + print( + f"Running precommit checks using {args.num_workers} workers and server: {SERVER} " + f"(server wait timeout: {args.timeout} seconds)" + ) + server_hello = ( + urlopen(Request(SERVER + "/"), timeout=args.timeout).read().decode("utf8") + ) + assert server_hello.startswith("abacus precommit server") + + # Store candidates before changing base directory and creating scratch dir. + file_list = [os.path.abspath(fn) for fn in args.files] + base_dir = Path(__file__).resolve().parent.parent.parent + os.chdir(base_dir) + SCRATCH_DIR.mkdir(parents=True, exist_ok=True) + + # Collect candidate files. This intentionally follows CP2K's behavior: + # without explicit file arguments, check git-tracked files first and fall + # back to walking the working tree only if git is not available. + if not file_list: + sys.stdout.write("Searching for files...\r") + sys.stdout.flush() + + walk_list = [] + try: + output = subprocess.check_output(["git", "ls-files"], encoding="utf8") + for line in filter(None, output.split("\n")): + dir_name, file_name = os.path.split(line) + walk_list.append((os.path.join(".", dir_name), [dir_name], [file_name])) + except Exception: + walk_list = [] + + for root, dirs, files in walk_list if walk_list else os.walk("."): + if should_skip_root(root): + continue + file_list += [os.path.join(root, fn) for fn in files] + + # Filter symlinks, backup copies, logs, hidden files, and very large files. + file_list = [fn for fn in file_list if os.path.exists(fn)] + file_list = [fn for fn in file_list if not os.path.isdir(fn)] + file_list = [fn for fn in file_list if not os.path.islink(fn)] + file_list = [fn for fn in file_list if not fn[-1] in ("~", "#")] + file_list = [fn for fn in file_list if not fn.endswith(".log")] + file_list = [fn for fn in file_list if not os.path.basename(fn).startswith(".")] + + # Sort files by size as larger ones will take longer to process. + file_list.sort(reverse=True, key=lambda fn: os.path.getsize(fn)) + + # Load cache. Keep CP2K's mtime-based cache semantics. + should_load_cache = CACHE_FILE.exists() and not args.no_cache + cache = json.loads(CACHE_FILE.read_text()) if should_load_cache else {} + + # Launch async processing of files. + futures = {} + executor = concurrent.futures.ThreadPoolExecutor(max_workers=args.num_workers) + for fn in file_list: + if os.path.getmtime(fn) != cache.get(fn, -1): + futures[fn] = executor.submit(process_file, fn, args.allow_modifications) + num_skipped = len(file_list) - len(futures) + + # Continuously update progressbar, save cache file, and print errors. + failed_files = set() + while True: + num_done = num_skipped + for fn, f in futures.items(): + if f.done(): + num_done += 1 + if not f.exception(): + cache[fn] = os.path.getmtime(fn) + elif fn not in failed_files: + failed_files.add(fn) + print_box(fn, str(f.exception())) + CACHE_FILE.write_text(json.dumps(cache)) + if file_list: + progressbar = "=" * int(60 * num_done / len(file_list)) + sys.stdout.write( + f"[{progressbar:60s}] {num_done} / {len(file_list)} files processed\r" + ) + sys.stdout.flush() + if num_done == len(file_list) or len(failed_files) >= 10: + executor.shutdown(wait=False) + break + sleep(args.progressbar_wait) + + print( + f"Summary: Found {len(file_list)}, " + f"skipped {num_skipped}, " + f"checked {num_done - num_skipped}, " + f"and failed {len(failed_files)} files." + (" " * 50) + ) + print("Status: " + ("FAILED" if failed_files else "OK")) + sys.exit(len(failed_files)) + + +# ====================================================================================== +def should_skip_root(root: str) -> bool: + # Keep the CP2K style: explicit project-local exclusions in the driver. + skipped_prefixes = ( + "./.git", + "./obj", + "./build", + "./cmake-build", + "./CMakeFiles", + "./external", + "./third_party", + "./deps", + "./lib", + "./bin", + "./docs/_build", + "./__pycache__", + ) + if root.startswith(skipped_prefixes): + return True + skipped_fragments = ( + "/.mypy_cache/", + "/.pytest_cache/", + "/__pycache__/", + "/CMakeFiles/", + ) + return any(fragment in root for fragment in skipped_fragments) + + +# ====================================================================================== +def print_box(fn: str, message: str) -> None: + print("+" + "-" * 160 + "+") + print(f"| {fn:^158s} |") + print("+" + "-" * 160 + "+") + for line in message.strip().split("\n"): + print(f"| {line:<158s} |") + print("+" + "-" * 160 + "+\n\n") + + +# ====================================================================================== +def process_file(fn: str, allow_modifications: bool) -> None: + basename = Path(fn).name + orig_content = Path(fn).read_bytes() + bak_fn = SCRATCH_DIR / f"{basename}_{time()}.bak" + shutil.copy2(fn, bak_fn) + + # C, C++, CUDA, HIP, OpenCL: ABACUS is C++-first-class, unlike CP2K. + if C_CPP_EXT_RE.match(fn): + run_remote_tool("clangformat", fn) + + # Python. + if re.match(r".*\.py$", fn): + ast.parse(orig_content, filename=fn) + run_remote_tool("black", fn) + + # Shell. (TODO: Re-enable shellcheck in the future) + # if re.match(r".*\.sh$", fn): + # run_remote_tool("shfmt", fn) + # run_remote_tool("shellcheck", fn) + + # Markdown. + if re.match(r".*\.md$", fn): + run_remote_tool("mdformat", fn) + + # CMake. + if re.match(r"(.*/CMakeLists.txt)|(.*\.cmake)$", fn): + run_remote_tool("cmakeformat", fn) + + # Makefile. + if re.match(r".*/Makefile", fn) or basename == "Makefile": + run_local_tool("./tools/precommit/format_makefile.py", fn) + + # Always run local project convention checks after formatters. + run_check_file_properties(fn) + + new_content = Path(fn).read_bytes() + if new_content == orig_content: + bak_fn.unlink() + elif not allow_modifications: + bak_fn.replace(fn) + diff: Iterator[str] + try: + orig_lines = orig_content.decode("utf8").split("\n") + new_lines = new_content.decode("utf8").split("\n") + diff = unified_diff(orig_lines, new_lines, "before", "after", lineterm="") + except Exception: + diff = iter([]) + raise Exception("File modified:\n" + "\n".join(diff)) + + +# ====================================================================================== +def run_check_file_properties(fn: str) -> None: + run_local_tool("./tools/precommit/check_file_properties.py", fn) + + +# ====================================================================================== +def run_local_tool(*cmd: str, timeout: int = 30) -> None: + p = subprocess.run(cmd, timeout=timeout, stdout=PIPE, stderr=STDOUT) + if p.returncode != 0: + raise Exception(p.stdout.decode("utf8")) + + +# ====================================================================================== +def run_remote_tool(tool: str, fn: str) -> None: + if os.path.getsize(fn) > 2**20: + return # skip files larger than 1MiB, matching CP2K behavior + + url = f"{SERVER}/{tool}" + r = http_post(url, fn) + if r.status == 304: + pass + elif r.status == 200: + Path(fn).write_bytes(r.read()) + else: + raise Exception(r.read().decode("utf8")) + + +# ====================================================================================== +def http_post(url: str, fn: str) -> HTTPResponse: + boundary = uuid.uuid1().hex + name = Path(fn).name + data = b"".join( + [ + f"--{boundary}\r\nContent-Disposition: ".encode("utf8"), + f'form-data; name="{name}"; filename="{name}"\r\n\r\n'.encode("utf8"), + Path(fn).read_bytes(), + f"\r\n--{boundary}--\r\n".encode("utf8"), + ] + ) + headers = { + "Content-Length": f"{len(data)}", + "Content-Type": f"multipart/form-data; boundary={boundary}", + } + try: + response = urlopen(Request(url, data=data, headers=headers), timeout=60) + return cast(HTTPResponse, response) + except HTTPError as err: + return cast(HTTPResponse, err) + + +# ====================================================================================== +if __name__ == "__main__": + main() + +# EOF diff --git a/tools/precommit/precommit_server.py b/tools/precommit/precommit_server.py new file mode 100755 index 00000000000..ce7993128f1 --- /dev/null +++ b/tools/precommit/precommit_server.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 + +# Based on CP2K's tools/precommit/precommit_server.py, adapted for ABACUS. + +import logging +import os +from os import path +import subprocess +from subprocess import PIPE, STDOUT +import tempfile +from time import time + +from flask import Flask, request + +app = Flask(__name__) +app.config["MAX_CONTENT_LENGTH"] = 1024 * 1024 # 1MiB +app.logger.setLevel(logging.INFO) +app.logger.info("ABACUS Precommit Server is up and running :-)") + + +# ====================================================================================== +@app.route("/") +def hello(): + return "abacus precommit server revision: " + os.environ.get("REVISION", "unknown") + + +# ====================================================================================== +@app.route("/black", methods=["POST"]) +def black(): + return run_tool(["black"]) + + +# ====================================================================================== +@app.route("/shfmt", methods=["POST"]) +def shfmt(): + return run_tool(["shfmt", "-i=2", "-ci", "-sr", "-w"]) + + +# ====================================================================================== +@app.route("/shellcheck", methods=["POST"]) +def shellcheck(): + return run_tool(["shellcheck"], timeout=30) + + +# ====================================================================================== +@app.route("/mdformat", methods=["POST"]) +@app.route("/markdownlint", methods=["POST"]) +def mdformat(): + return run_tool(["mdformat", "--wrap=100"], timeout=30) + + +# ====================================================================================== +@app.route("/clangformat", methods=["POST"]) +def clangformat(): + return run_tool(["clang_format_wrapper.sh"]) + + +# ====================================================================================== +@app.route("/cmakeformat", methods=["POST"]) +def cmakeformat(): + return run_tool(["cmake-format", "-i"], timeout=30) + + +# ====================================================================================== +def run_tool(cmd, timeout=3): + assert len(request.files) == 1 + orig_fn = list(request.files.keys())[0] + data_before = request.files[orig_fn].read() + data_kb = len(data_before) / 1024.0 + fn = path.basename(orig_fn) + workdir = tempfile.TemporaryDirectory() + abs_fn = path.join(workdir.name, fn) + open(abs_fn, "wb").write(data_before) + + t1 = time() + try: + p = subprocess.run( + cmd + [fn], cwd=workdir.name, timeout=timeout, stdout=PIPE, stderr=STDOUT + ) + except subprocess.TimeoutExpired: + app.logger.info(f"Timeout of {cmd[0]} on {data_kb:.1f}KB after {timeout}s.") + return f"Timeout while running {cmd[0]} - please try again.", 408 + t2 = time() + app.logger.info(f"Ran {cmd[0]} on {data_kb:.1f}KB in {t2-t1:.1f}s.") + + if p.returncode != 0: + return p.stdout, 422 + data_after = open(abs_fn, "rb").read() + if data_after == data_before: + return "Not Modified", 304 + return data_after, 200 + + +# ====================================================================================== +if __name__ == "__main__": + app.run() + +# EOF diff --git a/tools/precommit/requirements.txt b/tools/precommit/requirements.txt new file mode 100644 index 00000000000..a7853c39e68 --- /dev/null +++ b/tools/precommit/requirements.txt @@ -0,0 +1,30 @@ +black==26.3.1 +blinker==1.9.0 +click==8.3.1 +cmake-format==0.6.13 +cmakelang==0.6.13 +Flask==3.1.3 +gunicorn==25.1.0 +itsdangerous==2.2.0 +Jinja2==3.1.6 +markdown-it-py==3.0.0 +MarkupSafe==3.0.3 +mdformat==0.7.22 +mdformat-gfm==1.0.0 +mdformat_footnote==0.1.3 +mdformat_front_matters==2.0.0 +mdformat_frontmatter==2.0.10 +mdformat_myst==0.3.0 +mdformat_tables==1.0.0 +mdit-py-plugins==0.5.0 +mdurl==0.1.2 +mypy_extensions==1.1.0 +packaging==26.0 +pathspec==1.0.4 +platformdirs==4.9.2 +pytokens==0.4.1 +ruamel.yaml==0.19.1 +six==1.17.0 +toml==0.10.2 +wcwidth==0.6.0 +Werkzeug==3.1.6 diff --git a/tools/precommit/start_local_server.sh b/tools/precommit/start_local_server.sh new file mode 100755 index 00000000000..e4ce0b15edd --- /dev/null +++ b/tools/precommit/start_local_server.sh @@ -0,0 +1,11 @@ +#!/bin/bash -e + +# Based on CP2K's tools/precommit/start_local_server.sh, adapted for ABACUS. + +set -x + +SHORT_SHA=$(git rev-parse --short HEAD) +podman build --build-arg "REVISION=${SHORT_SHA}" -t abacus-precommit . +podman run --rm -p127.0.0.1:8080:8080 abacus-precommit + +# EOF From 4c28d11ef88c62e56e7dfbfb02e037ddf7fefb0c Mon Sep 17 00:00:00 2001 From: SY Wang Date: Tue, 12 May 2026 18:12:08 +0800 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tools/precommit/format_makefile.py | 6 ++-- tools/precommit/precommit_server.py | 45 +++++++++++++++-------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/tools/precommit/format_makefile.py b/tools/precommit/format_makefile.py index 39f880120d8..6e4d11f69d7 100755 --- a/tools/precommit/format_makefile.py +++ b/tools/precommit/format_makefile.py @@ -2,7 +2,6 @@ # author: Ole Schuett -import re import sys from pathlib import Path @@ -27,7 +26,7 @@ def main() -> None: if prev_continuation: lines_out.append(" " * 8 + line.strip()) - # Tabbed lines are indented with excatly one tab. + # Tabbed lines are indented with exactly one tab. elif line.startswith("\t"): lines_out.append("\t" + line.strip()) @@ -38,6 +37,7 @@ def main() -> None: makefile.write_text("\n".join(lines_out), encoding="utf8") -main() +if __name__ == "__main__": + main() # EOF diff --git a/tools/precommit/precommit_server.py b/tools/precommit/precommit_server.py index ce7993128f1..ab14e649866 100755 --- a/tools/precommit/precommit_server.py +++ b/tools/precommit/precommit_server.py @@ -68,27 +68,30 @@ def run_tool(cmd, timeout=3): data_before = request.files[orig_fn].read() data_kb = len(data_before) / 1024.0 fn = path.basename(orig_fn) - workdir = tempfile.TemporaryDirectory() - abs_fn = path.join(workdir.name, fn) - open(abs_fn, "wb").write(data_before) - - t1 = time() - try: - p = subprocess.run( - cmd + [fn], cwd=workdir.name, timeout=timeout, stdout=PIPE, stderr=STDOUT - ) - except subprocess.TimeoutExpired: - app.logger.info(f"Timeout of {cmd[0]} on {data_kb:.1f}KB after {timeout}s.") - return f"Timeout while running {cmd[0]} - please try again.", 408 - t2 = time() - app.logger.info(f"Ran {cmd[0]} on {data_kb:.1f}KB in {t2-t1:.1f}s.") - - if p.returncode != 0: - return p.stdout, 422 - data_after = open(abs_fn, "rb").read() - if data_after == data_before: - return "Not Modified", 304 - return data_after, 200 + + with tempfile.TemporaryDirectory() as workdir: + abs_fn = path.join(workdir, fn) + with open(abs_fn, "wb") as f: + f.write(data_before) + + t1 = time() + try: + p = subprocess.run( + cmd + [fn], cwd=workdir, timeout=timeout, stdout=PIPE, stderr=STDOUT + ) + except subprocess.TimeoutExpired: + app.logger.info(f"Timeout of {cmd[0]} on {data_kb:.1f}KB after {timeout}s.") + return f"Timeout while running {cmd[0]} - please try again.", 408 + t2 = time() + app.logger.info(f"Ran {cmd[0]} on {data_kb:.1f}KB in {t2-t1:.1f}s.") + + if p.returncode != 0: + return p.stdout, 422 + with open(abs_fn, "rb") as f: + data_after = f.read() + if data_after == data_before: + return "Not Modified", 304 + return data_after, 200 # ====================================================================================== From 933e149726d02b591b0de2b3a121ad90b262d5b4 Mon Sep 17 00:00:00 2001 From: SY Wang Date: Tue, 12 May 2026 18:15:56 +0800 Subject: [PATCH 3/4] Change precommit server wait loop from 300 to 200 iterations Reduce wait time for precommit server readiness check. --- .github/workflows/precommit.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml index ce597fd0186..5a1a4270df0 100644 --- a/.github/workflows/precommit.yml +++ b/.github/workflows/precommit.yml @@ -32,17 +32,14 @@ jobs: - name: Wait for precommit server run: | - for i in {1..300}; do + for i in {1..200}; do if curl -fs -o /dev/null http://127.0.0.1:8080/ 2>/dev/null; then echo "precommit server is ready" exit 0 fi sleep 1 done - echo "precommit server did not become ready" >&2 - echo "==== precommit server log ====" >&2 - cat "$RUNNER_TEMP/precommit-server.log" >&2 || true exit 1 - name: Run precommit From 881d644e3a3c7726f729cce8a97c8e85f9057792 Mon Sep 17 00:00:00 2001 From: SY Wang Date: Tue, 12 May 2026 18:17:45 +0800 Subject: [PATCH 4/4] Mark Precommit test as Expected Failure --- .github/workflows/precommit.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/precommit.yml b/.github/workflows/precommit.yml index 5a1a4270df0..cb060a40e73 100644 --- a/.github/workflows/precommit.yml +++ b/.github/workflows/precommit.yml @@ -10,7 +10,8 @@ concurrency: jobs: test: - name: Precommit + #name: Precommit + name: Expected Failure runs-on: ubuntu-latest steps: - name: Checkout repository