diff --git a/.github/scripts/ci_changes.py b/.github/scripts/ci_changes.py index 496d7b5b..c59127c7 100644 --- a/.github/scripts/ci_changes.py +++ b/.github/scripts/ci_changes.py @@ -6,7 +6,17 @@ import tomllib from pathlib import Path -GROUPS = ("cpp", "python_quality", "python_build", "shell", "wheels") +GROUPS = ( + "cpp", + "cpp_format", + "cpp_tidy", + "python_quality", + "python_build", + "shell", + "wheels", +) + +CPP_SUFFIXES = (".c", ".cc", ".cpp", ".h", ".hpp") def classify(paths): @@ -23,19 +33,24 @@ def classify(paths): ".editorconfig", ): return set(GROUPS) - if path in ("CMakeLists.txt", "pyproject.toml") or path.startswith("cmake/"): - selected.update(("cpp", "python_quality", "python_build", "wheels")) + if path == "pyproject.toml": + selected.update(("python_quality", "python_build", "wheels")) + elif path == "CMakeLists.txt" or path.startswith("cmake/"): + selected.update(("cpp", "cpp_tidy", "python_build", "wheels")) elif path.startswith(("include/", "src/")): - selected.update(("cpp", "python_build", "wheels")) + selected.update(("cpp", "cpp_format", "cpp_tidy", "python_build", "wheels")) elif path.startswith("python_bindings/"): - selected.update(("python_quality", "python_build", "wheels")) - if not path.endswith(".py"): - selected.add("cpp") + selected.update(("python_build", "wheels")) + if path.endswith(".py"): + selected.add("python_quality") + elif path.endswith(CPP_SUFFIXES): + selected.update(("cpp_format", "cpp_tidy")) elif ( path.startswith( ( "tests/unit/", "tests/integration/", + "tests/dispatch/", "tests/common/", "tests/consumer/", "sample/cpp/", @@ -44,24 +59,34 @@ def classify(paths): or path == "tests/CMakeLists.txt" ): selected.add("cpp") - elif path.startswith(("tests/python/", "sample/python/")): + if path.endswith(CPP_SUFFIXES): + selected.add("cpp_format") + elif path.startswith("tests/python/"): + selected.update(("python_build", "wheels")) + if path.endswith(".py"): + selected.add("python_quality") + elif path.startswith("sample/python/"): selected.update(("python_quality", "python_build", "wheels")) elif path.startswith(("python/", ".github/scripts/")): selected.add("python_quality") elif path == ".github/workflows/test.yaml": - selected.update(("cpp", "shell")) + selected.update(("cpp", "cpp_format", "cpp_tidy")) elif path == ".github/workflows/python.yml": selected.update(("python_quality", "python_build")) elif path == ".github/workflows/release.yml": selected.update(("python_quality", "wheels")) - elif path in (".clang-format", ".clang-tidy"): - selected.add("cpp") + elif path == ".clang-format": + selected.add("cpp_format") + elif path == ".clang-tidy": + selected.add("cpp_tidy") elif path.startswith("scripts/") and path.endswith(".sh"): selected.add("shell") if path == "scripts/check-python.sh": selected.add("python_quality") - else: - selected.add("cpp") + elif path == "scripts/check-tidy.sh": + selected.add("cpp_tidy") + elif path != "scripts/check-includes.sh": + selected.add("cpp_format") else: # Unknown inputs may affect builds; never silently skip their checks. return set(GROUPS) diff --git a/.github/scripts/test_ci_changes.py b/.github/scripts/test_ci_changes.py index 48abb608..0d82b85a 100644 --- a/.github/scripts/test_ci_changes.py +++ b/.github/scripts/test_ci_changes.py @@ -26,22 +26,26 @@ def test_docs_only_skip_builds(self): def test_shared_code_runs_cpp_and_python(self): for path in ("include/rabitqlib/index/ivf/ivf.hpp", "src/simd/dispatch.cpp"): with self.subTest(path=path): - self.assertEqual(classify([path]), {"cpp", "python_build", "wheels"}) + self.assertEqual( + classify([path]), + {"cpp", "cpp_format", "cpp_tidy", "python_build", "wheels"}, + ) def test_language_specific_changes(self): cases = { - "tests/unit/rabitqlib/index/ivf_test.cpp": {"cpp"}, - "sample/cpp/ivf_rabitq_querying.cpp": {"cpp"}, + "tests/unit/rabitqlib/index/ivf_test.cpp": {"cpp", "cpp_format"}, + "tests/dispatch/fastscan_no_simd_test.cpp": {"cpp", "cpp_format"}, + "sample/cpp/ivf_rabitq_querying.cpp": {"cpp", "cpp_format"}, "python_bindings/ivf_bindings.cpp": { - "cpp", - "python_quality", + "cpp_format", + "cpp_tidy", "python_build", "wheels", }, "python_bindings/__init__.py": {"python_quality", "python_build", "wheels"}, + "python_bindings/licenses/LLVM-OpenMP.txt": {"python_build", "wheels"}, "tests/python/test_ivf.py": {"python_quality", "python_build", "wheels"}, "tests/python/fixtures/ivf_legacy_4bit.index": { - "python_quality", "python_build", "wheels", }, @@ -52,27 +56,33 @@ def test_language_specific_changes(self): }, ".github/scripts/prepare_release.py": {"python_quality"}, "scripts/check-python.sh": {"shell", "python_quality"}, - "scripts/check-tidy.sh": {"shell", "cpp"}, + "scripts/check-tidy.sh": {"shell", "cpp_tidy"}, + "scripts/check-format.sh": {"shell", "cpp_format"}, + "scripts/check-includes.sh": {"shell"}, + ".clang-format": {"cpp_format"}, + ".clang-tidy": {"cpp_tidy"}, + ".github/workflows/test.yaml": {"cpp", "cpp_format", "cpp_tidy"}, } for path, expected in cases.items(): with self.subTest(path=path): self.assertEqual(classify([path]), expected) - def test_build_configuration_checks_both_languages(self): - for path in ( - "pyproject.toml", - "CMakeLists.txt", - "cmake/RaBitQLibConfig.cmake.in", - ): - self.assertEqual( - classify([path]), {"cpp", "python_quality", "python_build", "wheels"} - ) + def test_build_configuration_selects_affected_checks(self): + self.assertEqual( + classify(["pyproject.toml"]), + {"python_quality", "python_build", "wheels"}, + ) + for path in ("CMakeLists.txt", "cmake/RaBitQLibConfig.cmake.in"): + with self.subTest(path=path): + self.assertEqual( + classify([path]), {"cpp", "cpp_tidy", "python_build", "wheels"} + ) def test_mixed_changes_and_renamed_source(self): self.assertIn("cpp", classify(["docs/new.md", "src/deleted.cpp"])) self.assertEqual( classify(["sample/python/tool.py", "tests/unit/example.cpp"]), - {"cpp", "python_quality", "python_build", "wheels"}, + {"cpp", "cpp_format", "python_quality", "python_build", "wheels"}, ) def test_unknown_inputs_and_filter_edits_run_everything(self): diff --git a/.github/workflows/changes.yml b/.github/workflows/changes.yml index 221dc9f9..ea98d24e 100644 --- a/.github/workflows/changes.yml +++ b/.github/workflows/changes.yml @@ -5,6 +5,10 @@ on: outputs: cpp: value: ${{ jobs.detect.outputs.cpp }} + cpp_format: + value: ${{ jobs.detect.outputs.cpp_format }} + cpp_tidy: + value: ${{ jobs.detect.outputs.cpp_tidy }} python_quality: value: ${{ jobs.detect.outputs.python_quality }} python_build: @@ -23,6 +27,8 @@ jobs: timeout-minutes: 5 outputs: cpp: ${{ steps.paths.outputs.cpp }} + cpp_format: ${{ steps.paths.outputs.cpp_format }} + cpp_tidy: ${{ steps.paths.outputs.cpp_tidy }} python_quality: ${{ steps.paths.outputs.python_quality }} python_build: ${{ steps.paths.outputs.python_build }} shell: ${{ steps.paths.outputs.shell }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 903501ef..f7ca0ae2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -13,7 +13,7 @@ jobs: format: needs: changes - if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.cpp == 'true') }} + if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.cpp_format == 'true') }} name: C++ formatting runs-on: ubuntu-latest timeout-minutes: 10 @@ -28,7 +28,7 @@ jobs: clang-tidy: needs: changes - if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.cpp == 'true') }} + if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.cpp_tidy == 'true') }} name: C++ static analysis runs-on: ubuntu-latest timeout-minutes: 30 @@ -55,44 +55,6 @@ jobs: - name: Run clang-tidy run: ./scripts/check-tidy.sh build-tidy - includes: - needs: changes - if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.cpp == 'true') }} - name: Include cleaner (advisory) - runs-on: ubuntu-latest - timeout-minutes: 30 - permissions: - contents: read - steps: - - uses: actions/checkout@v7 - - name: Install clang-tidy and build dependencies - run: | - sudo apt-get update -qq - sudo apt-get install -y clang-tidy clang libomp-dev cmake ninja-build - - name: Configure compilation database - run: >- - cmake -S . -B build-includes -G Ninja - -DCMAKE_CXX_COMPILER=clang++ - -DRABITQ_BUILD_SAMPLES=OFF - -DRABITQ_BUILD_TESTS=OFF - -DRABITQ_BUILD_PYTHON_BINDINGS=OFF - -DRABITQ_ENABLE_NATIVE_OPTIMIZATION=OFF - -DCMAKE_BUILD_TYPE=Release - - name: Analyze library includes - id: analysis - continue-on-error: true - shell: bash - run: | - set -o pipefail - ./scripts/check-includes.sh build-includes 2>&1 | tee include-report.txt - - name: Store include report - if: ${{ !cancelled() && steps.analysis.outcome != 'skipped' }} - uses: actions/upload-artifact@v7 - with: - name: include-report - path: include-report.txt - if-no-files-found: error - shellcheck: needs: changes if: ${{ !cancelled() && (needs.changes.result != 'success' || needs.changes.outputs.shell == 'true') }} diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index d2f5d368..405692d1 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -90,9 +90,9 @@ whether validation was focused or complete. ### Include dependency reports -The **Include cleaner (advisory)** job uses clang-tidy's `misc-include-cleaner` -check to report missing and unused includes. CI uses `ubuntu-latest` and -unversioned distribution packages: +The optional include dependency check uses clang-tidy's +`misc-include-cleaner` to report missing and unused includes. Run it locally when +reviewing include changes. On Ubuntu, install unversioned distribution packages: ```bash sudo apt-get update @@ -135,8 +135,7 @@ Vendored files are excluded. The script also ignores suggestions to include Eigen and hnswlib implementation headers behind their existing public headers; these vendor snapshots lack the export annotations needed by include-cleaner. `INCLUDE_JOBS` controls parallelism (default: 2). -The script fails for findings or analyzer errors; CI keeps this step advisory -and uploads the `include-report` artifact without modifying files. +The script fails for findings or analyzer errors and does not modify files. Review suggestions before applying them, especially for templates and public forwarding headers. For a source file, automatic fixes can be applied with: diff --git a/MAINTAINING.md b/MAINTAINING.md index 0c419a09..6f328f74 100644 --- a/MAINTAINING.md +++ b/MAINTAINING.md @@ -11,15 +11,18 @@ only the affected job groups: | Changes | Checks | | --- | --- | | Documentation and Markdown only | Documentation workflow for `docs/`; no C++ or Python builds | -| C++ headers or library sources | C++ formatting, analysis, tests, sanitizers, consumer build, and Python wheels/tests | -| C++ tests or examples | C++ checks | -| Python bindings | Python checks and wheel tests; C++ checks for compiled binding files | +| C++ headers or library sources | C++ formatting, clang-tidy, platform tests, sanitizers, consumer build, and Python builds/wheels | +| C++ tests or examples | C++ formatting and platform tests; no clang-tidy | +| Compiled Python bindings | C++ formatting and clang-tidy, plus Python checks and wheel tests | | Python tests | Python checks and wheel tests | | Python scripts or examples | Python lint | -| CMake or package configuration | C++ and Python build/test checks | -| Shell scripts | ShellCheck and the checks driven by those scripts | +| CMake configuration | C++ tests and clang-tidy, plus Python builds/wheels | +| `pyproject.toml` | Python lint, builds, and wheels; no C++ checks | +| Shell scripts | ShellCheck and any check driven by the changed script | -Shared headers still trigger broad regression suites; CI does not infer +The optional include-cleaner report is available through +`./scripts/check-includes.sh` and no longer runs on every C++ change. Shared +headers still trigger broad regression suites; CI does not infer individual test dependencies from C++ function changes. Unknown paths run all checks. Pushes compare the complete pushed range; pull requests compare against their base. Renames and deletions are included. Relevant jobs also run if