From 7aa3ce8a4d1175e8a4086878ae2612819d079ace Mon Sep 17 00:00:00 2001 From: coryrc Date: Fri, 19 Sep 2025 21:12:31 -0400 Subject: [PATCH] Shellcheck everything (#10730) * Shellcheck all shell scripts * Implement Shellcheck's recommendations * Shellcheck the distribution-specific files * Include the distro scripts to trigger action * Fix array usage (hopefully) * Use single-quote string TIL: single quote string in yaml treats everything as literal, but double quote allows backslash escaping. * Make all cmake commands use set+-x dance and fix macos getopts line Make Claude happy getopts has colon after a command which takes an argument --------- Co-authored-by: SoftFever --- .github/workflows/shellcheck.yml | 14 +++++-- build_linux.sh | 65 +++++++++++++++++++------------- build_release_macos.sh | 2 +- scripts/DockerBuild.sh | 12 +++--- scripts/DockerRun.sh | 9 ++--- scripts/linux.d/arch | 13 ++++--- scripts/linux.d/clear-linux-os | 9 +++-- scripts/linux.d/debian | 8 ++-- scripts/linux.d/fedora | 13 ++++--- scripts/pack_profiles.sh | 6 +-- scripts/run_gettext.sh | 4 +- 11 files changed, 90 insertions(+), 65 deletions(-) diff --git a/.github/workflows/shellcheck.yml b/.github/workflows/shellcheck.yml index a605cbd1d8..db702f467f 100644 --- a/.github/workflows/shellcheck.yml +++ b/.github/workflows/shellcheck.yml @@ -1,10 +1,14 @@ on: push: paths: - - build_linux.sh + - '**.sh' + - 'scripts/linux.d/*' pull_request: paths: - - build_linux.sh + - '**.sh' + - 'scripts/linux.d/*' + schedule: + - cron: '55 7 * * *' # run once a day near midnight US Pacific time workflow_dispatch: # allows for manual dispatch name: "Shellcheck" @@ -33,6 +37,8 @@ jobs: mv ~/shellcheck-"${INPUT_VERSION}"/shellcheck ~/shellcheck - uses: actions/checkout@v5 + with: + fetch-depth: 1 - - name: Shellcheck build_linux.sh - run: ~/shellcheck -e SC1090 build_linux.sh + - name: Shellcheck scripts + run: 'find . -not -name \*.md \( -path ./scripts/linux.d/\* -o -name \*.sh \) -print0 | xargs -0 ~/shellcheck' diff --git a/build_linux.sh b/build_linux.sh index 4ad73034e8..5be72253fb 100755 --- a/build_linux.sh +++ b/build_linux.sh @@ -133,6 +133,7 @@ if [ ! -f "./scripts/linux.d/${DISTRIBUTION}" ] ; then exit 1 else echo "resolving system dependencies for distribution \"${DISTRIBUTION}\" ..." + # shellcheck source=/dev/null source "./scripts/linux.d/${DISTRIBUTION}" fi @@ -155,17 +156,17 @@ if [[ -z "${SKIP_RAM_CHECK}" ]] ; then check_available_memory_and_disk fi -export CMAKE_C_CXX_COMPILER_CLANG="" +export CMAKE_C_CXX_COMPILER_CLANG=() if [[ -n "${USE_CLANG}" ]] ; then - export CMAKE_C_CXX_COMPILER_CLANG="-DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++" + export CMAKE_C_CXX_COMPILER_CLANG=(-DCMAKE_C_COMPILER=/usr/bin/clang -DCMAKE_CXX_COMPILER=/usr/bin/clang++) fi # Configure use of ld.lld as the linker when requested -export CMAKE_LLD_LINKER_ARGS="" +export CMAKE_LLD_LINKER_ARGS=() if [[ -n "${USE_LLD}" ]] ; then if command -v ld.lld >/dev/null 2>&1 ; then LLD_BIN=$(command -v ld.lld) - export CMAKE_LLD_LINKER_ARGS="-DCMAKE_LINKER=${LLD_BIN} -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_SHARED_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_MODULE_LINKER_FLAGS=-fuse-ld=lld" + export CMAKE_LLD_LINKER_ARGS=(-DCMAKE_LINKER="${LLD_BIN}" -DCMAKE_EXE_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_SHARED_LINKER_FLAGS=-fuse-ld=lld -DCMAKE_MODULE_LINKER_FLAGS=-fuse-ld=lld) else echo "Error: ld.lld not found. Please install the 'lld' package (e.g., sudo apt install lld) or omit -L." exit 1 @@ -174,7 +175,8 @@ fi if [[ -n "${BUILD_DEPS}" ]] ; then echo "Configuring dependencies..." - BUILD_ARGS="${DEPS_EXTRA_BUILD_ARGS} -DDEP_WX_GTK3=ON" + read -r -a BUILD_ARGS <<< "${DEPS_EXTRA_BUILD_ARGS}" + BUILD_ARGS+=(-DDEP_WX_GTK3=ON) if [[ -n "${CLEAN_BUILD}" ]] then rm -fr deps/build @@ -183,17 +185,26 @@ if [[ -n "${BUILD_DEPS}" ]] ; then if [[ -n "${BUILD_DEBUG}" ]] ; then # build deps with debug and release else cmake will not find required sources mkdir -p deps/build/release - CMAKE_CMD="cmake ${CMAKE_C_CXX_COMPILER_CLANG} ${CMAKE_LLD_LINKER_ARGS} -S deps -B deps/build/release -DSLIC3R_PCH=${SLIC3R_PRECOMPILED_HEADERS} -G Ninja -DDESTDIR=${SCRIPT_PATH}/deps/build/destdir -DDEP_DOWNLOAD_DIR=${SCRIPT_PATH}/deps/DL_CACHE ${COLORED_OUTPUT} ${BUILD_ARGS}" - echo "${CMAKE_CMD}" - ${CMAKE_CMD} + set -x + cmake -S deps -B deps/build/release "${CMAKE_C_CXX_COMPILER_CLANG[@]}" "${CMAKE_LLD_LINKER_ARGS[@]}" -G Ninja \ + -DSLIC3R_PCH="${SLIC3R_PRECOMPILED_HEADERS}" \ + -DDESTDIR="${SCRIPT_PATH}/deps/build/destdir" \ + -DDEP_DOWNLOAD_DIR="${SCRIPT_PATH}/deps/DL_CACHE" \ + "${COLORED_OUTPUT}" \ + "${BUILD_ARGS[@]}" + set +x cmake --build deps/build/release - BUILD_ARGS="${BUILD_ARGS} -DCMAKE_BUILD_TYPE=Debug" + BUILD_ARGS+=(-DCMAKE_BUILD_TYPE=Debug) fi - # If this isn't in one quote, then empty variables can add two single quotes and mess up argument parsing for cmake. - CMAKE_CMD="cmake -S deps -B deps/build ${CMAKE_C_CXX_COMPILER_CLANG} ${CMAKE_LLD_LINKER_ARGS} -G Ninja ${COLORED_OUTPUT} ${BUILD_ARGS}" - echo "${CMAKE_CMD}" - ${CMAKE_CMD} + set -x + cmake -S deps -B deps/build "${CMAKE_C_CXX_COMPILER_CLANG[@]}" "${CMAKE_LLD_LINKER_ARGS[@]}" -G Ninja \ + -DSLIC3R_PCH="${SLIC3R_PRECOMPILED_HEADERS}" \ + -DDESTDIR="${SCRIPT_PATH}/deps/build/destdir" \ + -DDEP_DOWNLOAD_DIR="${SCRIPT_PATH}/deps/DL_CACHE" \ + "${COLORED_OUTPUT}" \ + "${BUILD_ARGS[@]}" + set +x cmake --build deps/build fi @@ -202,29 +213,29 @@ if [[ -n "${BUILD_ORCA}" ]] ; then if [[ -n "${CLEAN_BUILD}" ]] ; then rm -fr build fi - BUILD_ARGS="${ORCA_EXTRA_BUILD_ARGS}" + read -r -a BUILD_ARGS <<< "${ORCA_EXTRA_BUILD_ARGS}" if [[ -n "${FOUND_GTK3_DEV}" ]] ; then - BUILD_ARGS="${BUILD_ARGS} -DSLIC3R_GTK=3" + BUILD_ARGS+=(-DSLIC3R_GTK=3) fi if [[ -n "${BUILD_DEBUG}" ]] ; then - BUILD_ARGS="${BUILD_ARGS} -DCMAKE_BUILD_TYPE=Debug -DBBL_INTERNAL_TESTING=1" + BUILD_ARGS+=(-DCMAKE_BUILD_TYPE=Debug -DBBL_INTERNAL_TESTING=1) else - BUILD_ARGS="${BUILD_ARGS} -DBBL_RELEASE_TO_PUBLIC=1 -DBBL_INTERNAL_TESTING=0" + BUILD_ARGS+=(-DBBL_RELEASE_TO_PUBLIC=1 -DBBL_INTERNAL_TESTING=0) fi if [[ -n "${BUILD_TESTS}" ]] ; then - BUILD_ARGS="${BUILD_ARGS} -DBUILD_TESTS=ON" + BUILD_ARGS+=(-DBUILD_TESTS=ON) fi echo "Configuring OrcaSlicer..." - cmake -S . -B build ${CMAKE_C_CXX_COMPILER_CLANG} ${CMAKE_LLD_LINKER_ARGS} -G "Ninja Multi-Config" \ --DSLIC3R_PCH=${SLIC3R_PRECOMPILED_HEADERS} \ --DCMAKE_PREFIX_PATH=${SCRIPT_PATH}/deps/build/destdir/usr/local \ --DSLIC3R_STATIC=1 \ --DORCA_TOOLS=ON \ -${COLORED_OUTPUT} \ -${BUILD_ARGS} - echo "${CMAKE_CMD}" - ${CMAKE_CMD} + set -x + cmake -S . -B build "${CMAKE_C_CXX_COMPILER_CLANG[@]}" "${CMAKE_LLD_LINKER_ARGS[@]}" -G "Ninja Multi-Config" \ + -DSLIC3R_PCH="${SLIC3R_PRECOMPILED_HEADERS}" \ + -DCMAKE_PREFIX_PATH="${SCRIPT_PATH}/deps/build/destdir/usr/local" \ + -DSLIC3R_STATIC=1 \ + -DORCA_TOOLS=ON \ + "${COLORED_OUTPUT}" \ + "${BUILD_ARGS[@]}" + set +x echo "done" echo "Building OrcaSlicer ..." if [[ -n "${BUILD_DEBUG}" ]] ; then diff --git a/build_release_macos.sh b/build_release_macos.sh index 4bc9b14ded..4ef49bbc6e 100755 --- a/build_release_macos.sh +++ b/build_release_macos.sh @@ -3,7 +3,7 @@ set -e set -o pipefail -while getopts ":dpa:snt:xbc:h" opt; do +while getopts ":dpa:snt:xbc:1h" opt; do case "${opt}" in d ) export BUILD_TARGET="deps" diff --git a/scripts/DockerBuild.sh b/scripts/DockerBuild.sh index b0f3044fb0..7f3bdb0816 100755 --- a/scripts/DockerBuild.sh +++ b/scripts/DockerBuild.sh @@ -1,6 +1,6 @@ #!/bin/bash SCRIPT_DIR=$(cd -P -- "$(dirname -- "$0")" && printf '%s\n' "$(pwd -P)") -PROJECT_ROOT=$(dirname "$SCRIPT_DIR") +PROJECT_ROOT="$(dirname "$SCRIPT_DIR")" set -x # Wishlist hint: For developers, creating a Docker Compose @@ -9,9 +9,9 @@ set -x # the simplicity of a single Docker image and a one-time compilation # seems better. docker build -t orcaslicer \ - --build-arg USER=$USER \ - --build-arg UID=$(id -u) \ - --build-arg GID=$(id -g) \ - --build-arg NCORES=$NCORES \ + --build-arg USER="$USER" \ + --build-arg UID="$(id -u)" \ + --build-arg GID="$(id -g)" \ + --build-arg NCORES="$NCORES" \ -f "$SCRIPT_DIR/Dockerfile" \ - $PROJECT_ROOT + "$PROJECT_ROOT" diff --git a/scripts/DockerRun.sh b/scripts/DockerRun.sh index c05022b1b8..07c85df922 100755 --- a/scripts/DockerRun.sh +++ b/scripts/DockerRun.sh @@ -13,11 +13,11 @@ docker run \ `# Some X installs will not have permissions to talk to sockets for shared memory` \ --ipc host \ `# Run as your workstations username to keep permissions the same` \ - -u $USER \ + -u "$USER" \ `# Bind mount your home directory into the container for loading/saving files` \ - -v $HOME:/home/$USER \ + -v "$HOME:/home/$USER" \ `# Pass the X display number to the container` \ - -e DISPLAY=$DISPLAY \ + -e DISPLAY="$DISPLAY" \ `# It seems that libGL and dbus things need privileged mode` \ --privileged=true \ `# Attach tty for running orca slicer with command line things` \ @@ -25,5 +25,4 @@ docker run \ `# Clean up after yourself` \ --rm \ `# Pass all parameters from this script to the orca slicer ENTRYPOINT binary` \ - orcaslicer $* - + orcaslicer "$@" diff --git a/scripts/linux.d/arch b/scripts/linux.d/arch index 8ce865c988..48aafb587b 100644 --- a/scripts/linux.d/arch +++ b/scripts/linux.d/arch @@ -1,4 +1,6 @@ +#!/bin/bash # these are the Arch Linux specific build functions +export FOUND_GTK3 FOUND_GTK3=$(pacman -Q gtk3) # Addtional Dev packages for OrcaSlicer @@ -30,16 +32,17 @@ export REQUIRED_DEV_PACKAGES=( if [[ -n "$UPDATE_LIB" ]] then echo -n -e "Updating linux ...\n" - NEEDED_PKGS="" - for PKG in ${REQUIRED_DEV_PACKAGES[@]}; do - pacman -Q ${PKG} > /dev/null || NEEDED_PKGS+=" ${PKG}" + NEEDED_PKGS=() + for PKG in "${REQUIRED_DEV_PACKAGES[@]}"; do + pacman -Q "${PKG}" > /dev/null || NEEDED_PKGS+=("${PKG}") done - if [ -n "${NEEDED_PKGS}" ]; then - sudo pacman -Syy --noconfirm ${NEEDED_PKGS} + if [[ "${#NEEDED_PKGS[*]}" -gt 0 ]]; then + sudo pacman -Syy --noconfirm "${NEEDED_PKGS[@]}" fi echo -e "done\n" exit 0 fi +export FOUND_GTK3_DEV FOUND_GTK3_DEV=${FOUND_GTK3} diff --git a/scripts/linux.d/clear-linux-os b/scripts/linux.d/clear-linux-os index 9453ddcce5..9174634867 100644 --- a/scripts/linux.d/clear-linux-os +++ b/scripts/linux.d/clear-linux-os @@ -1,5 +1,7 @@ +#!/bin/bash # these are the Clear Linux specific build functions -FOUND_GTK3=$(ls /usr/lib64/libgtk-3.so.* 2>/dev/null | tail -n 1 || true) +export FOUND_GTK3 +FOUND_GTK3=$(find /usr/lib64/libgtk-3.so.* 2>/dev/null | tail -n 1 || true) # Addtional bundles for OrcaSlicer export REQUIRED_BUNDLES=( @@ -25,9 +27,10 @@ export REQUIRED_BUNDLES=( if [[ -n "$UPDATE_LIB" ]] then echo "Updating linux ..." - echo swupd bundle-add -y ${REQUIRED_BUNDLES[@]} + echo swupd bundle-add -y "${REQUIRED_BUNDLES[@]}" echo -e "done\n" exit 0 fi -FOUND_GTK3_DEV=$(ls /usr/lib64/libgtk-3.so 2>/dev/null || true) +export FOUND_GTK3_DEV +FOUND_GTK3_DEV=$(find /usr/lib64/libgtk-3.so 2>/dev/null || true) diff --git a/scripts/linux.d/debian b/scripts/linux.d/debian index cce41d74ce..206856a6f4 100644 --- a/scripts/linux.d/debian +++ b/scripts/linux.d/debian @@ -1,3 +1,5 @@ +#!/bin/bash +export FOUND_GTK3 FOUND_GTK3=$(dpkg -l libgtk* | grep gtk-3 || echo '') REQUIRED_DEV_PACKAGES=( @@ -27,6 +29,7 @@ REQUIRED_DEV_PACKAGES=( if [[ -n "$UPDATE_LIB" ]] then + # shellcheck source=/dev/null source /etc/os-release if [ "${ID}" == "ubuntu" ] && [ -n "${VERSION_ID}" ]; then # It's ubuntu and we have a VERSION_ID like "24.04". @@ -50,14 +53,13 @@ then REQUIRED_DEV_PACKAGES+=(libwebkit2gtk-4.1-dev) fi - # TODO: optimize this by checking which, if any, packages are already installed - # install them all at once sudo apt update - sudo apt install -y ${REQUIRED_DEV_PACKAGES[@]} + sudo apt install -y "${REQUIRED_DEV_PACKAGES[@]}" echo -e "done\n" exit 0 fi +export FOUND_GTK3_DEV FOUND_GTK3_DEV=$(dpkg -l libgtk* | grep gtk-3-dev || echo '') diff --git a/scripts/linux.d/fedora b/scripts/linux.d/fedora index ed91883dd9..b49e0cda22 100644 --- a/scripts/linux.d/fedora +++ b/scripts/linux.d/fedora @@ -1,3 +1,5 @@ +#!/bin/bash +export FOUND_GTK3 FOUND_GTK3=$(rpm -qa | grep -P '^gtk3' || true) REQUIRED_DEV_PACKAGES=( @@ -34,16 +36,17 @@ REQUIRED_DEV_PACKAGES=( if [[ -n "$UPDATE_LIB" ]] then - NEEDED_PKGS="" - for PKG in ${REQUIRED_DEV_PACKAGES[@]}; do - rpm -q ${PKG} > /dev/null || NEEDED_PKGS+=" ${PKG}" + NEEDED_PKGS=() + for PKG in "${REQUIRED_DEV_PACKAGES[@]}"; do + rpm -q "${PKG}" > /dev/null || NEEDED_PKGS+=("${PKG}") done - if [ -n "${NEEDED_PKGS}" ]; then - sudo dnf install -y ${NEEDED_PKGS} + if [[ "${#NEEDED_PKGS[*]}" -gt 0 ]]; then + sudo dnf install -y "${NEEDED_PKGS[@]}" fi echo -e "done\n" exit 0 fi +export FOUND_GTK3_DEV FOUND_GTK3_DEV=$(rpm -qa | grep -P '^gtk3-devel' || true) diff --git a/scripts/pack_profiles.sh b/scripts/pack_profiles.sh index 946f7814bc..54810cc7d9 100755 --- a/scripts/pack_profiles.sh +++ b/scripts/pack_profiles.sh @@ -59,14 +59,14 @@ for VENDOR in "$@"; do done # Create zip file -cd "$TEMP_DIR" +pushd "$TEMP_DIR" || exit 1 zip -r "$OUTPUT_FILE" profiles/ # Move zip file to original directory mv "$OUTPUT_FILE" "$ORIGINAL_DIR/" # Return to original directory -cd "$ORIGINAL_DIR" +popd || exit 1 # Clean up rm -rf "$TEMP_DIR" @@ -78,4 +78,4 @@ if [ -f "$OUTPUT_FILE" ]; then else echo "Error: Failed to create zip file" exit 1 -fi \ No newline at end of file +fi diff --git a/scripts/run_gettext.sh b/scripts/run_gettext.sh index 561d6da5ed..e828ff1632 100755 --- a/scripts/run_gettext.sh +++ b/scripts/run_gettext.sh @@ -31,9 +31,7 @@ do msgmerge -N -o "$dir/OrcaSlicer_${lang}.po" "$dir/OrcaSlicer_${lang}.po" "$pot_file" fi mkdir -p "resources/i18n/${lang}" - msgfmt --check-format -o "resources/i18n/${lang}/OrcaSlicer.mo" "$dir/OrcaSlicer_${lang}.po" - # Check the exit status of the msgfmt command - if [ $? -ne 0 ]; then + if ! msgfmt --check-format -o "resources/i18n/${lang}/OrcaSlicer.mo" "$dir/OrcaSlicer_${lang}.po"; then echo "Error encountered with msgfmt command for language ${lang}." exit 1 # Exit the script with an error status fi