Skip to content

Commit cc68aee

Browse files
authored
Add shellcheck to pre-commit and fix warnings (#6246)
`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: - Dante Gama Dessavre (https://github.com/dantegd) - James Lamb (https://github.com/jameslamb) URL: #6246
1 parent 3a8ea8c commit cc68aee

17 files changed

+70
-52
lines changed

.pre-commit-config.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ repos:
8787
hooks:
8888
- id: rapids-dependency-file-generator
8989
args: ["--clean"]
90+
- repo: https://github.com/shellcheck-py/shellcheck-py
91+
rev: v0.10.0.1
92+
hooks:
93+
- id: shellcheck
94+
args: ["--severity=warning"]
95+
files: ^ci/
9096

9197
default_language_version:
9298
python: python3

build.sh

+13-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ ARGS=$*
1616

1717
# NOTE: ensure all dir changes are relative to the location of this
1818
# script, and that this script resides in the repo dir!
19-
REPODIR=$(cd $(dirname $0); pwd)
19+
REPODIR=$(cd "$(dirname $0)"; pwd)
2020

2121
VALIDTARGETS="clean libcuml cuml cuml-cpu cpp-mgtests prims bench prims-bench cppdocs pydocs"
2222
VALIDFLAGS="-v -g -n --allgpuarch --singlegpu --nolibcumltest --nvtx --show_depr_warn --codecov --ccache --configure-only -h --help "
@@ -96,11 +96,21 @@ export CMAKE_GENERATOR="${CMAKE_GENERATOR:=Ninja}"
9696
CUML_EXTRA_CMAKE_ARGS=${CUML_EXTRA_CMAKE_ARGS:=""}
9797

9898
function hasArg {
99-
(( ${NUMARGS} != 0 )) && (echo " ${ARGS} " | grep -q " $1 ")
99+
(( NUMARGS != 0 )) && (echo " ${ARGS} " | grep -q " $1 ")
100100
}
101101

102+
# Check for valid usage
103+
if (( NUMARGS != 0 )); then
104+
for a in ${ARGS}; do
105+
if ! (echo " ${VALIDARGS} " | grep -q " ${a} "); then
106+
echo "Invalid option or formatting, check --help: ${a}"
107+
exit 1
108+
fi
109+
done
110+
fi
111+
102112
function completeBuild {
103-
(( ${NUMARGS} == 0 )) && return
113+
(( NUMARGS == 0 )) && return
104114
for a in ${ARGS}; do
105115
if (echo " ${VALIDTARGETS} " | grep -q " ${a} "); then
106116
false; return

ci/build_docs.sh

+4-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ PYTHON_CHANNEL=$(rapids-download-conda-from-s3 python)
99
rapids-logger "Create test conda environment"
1010
. /opt/conda/etc/profile.d/conda.sh
1111

12-
export RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)"
12+
RAPIDS_VERSION_MAJOR_MINOR="$(rapids-version-major-minor)"
13+
export RAPIDS_VERSION_MAJOR_MINOR
1314

1415
rapids-dependency-file-generator \
1516
--output conda \
@@ -24,7 +25,8 @@ conda activate docs
2425

2526
rapids-print-env
2627

27-
export RAPIDS_DOCS_DIR="$(mktemp -d)"
28+
RAPIDS_DOCS_DIR="$(mktemp -d)"
29+
export RAPIDS_DOCS_DIR
2830

2931
rapids-logger "Build CPP docs"
3032
pushd cpp

ci/build_wheel_cuml.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ set -euo pipefail
66
package_name="cuml"
77
package_dir="python/cuml"
88

9-
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"
9+
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")"
1010

1111
# Download the libcuml wheel built in the previous step and make it
1212
# available for pip to find.

ci/build_wheel_libcuml.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ set -euo pipefail
66
package_name="libcuml"
77
package_dir="python/libcuml"
88

9-
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"
9+
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")"
1010

1111
rapids-logger "Generating build requirements"
1212

ci/checks/black_lists.sh

+9-10
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
# PR_TARGET_BRANCH is set by the CI environment
88

9-
git checkout --quiet $PR_TARGET_BRANCH
9+
git checkout --quiet "$PR_TARGET_BRANCH"
1010

1111
# Switch back to tip of PR branch
1212
git checkout --quiet current-pr-branch
@@ -20,16 +20,16 @@ set +H
2020
RETVAL=0
2121

2222
for black_listed in cudaDeviceSynchronize cudaMalloc cudaMallocManaged cudaFree cudaMallocHost cudaHostAlloc cudaFreeHost; do
23-
TMP=`git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" $PR_TARGET_BRANCH | grep '^+' | grep -v '^+++' | grep "$black_listed"`
23+
TMP=$(git --no-pager diff --ignore-submodules -w --minimal -U0 -S"$black_listed" "$PR_TARGET_BRANCH" | grep '^+' | grep -v '^+++' | grep "$black_listed")
2424
if [ "$TMP" != "" ]; then
25-
for filename in `git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$black_listed" $PR_TARGET_BRANCH`; do
25+
for filename in $(git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$black_listed" "$PR_TARGET_BRANCH"); do
2626
basefilename=$(basename -- "$filename")
2727
filext="${basefilename##*.}"
2828
if [ "$filext" != "md" ] && [ "$filext" != "sh" ]; then
29-
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"`
29+
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")
3030
if [ "$TMP2" != "" ]; then
3131
echo "=== ERROR: black listed function call $black_listed added to $filename ==="
32-
git --no-pager diff --ignore-submodules -w --minimal -S"$black_listed" $PR_TARGET_BRANCH -- $filename
32+
git --no-pager diff --ignore-submodules -w --minimal -S"$black_listed" "$PR_TARGET_BRANCH" -- "$filename"
3333
echo "=== END ERROR ==="
3434
RETVAL=1
3535
fi
@@ -39,17 +39,16 @@ for black_listed in cudaDeviceSynchronize cudaMalloc cudaMallocManaged cudaFree
3939
done
4040

4141
for cond_black_listed in cudaMemcpy cudaMemset; do
42-
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)"`
43-
42+
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)")
4443
if [ "$TMP" != "" ]; then
45-
for filename in `git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$cond_black_listed" $PR_TARGET_BRANCH`; do
44+
for filename in $(git --no-pager diff --ignore-submodules -w --minimal --name-only -S"$cond_black_listed" "$PR_TARGET_BRANCH"); do
4645
basefilename=$(basename -- "$filename")
4746
filext="${basefilename##*.}"
4847
if [ "$filext" != "md" ] && [ "$filext" != "sh" ]; then
49-
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"`
48+
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")
5049
if [ "$TMP2" != "" ]; then
5150
echo "=== ERROR: black listed function call $cond_black_listed added to $filename ==="
52-
git --no-pager diff --ignore-submodules -w --minimal -S"$cond_black_listed" $PR_TARGET_BRANCH -- $filename
51+
git --no-pager diff --ignore-submodules -w --minimal -S"$cond_black_listed" "$PR_TARGET_BRANCH" -- "$filename"
5352
echo "=== END ERROR ==="
5453
RETVAL=1
5554
fi

