From 2bc43ab42d4c22f7b728e34bf38315e683d84cdf Mon Sep 17 00:00:00 2001 From: tomeichlersmith Date: Tue, 5 Nov 2024 13:08:44 -0600 Subject: [PATCH 1/5] use denv v1.1.0's quiet no-overwrite init flags this simplifies just init for newer denv and avoids a confusing error message that is usually just passed by --- justfile | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/justfile b/justfile index 305d4b760..5e4e8ce5c 100644 --- a/justfile +++ b/justfile @@ -102,18 +102,28 @@ debug config_py *ARGS: # initialize a containerized development environment init: #!/usr/bin/env sh - # while setting the denv_workspace is helpful for other - # commands that can assume the denv is already initialized, - # we need to unset this environment variable to make sure - # the test is done appropriately. - # just makes sure this recipe runs from the directory of - # the justfile so we know we are in the correct location. - unset denv_workspace - if denv check --workspace --quiet; then - echo "\033[32mWorkspace already initialized.\033[0m" - denv config print + denv_major=$(denv version | sed 's/denv v//' | cut -f 1 -d.) + denv_minor=$(denv version | set 's/denv v//' | cut -f 2 -d.) + if [ "${denv_major}" -lt 1 ] || [ "${denv_minor}" -lt "1" ]; then + # denv v1.0.X or earlier, manually check for workspace + # which may print a confusing error from denv when no workspace is found + unset denv_workspace + # while setting the denv_workspace is helpful for other + # commands that can assume the denv is already initialized, + # we need to unset this environment variable to make sure + # the test is done appropriately. + # just makes sure this recipe runs from the directory of + # the justfile so we know we are in the correct location. + if denv check --workspace --quiet; then + echo "\033[32mWorkspace already initialized.\033[0m" + denv config print + else + denv init --clean-env --name ldmx ldmx/dev:latest ${LDMX_BASE} + fi else - denv init --clean-env --name ldmx ldmx/dev:latest ${LDMX_BASE} + # denv v1.1.0 and later has updated denv init to allow us + # to avoid overwriting quietly + denv init --clean-env --no-over --no-mkdir --name ldmx ldmx/dev:latest ${LDMX_BASE} fi # check that the necessary programs for running ldmx-sw are present From f4375531bbbc8918ebc1f98ccc595b591e694aff Mon Sep 17 00:00:00 2001 From: tomeichlersmith Date: Fri, 22 Nov 2024 15:49:44 -0600 Subject: [PATCH 2/5] patch up new init recipe typo was preventing getting the minor version causing that check to always fail. also putting quotes around all the environment variables now as well. --- justfile | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/justfile b/justfile index 5e4e8ce5c..6cc659760 100644 --- a/justfile +++ b/justfile @@ -103,8 +103,8 @@ debug config_py *ARGS: init: #!/usr/bin/env sh denv_major=$(denv version | sed 's/denv v//' | cut -f 1 -d.) - denv_minor=$(denv version | set 's/denv v//' | cut -f 2 -d.) - if [ "${denv_major}" -lt 1 ] || [ "${denv_minor}" -lt "1" ]; then + denv_minor=$(denv version | sed 's/denv v//' | cut -f 2 -d.) + if [ "${denv_major}" -lt "1" ] || [ "${denv_minor}" -lt "1" ]; then # denv v1.0.X or earlier, manually check for workspace # which may print a confusing error from denv when no workspace is found unset denv_workspace @@ -116,15 +116,15 @@ init: # the justfile so we know we are in the correct location. if denv check --workspace --quiet; then echo "\033[32mWorkspace already initialized.\033[0m" - denv config print else - denv init --clean-env --name ldmx ldmx/dev:latest ${LDMX_BASE} + denv init --clean-env --name ldmx ldmx/dev:latest "${LDMX_BASE}" fi else # denv v1.1.0 and later has updated denv init to allow us # to avoid overwriting quietly - denv init --clean-env --no-over --no-mkdir --name ldmx ldmx/dev:latest ${LDMX_BASE} + denv init --clean-env --no-over --no-mkdir --name ldmx ldmx/dev:latest "${LDMX_BASE}" fi + denv config print # check that the necessary programs for running ldmx-sw are present check: From 05cec345e808a4450c7a8850329f7d344721b3cd Mon Sep 17 00:00:00 2001 From: tomeichlersmith Date: Fri, 22 Nov 2024 15:51:45 -0600 Subject: [PATCH 3/5] have shell leave early if error --- justfile | 1 + 1 file changed, 1 insertion(+) diff --git a/justfile b/justfile index 6cc659760..0f9d7e92b 100644 --- a/justfile +++ b/justfile @@ -102,6 +102,7 @@ debug config_py *ARGS: # initialize a containerized development environment init: #!/usr/bin/env sh + set -eu denv_major=$(denv version | sed 's/denv v//' | cut -f 1 -d.) denv_minor=$(denv version | sed 's/denv v//' | cut -f 2 -d.) if [ "${denv_major}" -lt "1" ] || [ "${denv_minor}" -lt "1" ]; then From d4930c03b8d393689985b577c1c74b369bdc23d5 Mon Sep 17 00:00:00 2001 From: tomeichlersmith Date: Fri, 22 Nov 2024 15:52:30 -0600 Subject: [PATCH 4/5] patch current and add new shellcheck recipe new shellcheck recipe is focused on shellcheck'ing the recipes within the justfile itself. As they get more complicated, it is helpful to make sure that we maintain good shell style. Don't leave shellchecks early on an error so that we can make sure that the temporary files that are spawned are removed. --- justfile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/justfile b/justfile index 0f9d7e92b..d753b35a1 100644 --- a/justfile +++ b/justfile @@ -172,10 +172,19 @@ format-just: # check the scripts for common errors and bugs shellcheck: #!/usr/bin/env sh - set -exu + set -x format_list=$(mktemp) git ls-tree -r HEAD | awk '{ if ($1 == 100755 || $4 ~ /\.sh/) print $4 }' > ${format_list} shellcheck --severity style --shell sh $(cat ${format_list}) + rm "${format_list}" + +# check a script recipe also using shellcheck +shellcheck-recipe RECIPE: + #!/usr/bin/env sh + source=$(mktemp) + just -n {{ RECIPE }} 2> "${source}" + shellcheck --severity style --shell sh "${source}" + rm "${source}" # below are the mimics of ldmx # we could think about removing them if folks are happy with committing to the From 19ea462bb482766a9b765ef62dd4d609d47e7d99 Mon Sep 17 00:00:00 2001 From: tomeichlersmith Date: Fri, 22 Nov 2024 16:00:33 -0600 Subject: [PATCH 5/5] use xargs instead of cat allows possibility of white-space characters in filepaths --- justfile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/justfile b/justfile index d753b35a1..6fe50562e 100644 --- a/justfile +++ b/justfile @@ -174,8 +174,10 @@ shellcheck: #!/usr/bin/env sh set -x format_list=$(mktemp) - git ls-tree -r HEAD | awk '{ if ($1 == 100755 || $4 ~ /\.sh/) print $4 }' > ${format_list} - shellcheck --severity style --shell sh $(cat ${format_list}) + git ls-tree -r HEAD | awk '{ if ($1 == 100755 || $4 ~ /\.sh/) print $4 }' \ + > "${format_list}" + xargs --arg-file="${format_list}" \ + shellcheck --severity style --shell sh rm "${format_list}" # check a script recipe also using shellcheck