Skip to content

Commit 77fab53

Browse files
committed
Follow Bash best practices in check.sh
Changes: * Use `readarray` instead of unquoted command substitution to remove "clippy" from `DEFAULT_COMMANDS`. * Use `-n "$apiFeature"` instead of array-length check on a scalar variable. * Guard `cd` calls in `cmd_itest` with `|| return 1`. * Use printf (logf) instead of non-portable `echo -e/-n` flags for colors. * Remove dead-code process substitution in `cmd_itest` that ran grep+exit in a subshell (the exit only left the subshell, never actually aborted anything); the logfile grep afterwards already catches the same errors.
1 parent 47f2695 commit 77fab53

1 file changed

Lines changed: 25 additions & 33 deletions

File tree

check.sh

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,14 @@ END='\033[0m'
6161
################################################################################
6262

6363
# Drop-in replacement for `echo` that outputs to stderr and adds a newline.
64+
# Use `logf` for format strings with color codes (instead of nonportable `echo -e`).
6465
function log() {
6566
echo "$@" >&2
6667
}
68+
function logf() {
69+
# shellcheck disable=SC2059
70+
printf "$@" >&2
71+
}
6772

6873
# Converts a x.x.x version string to a feature string.
6974
# e.g. 4.3.0 -> godot/api-4-3, 4.3.1 -> godot/api-4-3-1
@@ -107,7 +112,7 @@ function findGodot() {
107112

108113
# User-defined GODOT4_BIN (deprecated, fallback to old name).
109114
elif [[ -n "$GODOT4_BIN" ]]; then
110-
log -e "${YELLOW}Warning: \`GODOT4_BIN\` is deprecated, use \`GDRUST_GODOT_BIN\` instead.${END}"
115+
logf "${YELLOW}Warning: \`GODOT4_BIN\` is deprecated, use \`GDRUST_GODOT_BIN\` instead.${END}\n"
111116
log "Using environment variable GODOT4_BIN=$(printf %q "$GODOT4_BIN")"
112117
godotBin="$GODOT4_BIN"
113118

@@ -154,7 +159,7 @@ function cmd_fmt() {
154159
if [[ $(rustup toolchain list) =~ nightly ]]; then
155160
run cargo +nightly fmt --all -- --check
156161
else
157-
log -e "${YELLOW}Warning: nightly toolchain not found; stable rustfmt might not pass CI.${END}"
162+
logf "${YELLOW}Warning: nightly toolchain not found; stable rustfmt might not pass CI.${END}\n"
158163
run cargo fmt --all -- --check
159164
fi
160165
}
@@ -191,45 +196,32 @@ function cmd_itest() {
191196
findGodot && \
192197
run cargo build -p itest "${extraCargoArgs[@]}" || return 1
193198

194-
# Logic to abort immediately if Godot outputs certain keywords (would otherwise fail only in CI).
195199
# Keep in sync with: .github/composite/godot-itest/action.yml (steps "Run Godot integration tests" and "Check for memory leaks").
196200

197201
local logFile
198202
logFile=$(mktemp)
199203

200-
cd itest/godot
201-
202-
# Explanation:
203-
# * tee: still output logs while scanning for errors.
204-
# * grep -q: no output, use exit code 0 if found -> thus also &&.
205-
# * pkill: stop Godot execution (since it hangs in headless mode); simple 'head -1' did not work as expected
206-
# since it's not available on Windows, use taskkill in that case.
207-
# * exit: the terminated process would return 143, but this is more explicit and future-proof.
204+
cd itest/godot || return 1
208205
"$godotBin" --headless -- "[${extraArgs[@]}]" 2>&1 \
209-
| tee "$logFile" \
210-
| tee >(grep -E "SCRIPT ERROR:|Can't open dynamic library|Error loading extension" -q && {
211-
printf "\n${RED}Error: Script or dlopen error, abort...${END}\n" >&2;
212-
# Unlike CI; do not kill processes called "godot" on user machine.
213-
exit 2
214-
})
206+
| tee "$logFile"
215207

216208
# PIPESTATUS[0] is Godot's exit code; $? would only give tee's exit code (masking crashes).
217209
local exitCode=${PIPESTATUS[0]}
218210

219211
# Check for unrecoverable errors in log.
220212
if grep -qE "SCRIPT ERROR:|Can't open dynamic library" "$logFile"; then
221-
log -e "\n${RED}Error: Unrecoverable Godot error detected in logs.${END}"
213+
logf "\n${RED}Error: Unrecoverable Godot error detected in logs.${END}\n"
222214
exitCode=2
223215
fi
224216

225217
# Check for memory leaks.
226218
if grep -q "ObjectDB instances leaked at exit" "$logFile"; then
227-
log -e "\n${RED}Error: Memory leak detected.${END}"
219+
logf "\n${RED}Error: Memory leak detected.${END}\n"
228220
exitCode=3
229221
fi
230222

231223
rm -f "$logFile"
232-
cd ../..
224+
cd ../.. || return 1
233225

234226
return $exitCode
235227
}
@@ -327,7 +319,7 @@ while [[ $# -gt 0 ]]; do
327319

328320
# Remove "clippy" from the default commands if the API version is specified
329321
# since it can produce unexpected errors.
330-
DEFAULT_COMMANDS=("${DEFAULT_COMMANDS[@]/clippy}")
322+
readarray -t DEFAULT_COMMANDS < <(printf '%s\n' "${DEFAULT_COMMANDS[@]}" | grep -v '^clippy$')
331323

332324
shift
333325
;;
@@ -359,15 +351,15 @@ done
359351
cmds=("${filtered_commands[@]}")
360352

361353
# Display warning about using clippy if an API version was provided.
362-
if [[ "${#apiFeature[@]}" -ne 0 ]]; then
354+
if [[ -n "$apiFeature" ]]; then
363355
log
364356
# Show different warning depending on if clippy was explicitly requested.
365357
if [[ "$runClippy" -eq 1 ]]; then
366-
log -e "${YELLOW}Warning: Clippy may produce unexpected errors when testing against a specific API version.${END}"
358+
logf "${YELLOW}Warning: Clippy may produce unexpected errors when testing against a specific API version.${END}\n"
367359
else
368-
log -e "${YELLOW}Warning: Clippy is disabled by default when using a specific Godot API version.${END}"
360+
logf "${YELLOW}Warning: Clippy is disabled by default when using a specific Godot API version.${END}\n"
369361
fi
370-
log -e "${YELLOW}For more information, see ${CYAN}https://github.com/godot-rust/gdext/pull/1016#issuecomment-2629002047${END}"
362+
logf "${YELLOW}For more information, see ${CYAN}https://github.com/godot-rust/gdext/pull/1016#issuecomment-2629002047${END}\n"
371363
log
372364
fi
373365

@@ -395,19 +387,19 @@ function compute_elapsed() {
395387
for cmd in "${cmds[@]}"; do
396388
"cmd_${cmd//-/_}" || {
397389
compute_elapsed
398-
log -ne "$RED\n=========================="
399-
log -ne "\ngodot-rust: checks FAILED."
400-
log -ne "\n==========================\n$END"
401-
log -ne "\nTotal duration: $elapsed.\n"
390+
logf "$RED\n=========================="
391+
logf "\ngodot-rust: checks FAILED."
392+
logf "\n==========================\n$END"
393+
logf "\nTotal duration: $elapsed.\n"
402394
exit 1
403395
}
404396
done
405397

406398
compute_elapsed
407-
log -ne "$CYAN\n=============================="
408-
log -ne "\ngodot-rust: checks SUCCESSFUL."
409-
log -ne "\n==============================\n$END"
410-
log -ne "\nTotal duration: $elapsed.\n"
399+
logf "$CYAN\n=============================="
400+
logf "\ngodot-rust: checks SUCCESSFUL."
401+
logf "\n==============================\n$END"
402+
logf "\nTotal duration: $elapsed.\n"
411403

412404
# If invoked with sh instead of bash, pressing Up arrow after executing `sh check.sh` may cause a `[A` to appear.
413405
# See https://unix.stackexchange.com/q/103608.

0 commit comments

Comments
 (0)