Merge pull request #14580 from keymanapp/refactor/common/shellcheck

This commit is contained in:
Eberhard Beilharz 2025-08-27 18:27:54 +02:00 committed by GitHub
commit 64e6fcd467
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 61 additions and 53 deletions

View file

@ -21,19 +21,20 @@
function test_bot_check_pr_body() {
local PRNUM=$1
local prinfo="$2"
local prbody prTestCommand prTestBody
set -o noglob
IFS=$'\n'
local prbody="$(echo "$prinfo" | "${JQ}" -r '.body')"
local prTestCommand="$(echo "$prbody" | grep 'Test-bot:' | cut -d: -f 2 - | cut -d' ' -f 1 -)"
local prTestBody="$(echo "$prbody" | grep -i '# User Testing')"
prbody="$(echo "${prinfo}" | "${JQ}" -r '.body')"
prTestCommand="$(echo "${prbody}" | grep 'Test-bot:' | cut -d: -f 2 - | cut -d' ' -f 1 -)"
prTestBody="$(echo "${prbody}" | grep -i '# User Testing' || true)"
unset IFS
set +o noglob
if ([[ "$prTestCommand" == skip ]] || [[ -z "${prTestCommand// }" ]]) && [[ -z "${prTestBody// }" ]]; then
if { [[ "${prTestCommand}" == skip ]] || [[ -z "${prTestCommand// }" ]]; } && [[ -z "${prTestBody// }" ]]; then
local platform
for platform in "${!build_platforms[@]}"; do
build_platforms[$platform]=build
build_platforms["${platform}"]=build
done
fi
}
@ -67,31 +68,32 @@ function build_bot_check_messages() {
IFS=$'\n'
local buildBotCommands=($(echo "$prcommits" | "${JQ}" -r '.[].commit.message' | grep 'Build-bot:' | cut -c 11- -))
local prCommands=($(echo "$prinfo" | "${JQ}" -r '.body' | tr -d '\r' | grep 'Build-bot:' | cut -c 11- -))
local buildBotCommandArray prCommandArray
buildBotCommandArray=($(echo "$prcommits" | "${JQ}" -r '.[].commit.message' | grep 'Build-bot:' | cut -c 11- -))
prCommandArray=($(echo "$prinfo" | "${JQ}" -r '.body' | tr -d '\r' | grep 'Build-bot:' | cut -c 11- -))
unset IFS
# The PR body Build-bot comment will be read last, which allows it to override
# all previous commands
if [[ ${#prCommands[@]} -gt 0 ]]; then
buildBotCommands+=("${prCommands[@]}")
if [[ ${#prCommandArray[@]} -gt 0 ]]; then
buildBotCommandArray+=("${prCommandArray[@]}")
fi
for buildBotCommand in "${buildBotCommands[@]}"; do
for buildBotCommands in "${buildBotCommandArray[@]}"; do
# Block illegal Build-bot: commands
if [[ ! "$buildBotCommand" =~ ^[a-z_,\ :,]+$ ]]; then
builder_echo warning "WARNING[Build-bot]: ignoring invalid command [2]: '${buildBotCommand}'"
if [[ ! "${buildBotCommands}" =~ ^[a-z_,\ :,]+$ ]]; then
builder_echo warning "WARNING[Build-bot]: ignoring invalid command [2]: '${buildBotCommands}'"
continue
fi
# debug_echo "buildBotCommand:{$buildBotCommand}"
# builder_echo debug "buildBotCommands:{$buildBotCommands}"
# We now know that our command has only a-z, comma, colon, and space, so we
# can parse without risking escaping our bash jail
if [[ ! -z "${buildBotCommand// }" ]]; then
build_bot_update_commands $buildBotCommand
if [[ ! -z "${buildBotCommands// }" ]]; then
build_bot_update_commands ${buildBotCommands}
fi
done
@ -111,7 +113,7 @@ function build_bot_update_commands() {
local command="$*"
local re='^(build|skip|release)( [a-z,]+)?$'
if [[ "$command" =~ $re ]]; then
if [[ "${command}" =~ ${re} ]]; then
# legacy (until aug 2025) format is "level [platform]" (comma format never used)
if [[ $# == 1 ]]; then
level=$1
@ -126,41 +128,41 @@ function build_bot_update_commands() {
unset IFS
fi
if [[ ! $level =~ ^$valid_build_levels$ ]]; then
if [[ ! ${level} =~ ^${valid_build_levels}$ ]]; then
# Just skip this build command
builder_echo warning "WARNING[Build-bot]: ignoring invalid build level '$level' in command '$command'"
builder_echo warning "WARNING[Build-bot]: ignoring invalid build level '${level}' in command '${command}'"
return 0
fi
builder_echo blue "Platforms to be updated from command '$command' are: ${platforms[@]}"
builder_echo blue "Platforms to be updated from command '${command}' are: ${platforms[*]}"
build_bot_verify_platforms platforms
local platform
for platform in "${platforms[@]}"; do
builder_echo "Build-bot: Updating build level for $platform to $level"
build_platforms[$platform]=$level
builder_echo "Build-bot: Updating build level for ${platform} to ${level}"
build_platforms["${platform}"]=${level}
done
else
# modern format is "level[:platform[,platform...]][ level[:platform[,platform...]]...]"
declare -a commands
IFS=' '
read -r -a commands <<< "$command"
read -r -a commands <<< "${command}"
unset IFS
for command in "${commands[@]}"; do
declare -a params
IFS=:
read -r -a params <<< "$command"
read -r -a params <<< "${command}"
level=${params[0]}
if [[ ! $level =~ ^$valid_build_levels$ ]]; then
if [[ ! ${level} =~ ^${valid_build_levels}$ ]]; then
# Just skip this build command
builder_echo warning "WARNING[Build-bot]: ignoring invalid build level '$level' in command '$command'"
builder_echo warning "WARNING[Build-bot]: ignoring invalid build level '${level}' in command '${command}'"
continue
fi
if [[ ${#params[@]} == 1 ]]; then
platforms="${!build_platforms[@]}"
platforms=("${!build_platforms[@]}")
else
# remaining parameters are comma separated platforms
IFS=','
@ -172,9 +174,9 @@ function build_bot_update_commands() {
local platform
for platform in "${platforms[@]}"; do
if [[ "${build_platforms[$platform]+x}" != "${level}" ]]; then
builder_echo "Build-bot: Updating build level for $platform to $level"
build_platforms[$platform]=$level
if [[ "${build_platforms[${platform}]+x}" != "${level}" ]]; then
builder_echo "Build-bot: Updating build level for ${platform} to ${level}"
build_platforms["${platform}"]=${level}
fi
done
done
@ -193,14 +195,14 @@ function build_bot_verify_platforms() {
local platform
for platform in "${input_platforms[@]}"; do
# We'll emit a warning with invalid platforms, then remove them from the array
if [[ ! $platform =~ ^(all|$available_platforms_regex)$ ]]; then
builder_echo warning "WARNING[Build-bot]: ignoring invalid platform '$platform'"
elif [[ $platform == all ]]; then
input_platforms=(${available_platforms[@]})
if [[ ! ${platform} =~ ^(all|${available_platforms_regex})$ ]]; then
builder_echo warning "WARNING[Build-bot]: ignoring invalid platform '${platform}'"
elif [[ ${platform} == all ]]; then
input_platforms=("${available_platforms[@]}")
return
else
if [[ ! "${output_platforms[@]}" =~ [[:\<:]]$platform[[:\>:]] ]]; then
output_platforms+=($platform)
if [[ ! "${output_platforms[@]}" =~ [[:\<:]]${platform}[[:\>:]] ]]; then
output_platforms+=("${platform}")
fi
fi
done

View file

@ -1,4 +1,5 @@
#!/usr/bin/env bash
# shellcheck disable=2154 disable=1091
## START STANDARD BUILD SCRIPT INCLUDE
# adjust relative paths as necessary
@ -25,7 +26,7 @@ test_build_bot_check_messages() {
builder_echo start test_build_bot_check_messages 'START TEST: build_bot_check_messages'
# Mostly real data (only Build-bot commands edited for test)
_do_test_build_bot_check_messages_file 14013 "[windows]=release" "$ALL_BUILD_PLATFORMS_SKIP_EXPECTED"
_do_test_build_bot_check_messages_file 14013 "[windows]=release" "${ALL_BUILD_PLATFORMS_SKIP_EXPECTED}"
# Simplified data for testing Build-bot command sequences only
_do_test_build_bot_check_messages_file 9999 "[windows]=release" '[windows]="skip" [developer]="release"'
@ -42,6 +43,7 @@ test_build_bot_check_messages() {
_do_test_build_bot_check_messages_inline 3 "[windows]=release" '[windows]="release"' '{ "body": "Build-bot: '"'"'echo foo" }' '[{ "commit": { "message": "maint(common): test\n" }}]'
_do_test_build_bot_check_messages_inline 4 "[windows]=release" '[windows]="release"' '{ "body": "Build-bot: echo *" }' '[{ "commit": { "message": "maint(common): test\n" }}]'
_do_test_build_bot_check_messages_inline 5 "[windows]=release" '[windows]="release"' '{ "body": "Build-bot: \\0" }' '[{ "commit": { "message": "maint(common): test\n" }}]'
# shellcheck disable=SC2016
_do_test_build_bot_check_messages_inline 5 "[windows]=release" '[windows]="release"' '{ "body": "Build-bot: `echo escaped`" }' '[{ "commit": { "message": "maint(common): test\n" }}]'
_do_test_build_bot_check_messages_inline 6 "[windows]=release" '[windows]="release"' '{ "body": "Build-bot: ' '[{ "commit": { "message": "maint(common): test\n" }}]'
# incomplete command
@ -62,13 +64,14 @@ _do_test_build_bot_check_messages_file() {
eval "declare -gA build_platforms=($2)"
eval "declare -A expected_build_platforms=($3)"
build_bot_check_messages $prnum "$(cat ${THIS_SCRIPT_PATH}/pr-$prnum-data.txt)" "$(cat ${THIS_SCRIPT_PATH}/pr-$prnum-commits.txt)"
# shellcheck disable=2312
build_bot_check_messages "${prnum}" "$(cat "${THIS_SCRIPT_PATH}/pr-${prnum}-data.txt")" "$(cat "${THIS_SCRIPT_PATH}/pr-${prnum}-commits.txt")"
for i in "${!expected_build_platforms[@]}"; do
assert-equal "${build_platforms[$i]:-missing}" "${expected_build_platforms[$i]:-missing}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]:-missing}" "${expected_build_platforms[${i}]:-missing}" "PR #${prnum}: build_platforms[${i}]"
done
for i in "${!build_platforms[@]}"; do
assert-equal "${build_platforms[$i]:-missing}" "${expected_build_platforms[$i]:-missing}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]:-missing}" "${expected_build_platforms[${i}]:-missing}" "PR #${prnum}: build_platforms[${i}]"
done
}
@ -89,13 +92,13 @@ _do_test_build_bot_check_messages_inline() {
local prinfo="$4"
local prcommits="$5"
build_bot_check_messages $prnum "$prinfo" "$prcommits"
build_bot_check_messages "${prnum}" "${prinfo}" "${prcommits}"
for i in "${!expected_build_platforms[@]}"; do
assert-equal "${build_platforms[$i]:-missing}" "${expected_build_platforms[$i]:-missing}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]:-missing}" "${expected_build_platforms[${i}]:-missing}" "PR #${prnum}: build_platforms[${i}]"
done
for i in "${!build_platforms[@]}"; do
assert-equal "${build_platforms[$i]:-missing}" "${expected_build_platforms[$i]:-missing}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]:-missing}" "${expected_build_platforms[${i}]:-missing}" "PR #${prnum}: build_platforms[${i}]"
done
}
@ -119,8 +122,8 @@ test_build_bot_update_commands() {
# not testing invalid legacy command: _do_test_build_bot_update_commands '[windows]="release"' "build foo" '[windows]="release"'
_do_test_build_bot_update_commands '[windows]="release"' "build:common" '[common]="build" [windows]="release"'
_do_test_build_bot_update_commands '[windows]="release"' "build common" '[common]="build" [windows]="release"'
_do_test_build_bot_update_commands '[windows]="release"' "build:all" "$ALL_BUILD_PLATFORMS_BUILD_EXPECTED"
_do_test_build_bot_update_commands '[windows]="release"' "build all" "$ALL_BUILD_PLATFORMS_BUILD_EXPECTED"
_do_test_build_bot_update_commands '[windows]="release"' "build:all" "${ALL_BUILD_PLATFORMS_BUILD_EXPECTED}"
_do_test_build_bot_update_commands '[windows]="release"' "build all" "${ALL_BUILD_PLATFORMS_BUILD_EXPECTED}"
builder_echo end test_build_bot_update_commands success 'SUCCESS: build_bot_update_commands'
}
@ -134,16 +137,17 @@ test_build_bot_update_commands() {
#
_do_test_build_bot_update_commands() {
eval "declare -gA build_platforms=($1)"
local update_command="$2"
local update_commands="$2"
eval "declare -A expected_build_platforms=($3)"
build_bot_update_commands $update_command
# shellcheck disable=SC2086 # intentionally no quotes
build_bot_update_commands ${update_commands}
for i in "${!expected_build_platforms[@]}"; do
assert-equal "${build_platforms[$i]}" "${expected_build_platforms[$i]}" "build_platforms[$i]"
assert-equal "${build_platforms[${i}]}" "${expected_build_platforms[${i}]}" "build_platforms[${i}]"
done
for i in "${!build_platforms[@]}"; do
assert-equal "${build_platforms[$i]}" "${expected_build_platforms[$i]}" "build_platforms[$i]"
assert-equal "${build_platforms[${i}]}" "${expected_build_platforms[${i}]}" "build_platforms[${i}]"
done
}
@ -159,14 +163,16 @@ test_build_bot_verify_platforms() {
}
_do_test_build_bot_verify_platforms() {
# shellcheck disable=SC2206 # intentionally no quotes
local platforms=($1)
# shellcheck disable=SC2206 # intentionally no quotes
local expected_platforms=($2)
build_bot_verify_platforms platforms
assert-equal ${#platforms[@]} ${#expected_platforms[@]} "#platforms[@]"
for i in "${!expected_platforms[@]}"; do
assert-equal "${platforms[$i]}" "${expected_platforms[$i]}" "platforms[$i]"
assert-equal "${platforms[${i}]}" "${expected_platforms[${i}]}" "platforms[${i}]"
done
}
@ -194,13 +200,13 @@ _do_test_test_bot_check_pr_body() {
local prinfo="$3"
eval "declare -A expected_build_platforms=($4)"
test_bot_check_pr_body $prnum "$prinfo"
test_bot_check_pr_body "${prnum}" "${prinfo}"
for i in "${!expected_build_platforms[@]}"; do
assert-equal "${build_platforms[$i]}" "${expected_build_platforms[$i]}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]}" "${expected_build_platforms[${i}]}" "PR #${prnum}: build_platforms[${i}]"
done
for i in "${!build_platforms[@]}"; do
assert-equal "${build_platforms[$i]}" "${expected_build_platforms[$i]}" "PR #$prnum: build_platforms[$i]"
assert-equal "${build_platforms[${i}]}" "${expected_build_platforms[${i}]}" "PR #${prnum}: build_platforms[${i}]"
done
}
@ -209,4 +215,4 @@ _do_test_test_bot_check_pr_body() {
test_build_bot_verify_platforms
test_build_bot_update_commands
test_build_bot_check_messages
test_test_bot_check_pr_body
test_test_bot_check_pr_body