From 9ff3c5e9624200ecc172dfb214ef8b785a57370c Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Sun, 25 Aug 2024 17:22:13 +0200 Subject: [PATCH 1/9] Fix syntax-check directory Currently, only files in the `.github/jobs` directory are tested. --- .github/jobs/syntax-check | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/jobs/syntax-check b/.github/jobs/syntax-check index ac9d5a8950..b92edeeb66 100755 --- a/.github/jobs/syntax-check +++ b/.github/jobs/syntax-check @@ -3,7 +3,9 @@ set -euo pipefail # shellcheck disable=SC2242 -cd "$(dirname "$0")" || exit -1 + +# Change directory to the root of the repo relative to this script file +cd "$(dirname "$0")/../../" || exit -1 PHP=$(command -v php) if [ ! -x "$PHP" ]; then From e5c802bcdaca853f16ccda5f2d920d30154e85c2 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Sun, 25 Aug 2024 17:47:17 +0200 Subject: [PATCH 2/9] Use shell compatible exit code --- .github/jobs/syntax-check | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/jobs/syntax-check b/.github/jobs/syntax-check index b92edeeb66..7ff9ca252d 100755 --- a/.github/jobs/syntax-check +++ b/.github/jobs/syntax-check @@ -5,23 +5,23 @@ set -euo pipefail # shellcheck disable=SC2242 # Change directory to the root of the repo relative to this script file -cd "$(dirname "$0")/../../" || exit -1 +cd "$(dirname "$0")/../../" || exit 1 PHP=$(command -v php) if [ ! -x "$PHP" ]; then echo "PHP ($PHP) not found or not executable" # shellcheck disable=SC2242 - exit -1 + exit 1 fi if [ ! -x /usr/bin/checkbashisms ]; then echo "/usr/bin/checkbashisms not found or not executable" # shellcheck disable=SC2242 - exit -1 + exit 1 fi if [ ! -x /usr/bin/shellcheck ]; then echo "/usr/bin/shellcheck not found or not executable" # shellcheck disable=SC2242 - exit -1 + exit 1 fi find . \( \ From 692ac6b7f22c71a2b97bbfe071cfb2e328857597 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Sun, 25 Aug 2024 18:21:43 +0200 Subject: [PATCH 3/9] Fix shellcheck errors --- etc/gen_all_secrets | 2 +- gitlab/integration.sh | 4 ++-- judge/runguard_test/runguard_test.sh | 19 ++++++++++++------- judge/version_check.sh | 1 + 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/etc/gen_all_secrets b/etc/gen_all_secrets index 72f588ddd8..672dd71856 100755 --- a/etc/gen_all_secrets +++ b/etc/gen_all_secrets @@ -11,7 +11,7 @@ create_pw_file() fi # shellcheck disable=SC2039 - [ -n "$QUIET" ] || echo -n "Running '$SCRIPT'... " + [ -n "$QUIET" ] || echo "Running '$SCRIPT'... " touch "$FILE" chmod go= "$FILE" "./$SCRIPT" > "$FILE" diff --git a/gitlab/integration.sh b/gitlab/integration.sh index a4678e19ea..c3b57d1956 100755 --- a/gitlab/integration.sh +++ b/gitlab/integration.sh @@ -82,8 +82,8 @@ mount -o remount,exec,dev /builds section_end mount section_start check_cgroup_v1 "Checking for cgroup v1 availability" -grep cgroup$ /proc/filesystems -if [ $? -eq 0 ]; then + +if grep cgroup$ /proc/filesystems; then cgroupv1=1 else echo "Skipping tests that rely on cgroup v1" diff --git a/judge/runguard_test/runguard_test.sh b/judge/runguard_test/runguard_test.sh index 6ce5b7650a..e6434f93f1 100755 --- a/judge/runguard_test/runguard_test.sh +++ b/judge/runguard_test/runguard_test.sh @@ -1,10 +1,13 @@ #!/bin/bash +# Ignore unreachable code, as it is called by `make test`. +# shellcheck disable=SC2317 -cd "$(dirname "${BASH_SOURCE}")" +cd "$(dirname "${BASH_SOURCE[0]}")" || exit 1 RUNGUARD=../runguard LOG1="$(mktemp)" LOG2="$(mktemp)" +# shellcheck disable=SC2154 META=$(mktemp -p "$judgehost_tmpdir") fail() { @@ -98,7 +101,7 @@ test_streamsize() { exec_check_fail sudo $RUNGUARD -u domjudge-run-0 -t 1 -s 123 yes DOMjudge expect_stdout "DOMjudge" limit=$((123*1024)) - actual=$(cat "$LOG1" | wc -c) + actual=$(wc -c < "$LOG1") [ $limit -eq $actual ] || fail "stdout not limited to ${limit}B, but wrote ${actual}B" } @@ -107,7 +110,7 @@ test_streamsize_stderr() { expect_stderr "DOMjudge" # Allow 100 bytes extra, for the runguard time limit message. limit=$((42*1024 + 100)) - actual=$(cat "$LOG2" | wc -c) + actual=$(wc -c < "$LOG2") [ $limit -gt $actual ] || fail "stdout not limited to ${limit}B, but wrote ${actual}B" } @@ -120,7 +123,7 @@ test_redir_stdout() { grep -q "foobar" "$stdout" || fail "did not find expected 'foobar' in redirect stdout" # Verify that stdout is empty. - actual=$(cat "$LOG1" | wc -c) + actual=$(wc -c < "$LOG1") [ $actual -eq 0 ] || fail "stdout should be empty, but contains ${actual}B" # This will fail because of the timeout. @@ -129,13 +132,13 @@ test_redir_stdout() { expect_stderr "hard wall time" # Verify that stdout is empty. - actual=$(cat "$LOG1" | wc -c) + actual=$(wc -c < "$LOG1") [ $actual -eq 0 ] || fail "stdout should be empty, but contains ${actual}B" # Verify that redirected stdout has the right contents. grep -q "DOMjudge" "$stdout" || fail "did not find expected 'DOMjudge' in redirect stdout" limit=$((23*1024)) - actual=$(cat "$stdout" | wc -c) + actual=$(wc -c < "$stdout") [ $limit -eq $actual ] || fail "redirected stdout not limited to ${limit}B, but wrote ${actual}B" rm "$stdout" @@ -156,7 +159,7 @@ test_redir_stderr() { # Verify that redirected stdout has the right contents. grep -q "DOMjudge" "$stderr" || fail "did not find expected 'DOMjudge' in redirect stderr" limit=$((11*1024)) - actual=$(cat "$stderr" | wc -c) + actual=$(wc -c < "$stderr") [ $limit -eq $actual ] || fail "redirected stdout not limited to ${limit}B, but wrote ${actual}B" rm "$stderr" @@ -164,6 +167,7 @@ test_redir_stderr() { test_rootdir_changedir() { # Prepare test directory. + # shellcheck disable=SC2154 almost_empty_dir="$judgehost_judgedir/runguard_tests/almost_empty" mkdir -p "$almost_empty_dir"/exists cp hello "$almost_empty_dir"/ @@ -243,6 +247,7 @@ test_meta() { exec_check_fail sudo $RUNGUARD -u domjudge-run-0 -M "$META" false expect_meta 'exitcode: 1' + # shellcheck disable=SC2024 echo "DOMjudge" | sudo $RUNGUARD -u domjudge-run-0 -t 2 -M "$META" rev > "$LOG1" 2> "$LOG2" expect_meta 'wall-time: 0.0' expect_meta 'stdout-bytes: 9' diff --git a/judge/version_check.sh b/judge/version_check.sh index 9fbc3fb1a3..35fee759e3 100755 --- a/judge/version_check.sh +++ b/judge/version_check.sh @@ -66,6 +66,7 @@ touch version_check.out version_check.meta if [ -e "$WORKDIR/version_check-script" ]; then mv "$WORKDIR/version_check-script" "$WORKDIR/version_check-script-old" fi +# shellcheck disable=SC2174 mkdir -m 0777 -p "$WORKDIR/version_check-script" cp -a "$VERSION_CHECK_SCRIPT" "$PWD/version_check-script/" From ec379b7002ef89de9cf40a2ffab4128817a2a2f8 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Sun, 25 Aug 2024 21:06:42 +0200 Subject: [PATCH 4/9] Fix syntax-check errors As the CI job was not correctly run for some time, a lot of small errors were in the repository. --- .github/jobs/syntax-check | 5 +++-- misc-tools/dj_make_chroot.in | 3 ++- misc-tools/dj_run_chroot.in | 1 + sql/dj_setup_database.in | 30 ++++++++++++++-------------- sql/files/defaultdata/r/run | 12 +++++++---- sql/files/defaultdata/swift/run | 2 +- webapp/config/preload.php | 2 +- webapp/src/Utils/EventFeedFormat.php | 2 +- webapp/tests/bootstrap.php | 2 +- 9 files changed, 33 insertions(+), 26 deletions(-) diff --git a/.github/jobs/syntax-check b/.github/jobs/syntax-check index 7ff9ca252d..1c6460cc29 100755 --- a/.github/jobs/syntax-check +++ b/.github/jobs/syntax-check @@ -25,7 +25,7 @@ if [ ! -x /usr/bin/shellcheck ]; then fi find . \( \ - -path ./webapp/vendor -prune \ + -path ./webapp/vendor -prune \ -o -path ./webapp/var -prune \ -o -path ./output -prune \ -o -path ./.git -prune \ @@ -34,7 +34,8 @@ find . \( \ -o -type f \) \ -a -type f | \ while read -r i ; do - if [[ "$i" == *\.php ]] || grep -q "^#\\!.*php" "$i"; then + if [[ "$i" == *\.php ]] || grep -q "^#\\!.*php" "$i" && \ + echo "$i" | grep -qvE '(^\./(webapp/resources/adminer)\.php)'; then $PHP -l "$i" | sed -e 's|No syntax errors detected in ./|check PHP syntax: |' if [[ "$i" == *\.php ]] && [ "$i" != "./webapp/config/bundles.php" ] && ! head -1 "$i"|grep -q "strict_types"; then echo "PHP file missing strict types declaration: $i" diff --git a/misc-tools/dj_make_chroot.in b/misc-tools/dj_make_chroot.in index e8aa003bf9..2291e02187 100755 --- a/misc-tools/dj_make_chroot.in +++ b/misc-tools/dj_make_chroot.in @@ -234,7 +234,7 @@ if [ "$DISTRO" = 'Ubuntu' ]; then # x86(_64) can use the main Ubuntu repo's, other # architectures need to use a mirror from ubuntu-ports. MAINSTREAM="amd64 i386" - if [ -z "${MAINSTREAM##*$ARCH*}" ]; then + if [ -z "${MAINSTREAM##*"$ARCH"*}" ]; then DEBMIRROR="http://us.archive.ubuntu.com/ubuntu/" else DEBMIRROR="http://ports.ubuntu.com/ubuntu-ports/" @@ -321,6 +321,7 @@ fi # Add indication to interactive shell that the user is in the chroot for bashrc in "root/.bashrc" "etc/bash.bashrc"; do + # shellcheck disable=SC2016 echo 'PS1=(chroot)$PS1' >> "$CHROOTDIR/$bashrc" done diff --git a/misc-tools/dj_run_chroot.in b/misc-tools/dj_run_chroot.in index f330468841..8189aacbb3 100755 --- a/misc-tools/dj_run_chroot.in +++ b/misc-tools/dj_run_chroot.in @@ -10,6 +10,7 @@ # Abort when a single command fails: set -e +# shellcheck disable=SC2317 cleanup() { # Unmount things on cleanup umount -f "$CHROOTDIR/proc" >/dev/null 2>&1 || /bin/true diff --git a/sql/dj_setup_database.in b/sql/dj_setup_database.in index 03742fc161..dad966f25a 100755 --- a/sql/dj_setup_database.in +++ b/sql/dj_setup_database.in @@ -53,24 +53,22 @@ EOF mysql_options() { - local user pass - # shellcheck disable=SC2153 if [ -n "$DBUSER" ]; then - user="-u $DBUSER" + _user="-u $DBUSER" else - user="${DBA_USER:+-u ${DBA_USER}}" + _user="${DBA_USER:+-u ${DBA_USER}}" fi # shellcheck disable=SC2153 if [ -n "$PASSWD" ]; then - pass="-p$PASSWD" + _pass="-p$PASSWD" else - [ -n "$PROMPT_PASSWD" ] && pass="-p" - [ -n "$DBA_PASSWD" ] && pass="-p$DBA_PASSWD" + [ -n "$PROMPT_PASSWD" ] && _pass="-p" + [ -n "$DBA_PASSWD" ] && _pass="-p$DBA_PASSWD" fi [ -z "$USE_SOCKET" ] && port="-P$DBPORT" - echo $user ${pass:+"$pass"} -h "$DBHOST" ${port:+"$port"} + echo $_user ${_pass:+"$_pass"} -h "$DBHOST" ${port:+"$port"} } # Wrapper around mysql command to allow setting options, user, etc. @@ -363,7 +361,7 @@ upgrade) # Check if we need to upgrade the Doctrine migrations table if ! echo "SHOW CREATE TABLE \`doctrine_migration_versions\`" | mysql "$DBNAME" >/dev/null 2>&1; then symfony_console doctrine:migrations:sync-metadata-storage -n - # shellcheck disable=SC2016 + # shellcheck disable=SC2016,SC2028 echo 'INSERT INTO `doctrine_migration_versions` (version, executed_at, execution_time) SELECT concat("DoctrineMigrations\\\\Version", version), executed_at, 1 @@ -386,7 +384,8 @@ dump) if [ -f "${DATABASEDUMPDIR}/${DUMPNAME}.sql.gz" ]; then while true; do - read -p "Overwrite existing database dump (y/N)? " yn + printf "Overwrite existing database dump (y/N)? " + read -r yn case $yn in [Yy]* ) break ;; ''|[Nn]* ) exit 0;; @@ -407,19 +406,20 @@ load) exit 1 fi ind=1 - for i in "$databases"; do + for i in $databases; do echo "$ind) $i" - : $((ind+=1)) + ind=$((ind+1)) done while true; do - read -p "Which database should be loaded? " db + echo "Which database should be loaded? " + read -r db ind=1 - for i in "$databases"; do + for i in $databases; do if [ "$ind" = "$db" ]; then FILE="$i" break fi - : $((ind+=1)) + ind=$((ind+1)) done if [ -n "$FILE" ]; then break diff --git a/sql/files/defaultdata/r/run b/sql/files/defaultdata/r/run index 7c860c7f61..40970d7a7a 100755 --- a/sql/files/defaultdata/r/run +++ b/sql/files/defaultdata/r/run @@ -30,10 +30,14 @@ fi # # Store intermediate files in the current dir (/compile) as its only # available during compilation step. -export TMPDIR=`pwd` -Rscript -e "parse('"$@"')" -EXITCODE=$? -[ "$EXITCODE" -ne 0 ] && exit $EXITCODE +TMPDIR=`pwd` +export TMPDIR +for f in "$@" +do + Rscript -e "parse('$f')" + EXITCODE=$? + [ "$EXITCODE" -ne 0 ] && exit $EXITCODE +done # Write executing script: cat > "$DEST" < Date: Sun, 25 Aug 2024 21:43:29 +0200 Subject: [PATCH 5/9] Make chmod only for script directory explicit --- judge/compile.sh | 4 ++-- judge/version_check.sh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/judge/compile.sh b/judge/compile.sh index 8d6eccc222..14d768b2ae 100755 --- a/judge/compile.sh +++ b/judge/compile.sh @@ -122,8 +122,8 @@ chmod a+rwx "$WORKDIR/compile" touch compile.out compile.meta # Copy compile script into chroot -# shellcheck disable=SC2174 -mkdir -m 0777 -p "$WORKDIR/compile-script" +mkdir -p "$WORKDIR/compile-script" +chmod 0777 "$WORKDIR/compile-script" cp -a "$(dirname "$COMPILE_SCRIPT")"/* "$WORKDIR/compile-script/" cd "$WORKDIR/compile" diff --git a/judge/version_check.sh b/judge/version_check.sh index 35fee759e3..76a8054e30 100755 --- a/judge/version_check.sh +++ b/judge/version_check.sh @@ -66,8 +66,8 @@ touch version_check.out version_check.meta if [ -e "$WORKDIR/version_check-script" ]; then mv "$WORKDIR/version_check-script" "$WORKDIR/version_check-script-old" fi -# shellcheck disable=SC2174 -mkdir -m 0777 -p "$WORKDIR/version_check-script" +mkdir -p "$WORKDIR/version_check-script" +chmod 0777 -p "$WORKDIR/version_check-script" cp -a "$VERSION_CHECK_SCRIPT" "$PWD/version_check-script/" cd "$WORKDIR/version_check" From b89ff1f3335b32afaed5d0bcdfdd1e4dc2726f09 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Sun, 25 Aug 2024 22:38:11 +0200 Subject: [PATCH 6/9] Use printf for the prompt question --- sql/dj_setup_database.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/dj_setup_database.in b/sql/dj_setup_database.in index dad966f25a..6bc35b34bf 100755 --- a/sql/dj_setup_database.in +++ b/sql/dj_setup_database.in @@ -411,7 +411,7 @@ load) ind=$((ind+1)) done while true; do - echo "Which database should be loaded? " + printf "Which database should be loaded? " read -r db ind=1 for i in $databases; do From 4f8a8768f494a3ed6eba8b08c3dba048963f7b80 Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Thu, 19 Sep 2024 09:48:48 +0200 Subject: [PATCH 7/9] Note we should revert shellcheck-disable later Suggested-by: Michael Vasseur <14887731+vmcj@users.noreply.github.com> Signed-off-by: Kevin Jilissen --- misc-tools/dj_run_chroot.in | 1 + 1 file changed, 1 insertion(+) diff --git a/misc-tools/dj_run_chroot.in b/misc-tools/dj_run_chroot.in index 8189aacbb3..a6c389b4f1 100755 --- a/misc-tools/dj_run_chroot.in +++ b/misc-tools/dj_run_chroot.in @@ -10,6 +10,7 @@ # Abort when a single command fails: set -e +# See https://github.com/koalaman/shellcheck/issues/2660 # shellcheck disable=SC2317 cleanup() { # Unmount things on cleanup From 58d999ec13c532404fb9233fbf7ea83850f29d3b Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Thu, 19 Sep 2024 10:13:00 +0200 Subject: [PATCH 8/9] Use printf instead Suggested-by: Jaap Eldering Signed-off-by: Kevin Jilissen --- etc/gen_all_secrets | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/etc/gen_all_secrets b/etc/gen_all_secrets index 672dd71856..7cec905cb7 100755 --- a/etc/gen_all_secrets +++ b/etc/gen_all_secrets @@ -11,7 +11,7 @@ create_pw_file() fi # shellcheck disable=SC2039 - [ -n "$QUIET" ] || echo "Running '$SCRIPT'... " + [ -n "$QUIET" ] || printf "Running '%s'... " "$SCRIPT" touch "$FILE" chmod go= "$FILE" "./$SCRIPT" > "$FILE" From 443ecda866831b34f5ea1cc4251807839693317a Mon Sep 17 00:00:00 2001 From: Kevin Jilissen Date: Thu, 19 Sep 2024 11:04:53 +0200 Subject: [PATCH 9/9] Use quiet grep for detecting cgroup version Suggested-by: Jaap Eldering Signed-off-by: Kevin Jilissen --- gitlab/integration.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitlab/integration.sh b/gitlab/integration.sh index c3b57d1956..6d74f2cad9 100755 --- a/gitlab/integration.sh +++ b/gitlab/integration.sh @@ -83,7 +83,7 @@ section_end mount section_start check_cgroup_v1 "Checking for cgroup v1 availability" -if grep cgroup$ /proc/filesystems; then +if grep -qs cgroup$ /proc/filesystems; then cgroupv1=1 else echo "Skipping tests that rely on cgroup v1"