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.

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

update verify shellcheck to reduce the time to less than 15min #1863

Merged
merged 2 commits into from Mar 19, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 39 additions & 3 deletions hack/verify-shellcheck.sh
Expand Up @@ -17,18 +17,54 @@
# allow overriding docker cli, which should work fine for this script
DOCKER="${DOCKER:-docker}"


SHELLCHECK_VERSION="0.9.0"
SHELLCHECK_IMAGE="docker.io/koalaman/shellcheck-alpine:v0.9.0@sha256:e19ed93c22423970d56568e171b4512c9244fc75dd9114045016b4a0073ac4b7"

# common arguments we'll pass to shellcheck
SHELLCHECK_OPTIONS=(
# allow following sourced files that are not specified in the command,
# we need this because we specify one file at a time in order to trivially
# detect which files are failing
"--external-sources"
# include our disabled lints
"--exclude=${SHELLCHECK_DISABLED}"
# set colorized output
"--color=${SHELLCHECK_COLORIZED_OUTPUT}"
)

# Currently disabled these errors will take care of them later
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we should move this comment to line 23.

DISABLED_ERRORS="SC2002,SC3028,SC3054,SC3014,SC3040,SC3046,SC3030,SC3010,SC3037,SC3045,SC3006,SC3018,SC3016,SC3011,SC3044,SC3043,SC3060,SC3024,SC1091,SC2066,SC2086,SC2034,SC1083,SC1009,SC1073,SC1072,SC2155,SC2046"
SHELLCHECK_DISABLED="SC2002,SC3028,SC3054,SC3014,SC3040,SC3046,SC3030,SC3010,SC3037,SC3045,SC3006,SC3018,SC3016,SC3011,SC3044,SC3043,SC3060,SC3024,SC1091,SC2066,SC2086,SC2034,SC1083,SC1009,SC1073,SC1072,SC2155,SC2046"
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this variable be defined before line 25?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


scripts_to_check=("$@")
if [[ "$#" == 0 ]]; then
# Find all shell scripts excluding:
# - Anything git-ignored - No need to lint untracked files.
# - ./_* - No need to lint output directories.
# - ./.git/* - Ignore anything in the git object store.
# - ./vendor* - Vendored code should be fixed upstream instead.
# - ./third_party/*, but re-include ./third_party/forked/* - only code we
# forked should be linted and fixed.
while IFS=$'\n' read -r script;
do git check-ignore -q "$script" || scripts_to_check+=("$script");
done < <(find . -name "*.sh" \
-not \( \
-path ./_\* -o \
-path ./.git\* -o \
-path ./vendor\* -o \
\( -path ./third_party\* -a -not -path ./third_party/forked\* \) \
\))
fi

# Download shellcheck-alpine from Docker Hub
echo "Downloading ShellCheck Docker image..."
"${DOCKER}" pull "${SHELLCHECK_IMAGE}"

# Run ShellCheck on all shell script files, excluding those in the 'vendor' directory
echo "Running ShellCheck..."
"${DOCKER}" run --rm -v "$(pwd):$(pwd)" -w "$(pwd)" "${SHELLCHECK_IMAGE}" \
find .. -type f -name '*.sh' -exec shellcheck --exclude="$DISABLED_ERRORS" {} +
"${DOCKER}" run \
--rm -v "$(pwd)" -w "$(pwd)" \
"${SHELLCHECK_IMAGE}" \
shellcheck "${SHELLCHECK_OPTIONS[@]}" "${scripts_to_check[@]}" >&2 || res=$?

echo "Shellcheck ran successfully"