Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add shellcheck to pre-commit and fix warnings #2575

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Add shellcheck to pre-commit and fix warnings #2575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbf7a30
Add `shellcheck` to pre-commit and fix warnings
gforsyth Feb 6, 2025
5aca68a
remove unused package_name variable and references
gforsyth Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 4 additions & 2 deletions ci/build_docs.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion ci/build_python.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions ci/build_wheel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
4 changes: 1 addition & 3 deletions ci/build_wheel_libraft.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions ci/build_wheel_pylibraft.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
4 changes: 2 additions & 2 deletions ci/build_wheel_raft_dask.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
4 changes: 2 additions & 2 deletions ci/check_style.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 9 additions & 9 deletions ci/checks/black_lists.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
13 changes: 6 additions & 7 deletions ci/release/update-version.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <major>.<minor> 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}'))")
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ci/test_cpp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
2 changes: 1 addition & 1 deletion ci/test_wheel_pylibraft.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions ci/test_wheel_raft_dask.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"

Expand Down
6 changes: 2 additions & 4 deletions ci/validate_wheel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,18 @@ set -euo pipefail

package_dir=$1
wheel_dir_relative_path=$2
package_name=$3

RAPIDS_CUDA_MAJOR="${RAPIDS_CUDA_VERSION%%.*}"

cd "${package_dir}"

rapids-logger "validate packages with 'pydistcheck'"

pydistcheck \
--inspect \
"$(echo ${wheel_dir_relative_path}/*.whl)"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously package_name was unused in this script

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is why https://github.com/rapidsai/raft/actions/runs/13188022880/job/36815130064?pr=2575 is failing like this:

Usage: pydistcheck [OPTIONS] [FILEPATHS]...
Try 'pydistcheck --help' for help.

Error: Invalid value for '[FILEPATHS]...': Path 'final_dist/raft-dask*.whl' does not exist.

package_name here is raft-dask (with a -), but the wheel filename begins with raft_dask (with a _). That's intentional behavior.

I think a better fix here would be to just remove package_name from the script (and the corresponding calls to it).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, stripped out package_name. thanks for tracking that down, @jameslamb !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No prob! It's my fault that package_name was left behind there unused in anyway 😅

"$(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)"
Loading