ci/release/update-version.sh

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
# Copyright (c) 2019-2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2019-2025, NVIDIA CORPORATION.
33
########################
44
# cuML Version Updater #
55
########################
@@ -13,14 +13,13 @@ NEXT_FULL_TAG=$1
1313

1414
# Get current version
1515
CURRENT_TAG=$(git tag --merged HEAD | grep -xE '^v.*' | sort --version-sort | tail -n 1 | tr -d 'v')
16-
CURRENT_MAJOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[1]}')
17-
CURRENT_MINOR=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[2]}')
18-
CURRENT_PATCH=$(echo $CURRENT_TAG | awk '{split($0, a, "."); print a[3]}')
16+
CURRENT_MAJOR=$(echo "$CURRENT_TAG" | awk '{split($0, a, "."); print a[1]}')
17+
CURRENT_MINOR=$(echo "$CURRENT_TAG" | awk '{split($0, a, "."); print a[2]}')
1918
CURRENT_SHORT_TAG=${CURRENT_MAJOR}.${CURRENT_MINOR}
2019

2120
# Get <major>.<minor> for next version
22-
NEXT_MAJOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[1]}')
23-
NEXT_MINOR=$(echo $NEXT_FULL_TAG | awk '{split($0, a, "."); print a[2]}')
21+
NEXT_MAJOR=$(echo "$NEXT_FULL_TAG" | awk '{split($0, a, "."); print a[1]}')
22+
NEXT_MINOR=$(echo "$NEXT_FULL_TAG" | awk '{split($0, a, "."); print a[2]}')
2423
NEXT_SHORT_TAG=${NEXT_MAJOR}.${NEXT_MINOR}
2524

