From 1a8e38b4ca1e63fcfceb164d578c6ab63d2282ef Mon Sep 17 00:00:00 2001 From: Gil Forsyth Date: Mon, 10 Feb 2025 09:38:07 -0500 Subject: [PATCH] Add `shellcheck` to pre-commit and fix warnings (#2575) `shellcheck` is a fast, static analysis tool for shell scripts. It's good at flagging up unused variables, unintentional glob expansions, and other potential execution and security headaches that arise from the wonders of `bash` (and other shlangs). This PR adds a `pre-commit` hook to run `shellcheck` on all of the `sh-lang` files in the `ci/` directory, and the changes requested by `shellcheck` to make the existing files pass the check. xref: rapidsai/build-planning#135 Authors: - Gil Forsyth (https://github.com/gforsyth) Approvers: - James Lamb (https://github.com/jameslamb) URL: https://github.com/rapidsai/raft/pull/2575 --- .pre-commit-config.yaml | 6 ++++++ ci/build_docs.sh | 6 ++++-- ci/build_python.sh | 1 - ci/build_wheel.sh | 4 ++-- ci/build_wheel_libraft.sh | 4 +--- ci/build_wheel_pylibraft.sh | 4 ++-- ci/build_wheel_raft_dask.sh | 4 ++-- ci/check_style.sh | 4 ++-- ci/checks/black_lists.sh | 18 +++++++++--------- ci/release/update-version.sh | 13 ++++++------- ci/test_cpp.sh | 2 +- ci/test_wheel_pylibraft.sh | 2 +- ci/test_wheel_raft_dask.sh | 4 ++-- ci/validate_wheel.sh | 6 ++---- 14 files changed, 40 insertions(+), 38 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6dfcc72417..21dc20e776 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -127,6 +127,12 @@ repos: hooks: - id: rapids-dependency-file-generator args: ["--clean"] + - repo: https://github.com/shellcheck-py/shellcheck-py + rev: v0.10.0.1 + hooks: + - id: shellcheck + args: ["--severity=warning"] + files: ^ci/ default_language_version: python: python3 diff --git a/ci/build_docs.sh b/ci/build_docs.sh index aff7674892..54dbb2b599 100755 --- a/ci/build_docs.sh +++ b/ci/build_docs.sh @@ -7,7 +7,8 @@ rapids-logger "Create test conda environment" . /opt/conda/etc/profile.d/conda.sh RAPIDS_VERSION="$(rapids-version)" -export RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" +RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" +export RAPIDS_VERSION_MAJOR_MINOR rapids-dependency-file-generator \ --output conda \ @@ -31,7 +32,8 @@ rapids-mamba-retry install \ "pylibraft=${RAPIDS_VERSION}" \ "raft-dask=${RAPIDS_VERSION}" -export RAPIDS_DOCS_DIR="$(mktemp -d)" +RAPIDS_DOCS_DIR="$(mktemp -d)" +export RAPIDS_DOCS_DIR rapids-logger "Build CPP docs" pushd cpp/doxygen diff --git a/ci/build_python.sh b/ci/build_python.sh index 7da665075f..131f99c212 100755 --- a/ci/build_python.sh +++ b/ci/build_python.sh @@ -18,7 +18,6 @@ rapids-logger "Begin py build" CPP_CHANNEL=$(rapids-download-conda-from-s3 cpp) version=$(rapids-generate-version) -git_commit=$(git rev-parse HEAD) export RAPIDS_PACKAGE_VERSION=${version} echo "${version}" > VERSION diff --git a/ci/build_wheel.sh b/ci/build_wheel.sh index e2e8919b95..845ac128f2 100755 --- a/ci/build_wheel.sh +++ b/ci/build_wheel.sh @@ -15,7 +15,7 @@ rm -rf /usr/lib64/libuc* source rapids-configure-sccache source rapids-date-string -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" +RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")" rapids-generate-version > ./VERSION @@ -53,4 +53,4 @@ sccache --show-adv-stats mkdir -p final_dist python -m auditwheel repair -w final_dist "${EXCLUDE_ARGS[@]}" dist/* -RAPIDS_PY_WHEEL_NAME="${underscore_package_name}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 ${package_type} final_dist +RAPIDS_PY_WHEEL_NAME="${underscore_package_name}_${RAPIDS_PY_CUDA_SUFFIX}" rapids-upload-wheels-to-s3 "${package_type}" final_dist diff --git a/ci/build_wheel_libraft.sh b/ci/build_wheel_libraft.sh index 10c69e1601..4468da37cd 100755 --- a/ci/build_wheel_libraft.sh +++ b/ci/build_wheel_libraft.sh @@ -26,7 +26,5 @@ rapids-pip-retry install \ # 0 really means "add --no-build-isolation" (ref: https://github.com/pypa/pip/issues/5735) export PIP_NO_BUILD_ISOLATION=0 -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" - ci/build_wheel.sh libraft ${package_dir} cpp -ci/validate_wheel.sh ${package_dir} final_dist libraft +ci/validate_wheel.sh ${package_dir} final_dist diff --git a/ci/build_wheel_pylibraft.sh b/ci/build_wheel_pylibraft.sh index 6f74e0e8c5..aed58446d2 100755 --- a/ci/build_wheel_pylibraft.sh +++ b/ci/build_wheel_pylibraft.sh @@ -5,7 +5,7 @@ set -euo pipefail package_dir="python/pylibraft" -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" +RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")" # Downloads libraft wheels from this current build, # then ensures 'pylibraft' wheel builds always use the 'libraft' just built in the same CI run. @@ -17,4 +17,4 @@ echo "libraft-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo /tmp/libraft_dist/libraft export PIP_CONSTRAINT="/tmp/constraints.txt" ci/build_wheel.sh pylibraft ${package_dir} python -ci/validate_wheel.sh ${package_dir} final_dist pylibraft +ci/validate_wheel.sh ${package_dir} final_dist diff --git a/ci/build_wheel_raft_dask.sh b/ci/build_wheel_raft_dask.sh index 0cacb6fe30..8241f7aff2 100755 --- a/ci/build_wheel_raft_dask.sh +++ b/ci/build_wheel_raft_dask.sh @@ -5,7 +5,7 @@ set -euo pipefail package_dir="python/raft-dask" -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" +RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")" # Downloads libraft wheels from this current build, # then ensures 'raft-dask' wheel builds always use the 'libraft' just built in the same CI run. @@ -17,4 +17,4 @@ echo "libraft-${RAPIDS_PY_CUDA_SUFFIX} @ file://$(echo /tmp/libraft_dist/libraft export PIP_CONSTRAINT="/tmp/constraints.txt" ci/build_wheel.sh raft-dask ${package_dir} python -ci/validate_wheel.sh ${package_dir} final_dist raft-dask +ci/validate_wheel.sh ${package_dir} final_dist diff --git a/ci/check_style.sh b/ci/check_style.sh index e0c30a2d41..3505035af8 100755 --- a/ci/check_style.sh +++ b/ci/check_style.sh @@ -18,8 +18,8 @@ conda activate checks RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)" FORMAT_FILE_URL="https://raw.githubusercontent.com/rapidsai/rapids-cmake/branch-${RAPIDS_VERSION_MAJOR_MINOR}/cmake-format-rapids-cmake.json" export RAPIDS_CMAKE_FORMAT_FILE=/tmp/rapids_cmake_ci/cmake-formats-rapids-cmake.json -mkdir -p $(dirname ${RAPIDS_CMAKE_FORMAT_FILE}) -wget -O ${RAPIDS_CMAKE_FORMAT_FILE} ${FORMAT_FILE_URL} +mkdir -p "$(dirname ${RAPIDS_CMAKE_FORMAT_FILE})" +wget -O ${RAPIDS_CMAKE_FORMAT_FILE} "${FORMAT_FILE_URL}" # Run pre-commit checks pre-commit run --all-files --show-diff-on-failure diff --git a/ci/checks/black_lists.sh b/ci/checks/black_lists.sh index cf289c120c..df43b17b1b 100755 --- a/ci/checks/black_lists.sh +++ b/ci/checks/black_lists.sh @@ -6,7 +6,7 @@ # PR_TARGET_BRANCH is set by the CI environment -git checkout --quiet $PR_TARGET_BRANCH +git checkout --quiet "$PR_TARGET_BRANCH" # Switch back to tip of PR branch git checkout --quiet current-pr-branch @@ -20,16 +20,16 @@ set +H RETVAL=0 for black_listed in cudaDeviceSynchronize cudaMalloc cudaMallocManaged cudaFree cudaMallocHost cudaHostAlloc cudaFreeHost; do - TMP=`git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" $PR_TARGET_BRANCH | grep '^+' | grep -v '^+++' | grep "$black_listed"` + TMP=$(git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" "$PR_TARGET_BRANCH" | grep '^+' | grep -v '^+++' | grep "$black_listed") if [ "$TMP" != "" ]; then - for filename in `git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$black_listed" $PR_TARGET_BRANCH`; do + for filename in $(git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$black_listed" "$PR_TARGET_BRANCH"); do basefilename=$(basename -- "$filename") filext="${basefilename##*.}" if [ "$filext" != "md" ] && [ "$filext" != "sh" ]; then - TMP2=`git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" $PR_TARGET_BRANCH -- $filename | grep '^+' | grep -v '^+++' | grep "$black_listed" | grep -vE "^\+[[:space:]]*/{2,}.*$black_listed"` + TMP2=$(git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" "$PR_TARGET_BRANCH" -- "$filename" | grep '^+' | grep -v '^+++' | grep "$black_listed" | grep -vE "^\+[[:space:]]*/{2,}.*$black_listed") if [ "$TMP2" != "" ]; then echo "=== ERROR: black listed function call $black_listed added to $filename ===" - git --no-pager diff --ignore-submodules -w --minimal -S"$black_listed" $PR_TARGET_BRANCH -- $filename + git --no-pager diff --ignore-submodules -w --minimal -S"$black_listed" "$PR_TARGET_BRANCH" -- "$filename" echo "=== END ERROR ===" RETVAL=1 fi @@ -39,17 +39,17 @@ for black_listed in cudaDeviceSynchronize cudaMalloc cudaMallocManaged cudaFree done for cond_black_listed in cudaMemcpy cudaMemset; do - TMP=`git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$cond_black_listed" $PR_TARGET_BRANCH | grep '^+' | grep -v '^+++' | grep -P "$cond_black_listed(?!Async)"` + TMP=$(git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$cond_black_listed" "$PR_TARGET_BRANCH" | grep '^+' | grep -v '^+++' | grep -P "$cond_black_listed(?!Async)") if [ "$TMP" != "" ]; then - for filename in `git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$cond_black_listed" $PR_TARGET_BRANCH`; do + for filename in $(git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$cond_black_listed" "$PR_TARGET_BRANCH"); do basefilename=$(basename -- "$filename") filext="${basefilename##*.}" if [ "$filext" != "md" ] && [ "$filext" != "sh" ]; then - TMP2=`git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$cond_black_listed" $PR_TARGET_BRANCH -- $filename | grep '^+' | grep -v '^+++' | grep -P "$cond_black_listed(?!Async)" | grep -vE "^\+[[:space:]]*/{2,}.*$cond_black_listed"` + TMP2=$(git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$cond_black_listed" "$PR_TARGET_BRANCH" -- "$filename" | grep '^+' | grep -v '^+++' | grep -P "$cond_black_listed(?!Async)" | grep -vE "^\+[[:space:]]*/{2,}.*$cond_black_listed") if [ "$TMP2" != "" ]; then echo "=== ERROR: black listed function call $cond_black_listed added to $filename ===" - git --no-pager diff --ignore-submodules -w --minimal -S"$cond_black_listed" $PR_TARGET_BRANCH -- $filename + git --no-pager diff --ignore-submodules -w --minimal -S"$cond_black_listed" "$PR_TARGET_BRANCH" -- "$filename" echo "=== END ERROR ===" RETVAL=1 fi diff --git a/ci/release/update-version.sh b/ci/release/update-version.sh index 1ab9157b89..244f66e99a 100755 --- a/ci/release/update-version.sh +++ b/ci/release/update-version.sh @@ -13,16 +13,15 @@ NEXT_FULL_TAG=$1 # Get current version CURRENT_TAG=$(git tag --merged HEAD | grep -xE '^v.*' | sort --version-sort | tail -n 1 | tr -d 'v') -CURRENT_MAJOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[1]}') -CURRENT_MINOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[2]}') -CURRENT_PATCH=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[3]}') +CURRENT_MAJOR=$(echo "$CURRENT_TAG" | awk '{split($0, a, "."); print a[1]}') +CURRENT_MINOR=$(echo "$CURRENT_TAG" | awk '{split($0, a, "."); print a[2]}') CURRENT_SHORT_TAG=${CURRENT_MAJOR}.${CURRENT_MINOR} # Get . for next version -NEXT_MAJOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[1]}') -NEXT_MINOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[2]}') +NEXT_MAJOR=$(echo "$NEXT_FULL_TAG" | awk '{split($0, a, "."); print a[1]}') +NEXT_MINOR=$(echo "$NEXT_FULL_TAG" | awk '{split($0, a, "."); print a[2]}') NEXT_SHORT_TAG=${NEXT_MAJOR}.${NEXT_MINOR} -NEXT_UCXX_SHORT_TAG="$(curl -sL https://version.gpuci.io/rapids/${NEXT_SHORT_TAG})" +NEXT_UCXX_SHORT_TAG="$(curl -sL https://version.gpuci.io/rapids/"${NEXT_SHORT_TAG}")" # Need to distutils-normalize the original version NEXT_SHORT_TAG_PEP440=$(python -c "from packaging.version import Version; print(Version('${NEXT_SHORT_TAG}'))") @@ -32,7 +31,7 @@ echo "Preparing release $CURRENT_TAG => $NEXT_FULL_TAG" # Inplace sed replace; workaround for Linux and Mac function sed_runner() { - sed -i.bak ''"$1"'' $2 && rm -f ${2}.bak + sed -i.bak ''"$1"'' "$2" && rm -f "${2}".bak } sed_runner 's/'"find_and_configure_ucxx(VERSION .*"'/'"find_and_configure_ucxx(VERSION ${NEXT_UCXX_SHORT_TAG_PEP440}"'/g' python/raft-dask/cmake/thirdparty/get_ucxx.cmake diff --git a/ci/test_cpp.sh b/ci/test_cpp.sh index 9d0edc6b21..64400858ec 100755 --- a/ci/test_cpp.sh +++ b/ci/test_cpp.sh @@ -43,4 +43,4 @@ export GTEST_OUTPUT=xml:${RAPIDS_TESTS_DIR}/ ./ci/run_ctests.sh -j8 && EXITCODE=$? || EXITCODE=$?; rapids-logger "Test script exiting with value: $EXITCODE" -exit ${EXITCODE} +exit "${EXITCODE}" diff --git a/ci/test_wheel_pylibraft.sh b/ci/test_wheel_pylibraft.sh index 0321e41bfb..aba0614767 100755 --- a/ci/test_wheel_pylibraft.sh +++ b/ci/test_wheel_pylibraft.sh @@ -4,7 +4,7 @@ set -euo pipefail mkdir -p ./dist -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" +RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")" RAPIDS_PY_WHEEL_NAME="libraft_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 cpp ./local-libraft-dep RAPIDS_PY_WHEEL_NAME="pylibraft_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 python ./dist diff --git a/ci/test_wheel_raft_dask.sh b/ci/test_wheel_raft_dask.sh index da3b40b353..e38b278d05 100755 --- a/ci/test_wheel_raft_dask.sh +++ b/ci/test_wheel_raft_dask.sh @@ -4,7 +4,7 @@ set -euo pipefail mkdir -p ./dist -RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})" +RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")" RAPIDS_PY_WHEEL_NAME="libraft_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 cpp ./local-libraft-dep RAPIDS_PY_WHEEL_NAME="pylibraft_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 python ./local-pylibraft-dep RAPIDS_PY_WHEEL_NAME="raft_dask_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 python ./dist @@ -13,7 +13,7 @@ RAPIDS_PY_WHEEL_NAME="raft_dask_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels rapids-pip-retry install -v \ ./local-libraft-dep/libraft*.whl \ ./local-pylibraft-dep/pylibraft*.whl \ - "$(echo ./dist/raft_dask_${RAPIDS_PY_CUDA_SUFFIX}*.whl)[test]" + "$(echo ./dist/raft_dask_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test]" test_dir="python/raft-dask/raft_dask/tests" diff --git a/ci/validate_wheel.sh b/ci/validate_wheel.sh index ec3867aa30..5c2c5f2cf6 100755 --- a/ci/validate_wheel.sh +++ b/ci/validate_wheel.sh @@ -5,9 +5,7 @@ set -euo pipefail package_dir=$1 wheel_dir_relative_path=$2 -package_name=$3 -RAPIDS_CUDA_MAJOR="${RAPIDS_CUDA_VERSION%%.*}" cd "${package_dir}" @@ -15,10 +13,10 @@ rapids-logger "validate packages with 'pydistcheck'" pydistcheck \ --inspect \ - "$(echo ${wheel_dir_relative_path}/*.whl)" + "$(echo "${wheel_dir_relative_path}"/*.whl)" rapids-logger "validate packages with 'twine'" twine check \ --strict \ - "$(echo ${wheel_dir_relative_path}/*.whl)" + "$(echo "${wheel_dir_relative_path}"/*.whl)"