diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index c0963a9fc5..0de8990758 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -38,4 +38,4 @@ jobs: - uses: actions/checkout@v4 - name: Run format checker run: | - bash .github/workflows/scripts/format/format.sh -check + bash .github/workflows/scripts/format/format.sh -fix diff --git a/.github/workflows/scripts/format/format.sh b/.github/workflows/scripts/format/format.sh index ed9841c210..c0f17fb088 100755 --- a/.github/workflows/scripts/format/format.sh +++ b/.github/workflows/scripts/format/format.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # Licensed to the Apache Software Foundation (ASF) under one or more # contributor license agreements. See the NOTICE file distributed with @@ -15,78 +15,24 @@ # See the License for the specific language governing permissions and # limitations under the License. -set -e -set -o pipefail -set -u +set -euo pipefail -# Parse action argument: must be -check or -fix -if [[ $# -ne 1 ]]; then - echo "Usage: $0 -check | -fix" +if [[ $# -ne 1 || "$1" != "-fix" ]]; then + echo "Usage: $0 -fix" exit 1 fi -ACTION="$1" - -if [[ "$ACTION" != "-check" && "$ACTION" != "-fix" ]]; then - echo "Invalid argument: $ACTION" - echo "Usage: $0 -check | -fix" - exit 1 -fi - -# Ensure Maven is installed. -mvn -version - # Directory containing the source code to check SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" SRC_DIR="$SCRIPT_DIR/../../../../" -# Set image name. -IMAGE_NAME=velox4j-format - -# Set Ubuntu version. -OS_VERSION=24.04 - -# Build the Docker image, passing the OS_VERSION build argument. -docker build --build-arg "OS_VERSION=$OS_VERSION" -t "$IMAGE_NAME:$OS_VERSION" "$SCRIPT_DIR" - -# Determine the clang-format command -if [[ "$ACTION" == "-check" ]]; then - FORMAT_COMMAND="clang-format-18 --dry-run --Werror" - CMAKE_FORMAT_COMMAND="cmake-format --first-comment-is-literal True --check" - MAVEN_COMMAND="spotless:check" -else - FORMAT_COMMAND="clang-format-18 -i" - CMAKE_FORMAT_COMMAND="cmake-format --first-comment-is-literal True -i" - MAVEN_COMMAND="spotless:apply" +if ! command -v pre-commit >/dev/null 2>&1; then + echo "Missing required command: pre-commit" + exit 1 fi -# CPP code path. -CPP_ROOT="$SRC_DIR/src/main/cpp/" -CPP_MAIN_DIR="main/" -CPP_TEST_DIR="test/" - -# 1. Run clang-format-18 on the CPP main code. -docker run --rm -v "$CPP_ROOT":/workspace -w /workspace "$IMAGE_NAME:$OS_VERSION" \ - sh -c "find $CPP_MAIN_DIR \( -name '*.h' -o -name '*.cc' -o -name '*.cpp' \) | xargs -r $FORMAT_COMMAND" - -# 2. Run clang-format-18 on the CPP test code. -docker run --rm -v "$CPP_ROOT":/workspace -w /workspace "$IMAGE_NAME:$OS_VERSION" \ - sh -c "find $CPP_TEST_DIR \( -name '*.h' -o -name '*.cc' -o -name '*.cpp' \) | xargs -r $FORMAT_COMMAND" - -# 3. Run cmake-format on root CMakeLists.txt. -docker run --rm -v "$CPP_ROOT":/workspace -w /workspace "$IMAGE_NAME:$OS_VERSION" \ - sh -c "$CMAKE_FORMAT_COMMAND CMakeLists.txt" - -# 4. Run cmake-format on CPP main code. -docker run --rm -v "$CPP_ROOT":/workspace -w /workspace "$IMAGE_NAME:$OS_VERSION" \ - sh -c "find $CPP_MAIN_DIR \( -name 'CMakeLists.txt' -o -name '*.cmake' \) | xargs -r $CMAKE_FORMAT_COMMAND" +PRE_COMMIT_BIN="$(command -v pre-commit)" -# 5. Run cmake-format on CPP test code. -docker run --rm -v "$CPP_ROOT":/workspace -w /workspace "$IMAGE_NAME:$OS_VERSION" \ - sh -c "find $CPP_TEST_DIR \( -name 'CMakeLists.txt' -o -name '*.cmake' \) | xargs -r $CMAKE_FORMAT_COMMAND" +cd "$SRC_DIR" -# 6. Run Maven Spotless check or apply under the project root. -( - cd "$SRC_DIR" - mvn "$MAVEN_COMMAND" -) +"$PRE_COMMIT_BIN" run --all-files diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000000..d681f00cc2 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,25 @@ +repos: + - repo: local + hooks: + - id: clang-format-fix + name: clang-format (fix) + language: python + additional_dependencies: + - clang-format==18.1.8 + entry: clang-format -i + files: ^src/main/cpp/(main|test)/.*\.(h|cc|cpp)$ + + - id: cmake-format-fix + name: cmake-format (fix) + language: python + additional_dependencies: + - cmakelang==0.6.13 + - pyyaml==6.0.3 + entry: cmake-format --first-comment-is-literal True -i + files: ^src/main/cpp/(CMakeLists\.txt|(main|test)/.*(CMakeLists\.txt|\.cmake))$ + + - id: spotless-apply + name: spotless (apply) + language: system + entry: mvn --batch-mode spotless:apply + pass_filenames: false diff --git a/README.md b/README.md index c4f2ab0511..0824878dae 100644 --- a/README.md +++ b/README.md @@ -273,13 +273,9 @@ the C++ code and Java code: bash .github/workflows/scripts/format/format.sh -fix ``` -Note, Docker environment is required to run the script. - -If you only need to check the code format without fixing them, use the subcommand`-check` instead: - -```shell -bash .github/workflows/scripts/format/format.sh -check -``` +The script is backed by `pre-commit` hooks for `clang-format`, `cmake-format`, and Spotless. +Install `pre-commit` before running it by following the official instructions: +https://pre-commit.com/#installation ## License diff --git a/src/main/cpp/main/velox4j/iterator/DownIterator.h b/src/main/cpp/main/velox4j/iterator/DownIterator.h index 8010755bdc..c194c8c059 100644 --- a/src/main/cpp/main/velox4j/iterator/DownIterator.h +++ b/src/main/cpp/main/velox4j/iterator/DownIterator.h @@ -27,7 +27,7 @@ class DownIteratorJniWrapper final : public spotify::jni::JavaClass { DownIteratorJniWrapper::initialize(env); } - DownIteratorJniWrapper() : JavaClass(){}; + DownIteratorJniWrapper() : JavaClass() {}; const char* getCanonicalName() const override; diff --git a/src/main/cpp/main/velox4j/jni/JniCommon.h b/src/main/cpp/main/velox4j/jni/JniCommon.h index c12b23e100..906971baf6 100644 --- a/src/main/cpp/main/velox4j/jni/JniCommon.h +++ b/src/main/cpp/main/velox4j/jni/JniCommon.h @@ -155,7 +155,7 @@ class SafeNativeArray { JNIEnv* env, JavaArrayType javaArray, JniNativeArrayType nativeArray) - : env_(env), javaArray_(javaArray), nativeArray_(nativeArray){}; + : env_(env), javaArray_(javaArray), nativeArray_(nativeArray) {}; JNIEnv* env_; JavaArrayType javaArray_; diff --git a/src/main/cpp/main/velox4j/lifecycle/ObjectStore.h b/src/main/cpp/main/velox4j/lifecycle/ObjectStore.h index cf753dc7f1..a2a44326c4 100644 --- a/src/main/cpp/main/velox4j/lifecycle/ObjectStore.h +++ b/src/main/cpp/main/velox4j/lifecycle/ObjectStore.h @@ -98,7 +98,7 @@ class ObjectStore { void releaseInternal(ResourceHandle handle); - explicit ObjectStore(StoreHandle storeId) : storeId_(storeId){}; + explicit ObjectStore(StoreHandle storeId) : storeId_(storeId) {}; StoreHandle storeId_; ResourceMap> store_; // Preserves handles of objects in the store in order, with the text diff --git a/src/main/cpp/main/velox4j/lifecycle/Session.h b/src/main/cpp/main/velox4j/lifecycle/Session.h index 3f007a9aab..dfd967014a 100644 --- a/src/main/cpp/main/velox4j/lifecycle/Session.h +++ b/src/main/cpp/main/velox4j/lifecycle/Session.h @@ -23,7 +23,7 @@ namespace velox4j { class Session { public: Session(MemoryManager* memoryManager) - : memoryManager_(memoryManager), objectStore_(ObjectStore::create()){}; + : memoryManager_(memoryManager), objectStore_(ObjectStore::create()) {}; virtual ~Session() = default; MemoryManager* memoryManager() { diff --git a/src/main/cpp/main/velox4j/memory/JavaAllocationListener.h b/src/main/cpp/main/velox4j/memory/JavaAllocationListener.h index 7cbb825907..cf9e80d2fc 100644 --- a/src/main/cpp/main/velox4j/memory/JavaAllocationListener.h +++ b/src/main/cpp/main/velox4j/memory/JavaAllocationListener.h @@ -26,7 +26,7 @@ class JavaAllocationListenerJniWrapper final : public spotify::jni::JavaClass { JavaAllocationListenerJniWrapper::initialize(env); } - JavaAllocationListenerJniWrapper() : JavaClass(){}; + JavaAllocationListenerJniWrapper() : JavaClass() {}; const char* getCanonicalName() const override;