2625
# Need to distutils-normalize the original version
@@ -30,7 +29,7 @@ echo "Preparing release $CURRENT_TAG => $NEXT_FULL_TAG"
3029

3130
# Inplace sed replace; workaround for Linux and Mac
3231
function sed_runner() {
33-
sed -i.bak ''"$1"'' $2 && rm -f ${2}.bak
32+
sed -i.bak ''"$1"'' "$2" && rm -f "${2}".bak
3433
}
3534

3635

ci/run_cuml_dask_pytests.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
2-
# Copyright (c) 2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2024-2025, NVIDIA CORPORATION.
33

44
# Support invoking run_cuml_dask_pytests.sh outside the script directory
5-
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests/dask
5+
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests/dask || exit 1
66

77
python -m pytest --cache-clear "$@" .

ci/run_cuml_integration_pytests.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
2-
# Copyright (c) 2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2024-2025, NVIDIA CORPORATION.
33

44
# Support invoking run_cuml_singlegpu_pytests.sh outside the script directory
5-
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests
5+
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests || exit 1
66

77
python -m pytest -p cudf.pandas --cache-clear --ignore=dask -m "not memleak" "$@" --quick_run .

ci/run_cuml_singlegpu_accel_pytests.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
# Copyright (c) 2024-2025, NVIDIA CORPORATION.
33

44
# Support invoking run_cuml_singlegpu_pytests.sh outside the script directory
5-
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests/accel
5+
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests/accel || exit 1
66

77
python -m pytest -p cuml.accel --cache-clear "$@" .
+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
2-
# Copyright (c) 2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2024-2025, NVIDIA CORPORATION.
33

44
# Support invoking run_cuml_singlegpu_pytests.sh outside the script directory
5-
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests
5+
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests || exit 1
66

77
python -m pytest --cache-clear --ignore=dask -m "memleak" "$@" .

ci/run_cuml_singlegpu_pytests.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/bin/bash
2-
# Copyright (c) 2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2024-2025, NVIDIA CORPORATION.
33

44
# Support invoking run_cuml_singlegpu_pytests.sh outside the script directory
5-
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests
5+
cd "$(dirname "$(realpath "${BASH_SOURCE[0]}")")"/../python/cuml/cuml/tests || exit 1
66

77
python -m pytest --cache-clear --ignore=dask -m "not memleak" "$@" .

ci/test_cpp.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
# Copyright (c) 2022-2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2022-2025, NVIDIA CORPORATION.
33

44
set -euo pipefail
55

@@ -39,4 +39,4 @@ export GTEST_OUTPUT=xml:${RAPIDS_TESTS_DIR}/
3939
./ci/run_ctests.sh -j9 && EXITCODE=$? || EXITCODE=$?;
4040

4141
rapids-logger "Test script exiting with value: $EXITCODE"
42-
exit ${EXITCODE}
42+
exit "${EXITCODE}"

ci/test_notebooks.sh

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
# Copyright (c) 2020-2024, NVIDIA CORPORATION.
2+
# Copyright (c) 2020-2025, NVIDIA CORPORATION.
33
set -euo pipefail
44

55
. /opt/conda/etc/profile.d/conda.sh
@@ -40,17 +40,19 @@ SKIPNBS="cuml_benchmarks.ipynb"
4040
NBTEST="$(realpath "$(dirname "$0")/utils/nbtest.sh")"
4141

4242
cd notebooks
43+
44+
# shellcheck disable=SC2044
4345
for nb in $(find . -name "*.ipynb"); do
4446
nbBasename=$(basename "${nb}")
4547
# Skip all NBs that use dask (in the code or even in their name)
46-
if ((echo "${nb}" | grep -qi dask) || \
47-
(grep -q dask "${nb}")); then
48+
if (echo "${nb}" | grep -qi dask) || \
49+
(grep -q dask "${nb}"); then
4850
echo "--------------------------------------------------------------------------------"
4951
echo "SKIPPING: ${nb} (suspected Dask usage, not currently automatable)"
5052
echo "--------------------------------------------------------------------------------"
5153
elif (echo " ${SKIPNBS} " | grep -q " ${nbBasename} "); then
5254
echo "--------------------------------------------------------------------------------"
53-
echo "SKIPPING: "${nb}" (listed in skip list)"
55+
echo "SKIPPING: ${nb} (listed in skip list)"
5456
echo "--------------------------------------------------------------------------------"
5557
else
5658
nvidia-smi

