From cf8e8e855b4a82a1d7c4d11437d630a9719b45a2 Mon Sep 17 00:00:00 2001 From: Dirk Heilig Date: Tue, 9 Apr 2024 13:02:00 +0200 Subject: [PATCH 1/6] fix missiong quotes to prevent globbing and path handling with special chars, exit on failing cd --- colorscad.sh | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/colorscad.sh b/colorscad.sh index 8605202..7af1626 100755 --- a/colorscad.sh +++ b/colorscad.sh @@ -1,11 +1,11 @@ #!/usr/bin/env bash # Get this script's directory -DIR_SCRIPT=$( +DIR_SCRIPT="$( X=$(command -v "$0") - cd "${X%${X##*/}}." + cd "${X%"${X##*/}"}." || exit 1 pwd -) +)" function usage { cat < /dev/null; then exit 1 fi -if [ -z "$INPUT" -o -z "$OUTPUT" ]; then +if [ -z "$INPUT" ] || [ -z "$OUTPUT" ]; then echo "You must provide both input (-i) and output (-o) files. See help (-h)." exit 1 fi @@ -143,14 +143,14 @@ if [ "$FORMAT" = 3mf ]; then fi # Convert OUTPUT to a full path, because we're going to change current directory (see below) -OUTPUT=$(cd "${OUTPUT%${OUTPUT##*/}}."; pwd)/${OUTPUT##*/} +OUTPUT="$(cd "${OUTPUT%"${OUTPUT##*/}"}." || exit 1 ; pwd)/${OUTPUT##*/}" # Change the current dir to the input's dir, for consistent behavior. # That is because not all OpenSCAD versions behave the same when the input is not in the current dir; # in some versions 'import()' is relative to the current dir instead of the input's dir, and in # other versions .csg output is written relative to the input's dir, instead of the current dir. ORIGINAL_PWD=$(pwd) -cd "${INPUT%${INPUT##*/}}." +cd "${INPUT%"${INPUT##*/}"}." || exit 1 INPUT=${INPUT##*/} # Create a temporary, unique .csg file in the input's directory. @@ -169,6 +169,8 @@ INPUT_CSG=$( # the default temp dir; on i.e. Ubuntu, openscad can be a snap package which doesn't have access to /tmp/ TEMPDIR=$(mktemp -d ./tmp.XXXXXX) # Cleanup trigger +# shellcheck disable=SC2064 +# this SHOULD expand now trap "rm -Rf '$(pwd)/${INPUT_CSG}' '$(pwd)/${TEMPDIR}'" EXIT # Convert input to a .csg file, mainly to resolve named colors. Also to evaluate functions etc. only once. @@ -220,7 +222,7 @@ if [ -z "$COLORS" ]; then echo "Error: no colors were found at all. Looks like something went wrong..." exit 1 fi -let COLOR_COUNT="$(echo "$COLORS" | wc -l)" +COLOR_COUNT="$(echo "$COLORS" | wc -l)" echo "${COLOR_COUNT} unique colors were found." if [ $VERBOSE -eq 1 ]; then echo @@ -257,8 +259,8 @@ function render_color { IFS=$'\n' JOB_ID=0 for COLOR in $COLORS; do - let JOB_ID++ - if [ $(jobs | wc -l) -ge $PARALLEL_JOB_LIMIT ]; then + (( JOB_ID++ )) + if [ "$(jobs | wc -l)" -ge "$PARALLEL_JOB_LIMIT" ]; then # Wait for one job to finish, before continuing wait_n fi @@ -286,7 +288,7 @@ if [ "$FORMAT" = amf ]; then B=$3 A=${4%]} echo " ${R// }${G// }${B// }${A// }" - let id++ + (( id++ )) done id=0 IFS=$'\n' @@ -298,16 +300,16 @@ if [ "$FORMAT" = amf ]; then sed "1,4 d; \$ d; s///" "${TEMPDIR}/${COLOR}.amf" else echo "Skipping ${COLOR}!" >&2 - let SKIPPED++ + (( SKIPPED++ )) fi - let id++ + (( id++ )) echo -ne "\r ${id}/${COLOR_COUNT} " >&2 done echo '' } > "$OUTPUT" # Strip original current dir prefix, if present, to make message smaller - OUT=${OUTPUT#${ORIGINAL_PWD}/} + OUT="${OUTPUT#"${ORIGINAL_PWD}"/}" echo echo "To create a compressed AMF, run:" echo " zip '${OUT}.zip' '$OUT' && mv '${OUT}.zip' '${OUT}'" @@ -320,7 +322,7 @@ if [ "$FORMAT" = amf ]; then elif [ "$FORMAT" = 3mf ]; then # Run from inside TEMPDIR, to support having a Windows-format 3mfmerge binary ( - cd "$TEMPDIR" + cd "$TEMPDIR" || exit 1 "${DIR_3MFMERGE}"/bin/3mfmerge merged.3mf < \ <(echo "$COLORS" | sed "s/\$/\.${FORMAT}/") ) From 1c0d42ee1756d965bc336629aae29970919161f1 Mon Sep 17 00:00:00 2001 From: Dirk Heilig Date: Tue, 16 Apr 2024 12:06:02 +0200 Subject: [PATCH 2/6] change color extracting and random name generation --- colorscad.sh | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/colorscad.sh b/colorscad.sh index 7af1626..01fee6e 100755 --- a/colorscad.sh +++ b/colorscad.sh @@ -158,8 +158,8 @@ INPUT=${INPUT##*/} # On macOS, 'mktemp' cannot create a file with a given extension, so use a workaround. INPUT_CSG=$( set -o noclobber - NAME= - until > "$NAME"; do + NAME=tmp.$$_${RANDOM}.csg + while [ -f "$NAME" ]; do NAME=tmp.$$_${RANDOM}.csg done 2>/dev/null echo "$NAME" @@ -282,11 +282,7 @@ if [ "$FORMAT" = amf ]; then id=0 IFS=$'\n' for COLOR in $COLORS; do - IFS=','; set -- $COLOR - R=${1#[} - G=$2 - B=$3 - A=${4%]} + IFS=, read -r R G B A <<<"${COLOR//[\[\] ]/}" echo " ${R// }${G// }${B// }${A// }" (( id++ )) done @@ -323,8 +319,9 @@ elif [ "$FORMAT" = 3mf ]; then # Run from inside TEMPDIR, to support having a Windows-format 3mfmerge binary ( cd "$TEMPDIR" || exit 1 + # shellcheck disable=SC2001 "${DIR_3MFMERGE}"/bin/3mfmerge merged.3mf < \ - <(echo "$COLORS" | sed "s/\$/\.${FORMAT}/") + <(echo "$COLORS" | sed "s/\$/\.${FORMAT}/") ) MERGE_STATUS=$? if ! [ -s "${TEMPDIR}"/merged.3mf ]; then From 11238142f7c9be58c3b4f204ed30980103db2813 Mon Sep 17 00:00:00 2001 From: schorsch3000 Date: Sat, 20 Apr 2024 18:45:42 +0200 Subject: [PATCH 3/6] remove now unnessesary space-stripping Co-authored-by: jschobben <16669720+jschobben@users.noreply.github.com> --- colorscad.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colorscad.sh b/colorscad.sh index 01fee6e..3caa0b1 100755 --- a/colorscad.sh +++ b/colorscad.sh @@ -283,7 +283,7 @@ if [ "$FORMAT" = amf ]; then IFS=$'\n' for COLOR in $COLORS; do IFS=, read -r R G B A <<<"${COLOR//[\[\] ]/}" - echo " ${R// }${G// }${B// }${A// }" + echo " ${R}${G}${B}${A}" (( id++ )) done id=0 From 5cefe3a582aad08dddcff945bc69628c8a0f36b7 Mon Sep 17 00:00:00 2001 From: schorsch3000 Date: Sat, 20 Apr 2024 18:46:02 +0200 Subject: [PATCH 4/6] remove not needed quotes Co-authored-by: jschobben <16669720+jschobben@users.noreply.github.com> --- colorscad.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colorscad.sh b/colorscad.sh index 3caa0b1..7b71711 100755 --- a/colorscad.sh +++ b/colorscad.sh @@ -305,7 +305,7 @@ if [ "$FORMAT" = amf ]; then } > "$OUTPUT" # Strip original current dir prefix, if present, to make message smaller - OUT="${OUTPUT#"${ORIGINAL_PWD}"/}" + OUT=${OUTPUT#"${ORIGINAL_PWD}"/} echo echo "To create a compressed AMF, run:" echo " zip '${OUT}.zip' '$OUT' && mv '${OUT}.zip' '${OUT}'" From 57d29d8b0c190b94cc404dd3537bb1039a30a898 Mon Sep 17 00:00:00 2001 From: Jesse Schobben <16669720+jschobben@users.noreply.github.com> Date: Tue, 16 Apr 2024 12:56:30 +0000 Subject: [PATCH 5/6] ci: run shellcheck on colorscad.sh --- .github/workflows/run-tests.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 5d12cf5..5072c7f 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -179,10 +179,23 @@ jobs: ./run.sh ${{ matrix.test-params }} working-directory: test + code-quality: + name: Check code quality + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: shellcheck colorscad.sh + run: | + shellcheck colorscad.sh + # TODO check tests, 3mfmerge + # Collect results, to be used as combined status check for PRs all-succeeded: name: All tests passed - needs: run-tests + needs: + - run-tests + - code-quality if: always() runs-on: ubuntu-latest steps: From 869e745d588c1f2dca9d243baa2d237abd4cb133 Mon Sep 17 00:00:00 2001 From: Dirk Heilig Date: Sat, 20 Apr 2024 18:51:32 +0200 Subject: [PATCH 6/6] use @jschobben's version that uses mktemp --- colorscad.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/colorscad.sh b/colorscad.sh index 7b71711..f6a1488 100755 --- a/colorscad.sh +++ b/colorscad.sh @@ -155,14 +155,9 @@ INPUT=${INPUT##*/} # Create a temporary, unique .csg file in the input's directory. # It needs to be in the input's directory, because it might contain relative "import" statements. -# On macOS, 'mktemp' cannot create a file with a given extension, so use a workaround. +# On macOS, 'mktemp' does not expand the XXXs because there's a .csg suffix, so use a workaround. INPUT_CSG=$( - set -o noclobber - NAME=tmp.$$_${RANDOM}.csg - while [ -f "$NAME" ]; do - NAME=tmp.$$_${RANDOM}.csg - done 2>/dev/null - echo "$NAME" + until mktemp "tmp.$$_${RANDOM}_XXXXXX.csg"; do sleep 1; done ) [ -z "$INPUT_CSG" ] && exit # Working directory. Use a dir relative to the input dir, because openscad might not have access to