Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
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
40 changes: 40 additions & 0 deletions .github/workflows/shell-check.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
#
# Licensed 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.

# A workflow to check copyright/license header
name: shell check

on:
pull_request:
types: [opened, synchronize, reopened]

jobs:
shell-check:
runs-on: ubuntu-latest
if: "!contains(github.event.pull_request.title, '[bot]')"
steps:
- name: Get checkout depth
run: |
echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 10 ))" >> $GITHUB_ENV

- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: ${{ env.PR_FETCH_DEPTH }}

- name: shell-check
uses: YanxuanLiu/spark-rapids-common/shell-check@shellcheck
Copy link
Collaborator

Choose a reason for hiding this comment

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

use nvidia repo instead of the personal one

with:
included_file_patterns: |
*.sh
145 changes: 145 additions & 0 deletions shell-check/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# Copyright (c) 2025, NVIDIA CORPORATION.
#
# Licensed 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.

name: 'Shell Script Syntax Check'
description: 'check shell script syntax'
inputs:
included_file_patterns:
description: "Comma-separated list of file patterns to include (e.g., '*.sh,*.bash')"
required: true
type: string
excluded_file_patterns:
description: "Comma-separated list of file patterns to exclude (e.g., 'src/**')"
required: false
type: string
default: ""

runs:
using: "composite"
steps:
- name: Install shellcheck
shell: bash
run: |
sudo apt-get update
sudo apt-get install -y shellcheck
env:
DEBIAN_FRONTEND: noninteractive

- name: Check shell scripts
shell: bash
run: |
# Initialize variables
error_log="shell_errors.log"
error_count=0
failed_files=()

# Clean up any previous error log
rm -f "$error_log"

# Format file patterns
IFS="," read -r -a INCLUDE_PATTERNS <<< "$(echo "${{ inputs.included_file_patterns }}" | tr -d ' ' | tr -d '\n')"
IFS="," read -r -a EXCLUDE_PATTERNS <<< "$(echo "${{ inputs.excluded_file_patterns }}" | tr -d ' ' | tr -d '\n')"
echo "Included file patterns: ${INCLUDE_PATTERNS[@]}"
echo "Excluded file patterns: ${EXCLUDE_PATTERNS[@]}"

# Get changed files
BASE_REF=$(git --no-pager log --oneline -1 | awk '{ print $NF }')
echo "Base REF: ${BASE_REF}"
FILES=$(git diff --name-only --diff-filter=AM ${BASE_REF} HEAD) || (echo "Your base commit ID is too old, please try upmerge first." && exit 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please help explain why there will be an issue of 'base commit ID is too old',

This error is a little odd, so long as the PR can be submitted, there should not be such issue?

Another concern, we may only D(elete) files in a PR, but not A(dd) or M(odify) anything, then we should also not error out here, right?

RENAME_FILES=$(git diff --name-status ${BASE_REF} HEAD | grep "^R" | grep -v "R100" | awk '{print $3}' || echo "")
echo "${RENAME_FILES[@]}"
FILES=($FILES $RENAME_FILES)
echo "Files to be detected: ${FILES[@]}"

set +e

# Check shell syntax
for FILE in "${FILES[@]}"; do
# Skip directories
[ -d "$FILE" ] && continue
# Check if file matches include patterns
included=false
for pattern in "${INCLUDE_PATTERNS[@]}"; do
if [[ $FILE == $pattern ]]; then
included=true
break
fi
done
excluded=false
for pattern in "${EXCLUDE_PATTERNS[@]}"; do
if [[ $FILE == $pattern ]]; then
excluded=true
break
fi
done

if $included && ! $excluded; then
echo "🔍 Checking: $FILE"

# Create a temporary file for this file's errors
tmp_error=$(mktemp)

# Check if file exists
# Run bash -n syntax check
bash -n "$FILE" > >(tee -a "$tmp_error") 2>&1 || {
echo "❌ bash -n failed for $FILE" | tee -a "$tmp_error"
((error_count++))
}

# Run shellcheck
shellcheck --color=always "$FILE" > >(tee -a "$tmp_error") 2>&1 || {
echo "❌ shellcheck failed for $FILE" | tee -a "$tmp_error"
((error_count++))
}

# If errors were found, add to main log
if [ -s "$tmp_error" ]; then
failed_files+=("$FILE")
echo -e "\n=== Errors in $FILE ===" >> "$error_log"
cat "$tmp_error" >> "$error_log"
echo -e "\n" >> "$error_log"
fi

rm -f "$tmp_error"
fi
done

# Final report
if [ -s "$error_log" ]; then
echo -e "\n❌ Found $error_count errors in ${#failed_files[@]} files:"
printf ' - %s\n' "${failed_files[@]}"

echo "::group::Detailed Error Report"
cat "$error_log"
echo "::endgroup::"

# Create GitHub annotations for each error
while IFS= read -r line; do
if [[ "$line" == *:*:*:* ]]; then
# Extract filename, line number, and message
file_path=$(echo "$line" | cut -d: -f1)
line_num=$(echo "$line" | cut -d: -f2)
message=$(echo "$line" | cut -d: -f4- | sed 's/^ *//')

echo "::error file=$file_path,line=$line_num::$message"
fi
done < "$error_log"

echo "::error::Found $error_count shell script errors"
exit 1
else
echo "✅ No shell script errors found"
fi
env:
SHELLCHECK_OPTS: "-e SC1090 -e SC1091" # Ignore source not found errors