ci/test_wheel.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
set -euo pipefail
55

66
mkdir -p ./dist
7-
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen ${RAPIDS_CUDA_VERSION})"
7+
RAPIDS_PY_CUDA_SUFFIX="$(rapids-wheel-ctk-name-gen "${RAPIDS_CUDA_VERSION}")"
88
RAPIDS_PY_WHEEL_NAME="cuml_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 python ./dist
99
RAPIDS_PY_WHEEL_NAME="libcuml_${RAPIDS_PY_CUDA_SUFFIX}" rapids-download-wheels-from-s3 cpp ./dist
1010

ci/utils/nbtest.sh

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
# Copyright (c) 2020-2021, NVIDIA CORPORATION.
2+
# Copyright (c) 2020-2025, NVIDIA CORPORATION.
33

44
MAGIC_OVERRIDE_CODE="
55
def my_run_line_magic(*args, **kwargs):
@@ -24,30 +24,30 @@ get_ipython().run_cell_magic=my_run_cell_magic
2424
NO_COLORS=--colors=NoColor
2525
EXITCODE=0
2626
NBTMPDIR="$WORKSPACE/tmp"
27-
mkdir -p ${NBTMPDIR}
27+
mkdir -p "${NBTMPDIR}"
2828

29-
for nb in $*; do
30-
NBFILENAME=$1
29+
for nb in "$@"; do
30+
NBFILENAME=$nb
3131
NBNAME=${NBFILENAME%.*}
3232
NBNAME=${NBNAME##*/}
3333
NBTESTSCRIPT=${NBTMPDIR}/${NBNAME}-test.py
3434
shift
3535

3636
echo --------------------------------------------------------------------------------
37-
echo STARTING: ${NBNAME}
37+
echo STARTING: "${NBNAME}"
3838
echo --------------------------------------------------------------------------------
39-
jupyter nbconvert --to script ${NBFILENAME} --output ${NBTMPDIR}/${NBNAME}-test
40-
echo "${MAGIC_OVERRIDE_CODE}" > ${NBTMPDIR}/tmpfile
41-
cat ${NBTESTSCRIPT} >> ${NBTMPDIR}/tmpfile
42-
mv ${NBTMPDIR}/tmpfile ${NBTESTSCRIPT}
39+
jupyter nbconvert --to script "${NBFILENAME}" --output "${NBTMPDIR}"/"${NBNAME}"-test
40+
echo "${MAGIC_OVERRIDE_CODE}" > "${NBTMPDIR}"/tmpfile
41+
cat "${NBTESTSCRIPT}" >> "${NBTMPDIR}"/tmpfile
42+
mv "${NBTMPDIR}"/tmpfile "${NBTESTSCRIPT}"
4343

4444
echo "Running \"ipython ${NO_COLORS} ${NBTESTSCRIPT}\" on $(date)"
4545
echo
4646
time bash -c "ipython ${NO_COLORS} ${NBTESTSCRIPT}; EC=\$?; echo -------------------------------------------------------------------------------- ; echo DONE: ${NBNAME}; exit \$EC"
4747
NBEXITCODE=$?
4848
echo EXIT CODE: ${NBEXITCODE}
4949
echo
50-
EXITCODE=$((EXITCODE | ${NBEXITCODE}))
50+
EXITCODE=$((EXITCODE | "${NBEXITCODE}"))
5151
done
5252

5353
exit ${EXITCODE}

ci/validate_wheel.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ rapids-logger "validate packages with 'pydistcheck'"
1212

1313
pydistcheck \
1414
--inspect \
15-
"$(echo ${wheel_dir_relative_path}/*.whl)"
15+
"$(echo "${wheel_dir_relative_path}"/*.whl)"
1616

1717
rapids-logger "validate packages with 'twine'"
1818

1919
twine check \
2020
--strict \
21-
"$(echo ${wheel_dir_relative_path}/*.whl)"
21+
"$(echo "${wheel_dir_relative_path}"/*.whl)"

0 commit comments

Comments
 (0)