From 080f7e69883a935e9a09b8a0cf040cdc2a4cd57c Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Sun, 1 Dec 2024 16:44:08 +0900 Subject: [PATCH 1/7] add basic checks into precommit --- .pre-commit-config.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 63b7ab9f91..8b409e159f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -39,3 +39,20 @@ repos: dev/release/run_rat.sh apache-arrow-go.tar.gz" always_run: true pass_filenames: false + + - repo: https://github.com/koalaman/shellcheck-precommit + rev: v0.10.0 + hooks: + - id: shellcheck + - repo: https://github.com/scop/pre-commit-shfmt + rev: v3.9.0-1 + hooks: + - id: shfmt + args: + # The default args is "--write --simplify" but we don't use + # "--simplify". Because it's conflicted will ShellCheck. + - "--write" + - repo: https://github.com/google/yamlfmt + rev: v0.13.0 + hooks: + - id: yamlfmt From e5565f72a3c5db083f7ca2d9229b253c823865ac Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Sun, 1 Dec 2024 18:21:03 +0900 Subject: [PATCH 2/7] add config file for shfmt --- .editconfig | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .editconfig diff --git a/.editconfig b/.editconfig new file mode 100644 index 0000000000..1b04cdad3c --- /dev/null +++ b/.editconfig @@ -0,0 +1,20 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +[*.sh] +indent_style = space +indent_size = 2 From 3b9fa03d33e5b0e2038e18bbed79574a885010fd Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Mon, 2 Dec 2024 00:56:22 +0900 Subject: [PATCH 3/7] fix warning and info by shfmt --- ci/scripts/java_build.sh | 9 ++++----- ci/scripts/java_test.sh | 9 ++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/ci/scripts/java_build.sh b/ci/scripts/java_build.sh index b5a12d9171..df1c6b19f2 100755 --- a/ci/scripts/java_build.sh +++ b/ci/scripts/java_build.sh @@ -22,16 +22,15 @@ if [[ "${ARROW_JAVA_BUILD:-ON}" != "ON" ]]; then exit fi -arrow_dir=${1} source_dir=${1} build_dir=${2} java_jni_dist_dir=${3} -: ${BUILD_DOCS_JAVA:=OFF} +: "${BUILD_DOCS_JAVA:=OFF}" mvn="mvn -B -DskipTests -Drat.skip=true -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn" -if [ $ARROW_JAVA_SKIP_GIT_PLUGIN ]; then +if [ "$ARROW_JAVA_SKIP_GIT_PLUGIN" ]; then mvn="${mvn} -Dmaven.gitcommitid.skip=true" fi @@ -79,9 +78,9 @@ ${mvn} -T 2C clean install if [ "${BUILD_DOCS_JAVA}" == "ON" ]; then # HTTP pooling is turned of to avoid download issues https://issues.apache.org/jira/browse/ARROW-11633 # GH-43378: Maven site plugins not compatible with multithreading - mkdir -p ${build_dir}/docs/java/reference + mkdir -p "${build_dir}"/docs/java/reference ${mvn} -Dcheckstyle.skip=true -Dhttp.keepAlive=false -Dmaven.wagon.http.pool=false clean install site - rsync -a target/site/apidocs/ ${build_dir}/docs/java/reference + rsync -a target/site/apidocs/ "${build_dir}"/docs/java/reference fi popd diff --git a/ci/scripts/java_test.sh b/ci/scripts/java_test.sh index 9d4bc018b3..58841bb6f1 100755 --- a/ci/scripts/java_test.sh +++ b/ci/scripts/java_test.sh @@ -22,7 +22,6 @@ if [[ "${ARROW_JAVA_TEST:-ON}" != "ON" ]]; then exit fi -arrow_dir=${1} source_dir=${1} build_dir=${2} java_jni_dist_dir=${3} @@ -31,7 +30,7 @@ mvn="mvn -B -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMave # Use `2 * ncores` threads mvn="${mvn} -T 2C" -pushd ${build_dir} +pushd "${build_dir}" ${mvn} -Darrow.test.dataRoot="${source_dir}/testing/data" clean test @@ -44,12 +43,12 @@ fi if [ "${#projects[@]}" -gt 0 ]; then ${mvn} clean test \ -Parrow-jni \ - -pl $(IFS=,; echo "${projects[*]}") \ - -Darrow.cpp.build.dir=${java_jni_dist_dir} + -pl "$(IFS=,; echo \"${projects[*]}\")" \ + -Darrow.cpp.build.dir="${java_jni_dist_dir}" fi if [ "${ARROW_JAVA_CDATA}" = "ON" ]; then - ${mvn} clean test -Parrow-c-data -pl c -Darrow.c.jni.dist.dir=${java_jni_dist_dir} + ${mvn} clean test -Parrow-c-data -pl c -Darrow.c.jni.dist.dir="${java_jni_dist_dir}" fi popd From a3e7c7fd2960e66179384109547c6e85179cae43 Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Mon, 2 Dec 2024 01:41:35 +0900 Subject: [PATCH 4/7] fix info by shfmt --- ci/scripts/java_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/java_test.sh b/ci/scripts/java_test.sh index 58841bb6f1..e9924bfcab 100755 --- a/ci/scripts/java_test.sh +++ b/ci/scripts/java_test.sh @@ -43,7 +43,7 @@ fi if [ "${#projects[@]}" -gt 0 ]; then ${mvn} clean test \ -Parrow-jni \ - -pl "$(IFS=,; echo \"${projects[*]}\")" \ + -pl "$(IFS=,; echo \""${projects[*]}"\")" \ -Darrow.cpp.build.dir="${java_jni_dist_dir}" fi From 86318a10fd09278be1218dca09c40367667c0789 Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Wed, 4 Dec 2024 03:22:33 +0900 Subject: [PATCH 5/7] rename --- .editconfig => .editorconfig | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename .editconfig => .editorconfig (100%) diff --git a/.editconfig b/.editorconfig similarity index 100% rename from .editconfig rename to .editorconfig From 331a5f9c941afda7b5945b1180a377856299c5e9 Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Mon, 23 Dec 2024 17:41:41 +0900 Subject: [PATCH 6/7] remove yamlfmt --- .pre-commit-config.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8b409e159f..64f9e1cda9 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,7 +25,6 @@ repos: - id: check-added-large-files - id: file-contents-sorter files: .gitignore - - repo: local hooks: - id: rat @@ -39,7 +38,6 @@ repos: dev/release/run_rat.sh apache-arrow-go.tar.gz" always_run: true pass_filenames: false - - repo: https://github.com/koalaman/shellcheck-precommit rev: v0.10.0 hooks: @@ -52,7 +50,3 @@ repos: # The default args is "--write --simplify" but we don't use # "--simplify". Because it's conflicted will ShellCheck. - "--write" - - repo: https://github.com/google/yamlfmt - rev: v0.13.0 - hooks: - - id: yamlfmt From afcd76d2261d0af0e62fc34604be0e5bc48ab86c Mon Sep 17 00:00:00 2001 From: Kazuaki Ishizaki Date: Mon, 23 Dec 2024 17:42:20 +0900 Subject: [PATCH 7/7] fix format failures --- ci/scripts/java_build.sh | 6 ++-- ci/scripts/java_jni_build.sh | 56 ++++++++++++++++++------------------ ci/scripts/java_test.sh | 9 ++++-- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/ci/scripts/java_build.sh b/ci/scripts/java_build.sh index df1c6b19f2..f3f2023759 100755 --- a/ci/scripts/java_build.sh +++ b/ci/scripts/java_build.sh @@ -49,13 +49,13 @@ cp -r "${source_dir}/dev" "${build_dir}" poms=$(find "${source_dir}" -not \( -path "${source_dir}"/build -prune \) -type f -name pom.xml) if [[ "$OSTYPE" == "darwin"* ]]; then - poms=$(echo "$poms" | xargs -n1 python -c "import sys; import os.path; print(os.path.relpath(sys.argv[1], '${source_dir}'))") + poms=$(echo "$poms" | xargs -n1 python -c "import sys; import os.path; print(os.path.relpath(sys.argv[1], '${source_dir}'))") else - poms=$(echo "$poms" | xargs -n1 realpath -s --relative-to="${source_dir}") + poms=$(echo "$poms" | xargs -n1 realpath -s --relative-to="${source_dir}") fi for source_root in $(echo "${poms}" | awk -F/ '{print $1}' | sort -u); do - cp -r "${source_dir}/${source_root}" "${build_dir}" + cp -r "${source_dir}/${source_root}" "${build_dir}" done pushd "${build_dir}" diff --git a/ci/scripts/java_jni_build.sh b/ci/scripts/java_jni_build.sh index 7b63dbeeaf..44388e33fe 100755 --- a/ci/scripts/java_jni_build.sh +++ b/ci/scripts/java_jni_build.sh @@ -27,54 +27,54 @@ prefix_dir="${build_dir}/java-jni" echo "=== Clear output directories and leftovers ===" # Clear output directories and leftovers -rm -rf ${build_dir} +rm -rf "${build_dir}" echo "=== Building Arrow Java C Data Interface native library ===" mkdir -p "${build_dir}" pushd "${build_dir}" case "$(uname)" in - Linux) - n_jobs=$(nproc) - ;; - Darwin) - n_jobs=$(sysctl -n hw.logicalcpu) - ;; - *) - n_jobs=${NPROC:-1} - ;; +Linux) + n_jobs=$(nproc) + ;; +Darwin) + n_jobs=$(sysctl -n hw.logicalcpu) + ;; +*) + n_jobs=${NPROC:-1} + ;; esac -: ${ARROW_JAVA_BUILD_TESTS:=${ARROW_BUILD_TESTS:-OFF}} -: ${CMAKE_BUILD_TYPE:=release} +: "${ARROW_JAVA_BUILD_TESTS:=${ARROW_BUILD_TESTS:-OFF}}" +: "${CMAKE_BUILD_TYPE:=release}" cmake \ - -DARROW_JAVA_JNI_ENABLE_DATASET=${ARROW_DATASET:-OFF} \ - -DARROW_JAVA_JNI_ENABLE_GANDIVA=${ARROW_GANDIVA:-OFF} \ - -DARROW_JAVA_JNI_ENABLE_ORC=${ARROW_ORC:-OFF} \ - -DBUILD_TESTING=${ARROW_JAVA_BUILD_TESTS} \ - -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \ - -DCMAKE_PREFIX_PATH=${arrow_install_dir} \ - -DCMAKE_INSTALL_PREFIX=${prefix_dir} \ - -DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \ + -DARROW_JAVA_JNI_ENABLE_DATASET="${ARROW_DATASET:-OFF}" \ + -DARROW_JAVA_JNI_ENABLE_GANDIVA="${ARROW_GANDIVA:-OFF}" \ + -DARROW_JAVA_JNI_ENABLE_ORC="${ARROW_ORC:-OFF}" \ + -DBUILD_TESTING="${ARROW_JAVA_BUILD_TESTS}" \ + -DCMAKE_BUILD_TYPE="${CMAKE_BUILD_TYPE}" \ + -DCMAKE_PREFIX_PATH="${arrow_install_dir}" \ + -DCMAKE_INSTALL_PREFIX="${prefix_dir}" \ + -DCMAKE_UNITY_BUILD="${CMAKE_UNITY_BUILD:-OFF}" \ -DProtobuf_USE_STATIC_LIBS=ON \ -GNinja \ - ${JAVA_JNI_CMAKE_ARGS:-} \ - ${arrow_dir} + "${JAVA_JNI_CMAKE_ARGS:-}" \ + "${arrow_dir}" export CMAKE_BUILD_PARALLEL_LEVEL=${n_jobs} -cmake --build . --config ${CMAKE_BUILD_TYPE} +cmake --build . --config "${CMAKE_BUILD_TYPE}" if [ "${ARROW_JAVA_BUILD_TESTS}" = "ON" ]; then ctest \ --output-on-failure \ - --parallel ${n_jobs} \ + --parallel "${n_jobs}" \ --timeout 300 fi -cmake --build . --config ${CMAKE_BUILD_TYPE} --target install +cmake --build . --config "${CMAKE_BUILD_TYPE}" --target install popd -mkdir -p ${dist_dir} +mkdir -p "${dist_dir}" # For Windows. *.dll are installed into bin/ on Windows. if [ -d "${prefix_dir}/bin" ]; then - mv ${prefix_dir}/bin/* ${dist_dir}/ + mv "${prefix_dir}"/bin/* "${dist_dir}"/ else - mv ${prefix_dir}/lib/* ${dist_dir}/ + mv "${prefix_dir}"/lib/* "${dist_dir}"/ fi diff --git a/ci/scripts/java_test.sh b/ci/scripts/java_test.sh index e9924bfcab..6449bbc2c5 100755 --- a/ci/scripts/java_test.sh +++ b/ci/scripts/java_test.sh @@ -42,9 +42,12 @@ if [ "${ARROW_JAVA_JNI}" = "ON" ]; then fi if [ "${#projects[@]}" -gt 0 ]; then ${mvn} clean test \ - -Parrow-jni \ - -pl "$(IFS=,; echo \""${projects[*]}"\")" \ - -Darrow.cpp.build.dir="${java_jni_dist_dir}" + -Parrow-jni \ + -pl "$( + IFS=, + echo \""${projects[*]}"\" + )" \ + -Darrow.cpp.build.dir="${java_jni_dist_dir}" fi if [ "${ARROW_JAVA_CDATA}" = "ON" ]; then