From b1c82a32a09e564655f3c8896a94c268ac2dd868 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 17 Dec 2021 10:07:26 -0800 Subject: [PATCH 1/3] Add tests for PATH suggestion output of installation script The "template" installation script stored in this repository prints advice to the user about adding the installation to their PATH environment variable. This advice is adjusted according to whether a previous installation was found in the PATH and according to the path(s) of their installation(s) so it is fairly complex. For this reason, it will be good to have a check in the "Test install script" CI workflow to ensure the output is as expected. --- .github/workflows/test-install-script.yml | 75 +++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/.github/workflows/test-install-script.yml b/.github/workflows/test-install-script.yml index ed98241b..0dac2193 100644 --- a/.github/workflows/test-install-script.yml +++ b/.github/workflows/test-install-script.yml @@ -191,3 +191,78 @@ jobs: shell: bash run: | "${PWD}/bin/${{ env.TOOL_NAME }}" --version | grep "^nightly-" + + path-suggestions: + needs: configure + strategy: + fail-fast: false + + matrix: + os: + - ubuntu-latest + - windows-latest + - macos-latest + + runs-on: ${{ matrix.os }} + + steps: + - name: Set install path environment variables + shell: bash + run: | + # See: https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable + FIRST_INSTALLATION_FOLDER="first-installation-folder" + echo "FIRST_INSTALLATION_FOLDER=${FIRST_INSTALLATION_FOLDER}" >> "$GITHUB_ENV" + echo "FIRST_BINDIR=${{ runner.temp }}/${FIRST_INSTALLATION_FOLDER}" >> "$GITHUB_ENV" + SECOND_INSTALLATION_FOLDER="second-installation-folder" + echo "SECOND_INSTALLATION_FOLDER=${SECOND_INSTALLATION_FOLDER}" >> "$GITHUB_ENV" + echo "SECOND_BINDIR=${{ runner.temp }}/${SECOND_INSTALLATION_FOLDER}" >> "$GITHUB_ENV" + + - name: Download script artifact + uses: actions/download-artifact@v2 + with: + name: ${{ env.SCRIPT_ARTIFACT_NAME }} + + - name: Make script executable + run: chmod u+x "${{ github.workspace }}/${{ env.SCRIPT_NAME }}" + + - name: Check script output without previous installation in PATH + shell: sh + env: + BINDIR: ${{ env.FIRST_BINDIR }} + run: | + mkdir -p "${{ env.BINDIR }}" + "${{ github.workspace }}/${{ env.SCRIPT_NAME }}" | \ + grep \ + -F \ + '${{ env.TOOL_NAME }} not found. You might want to add "${{ env.FIRST_BINDIR }}" to your $PATH' + + - name: Add first installation to PATH + shell: bash + run: | + # See: https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#adding-a-system-path + echo "${{ env.FIRST_BINDIR }}" >> "$GITHUB_PATH" + + - name: Check script output with previous installation in PATH (non-Windows) + if: runner.os != 'Windows' + shell: sh + env: + BINDIR: ${{ env.SECOND_BINDIR }} + run: | + mkdir -p "${{ env.BINDIR }}" + "${{ github.workspace }}/${{ env.SCRIPT_NAME }}" | \ + grep \ + -F \ + '${{ env.TOOL_NAME }} was found at ${{ env.FIRST_BINDIR }}/${{ env.TOOL_NAME }}. Please prepend "${{ env.BINDIR }}" to your $PATH' + + # ${{ runner.temp }} is a Windows style path on the windows runner, but the script output uses the equivalent POSIX style path. + # So a regex is used for the output check on Windows. + - name: Check script output with previous installation in PATH (Windows) + if: runner.os == 'Windows' + shell: sh + env: + BINDIR: ${{ env.SECOND_BINDIR }} + run: | + mkdir -p "${{ env.BINDIR }}" + "${{ github.workspace }}/${{ env.SCRIPT_NAME }}" | \ + grep \ + '${{ env.TOOL_NAME }} was found at .*/${{ env.FIRST_INSTALLATION_FOLDER }}/${{ env.TOOL_NAME }}\. Please prepend ".*/${{ env.SECOND_INSTALLATION_FOLDER }}" to your \$PATH' From 45b0ef01da5e5baf901840e0c15bd47e7a582d61 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 17 Dec 2021 10:20:51 -0800 Subject: [PATCH 2/3] Fix install script's check for previous installation The "template" installation script stored in this repository checks for an existing installation in the PATH in order to provide appropriate advice to the user about adding the installation to their their PATH environment variable. This check is done using `command -v`. It turns out that the exit status is shell dependent in the event the command is not found, so that it might be either 1 or 127 depending on the user's system. The script previously assumed that the exit status would be 1 when the command was not found in PATH, which resulted in spurious advice under these conditions: ``` An existing arduino-lint was found at . Please prepend "/home/runner/work/tooling-project-assets/tooling-project-assets/bin" to your $PATH or remove the existing one. ``` It seems safest to invert the logic so that the advice about an existing installation in PATH is only printed when one was found. --- other/installation-script/install.sh | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/other/installation-script/install.sh b/other/installation-script/install.sh index 8ebe1aa5..be326f5b 100755 --- a/other/installation-script/install.sh +++ b/other/installation-script/install.sh @@ -187,18 +187,19 @@ bye() { testVersion() { set +e EXECUTABLE_PATH="$(command -v $PROJECT_NAME)" - if [ "$?" = "1" ]; then - # $PATH is intentionally a literal in this message. - # shellcheck disable=SC2016 - echo "$PROJECT_NAME not found. You might want to add \"$EFFECTIVE_BINDIR\" to your "'$PATH' - else + if [ "$?" = "0" ]; then # Convert to resolved, absolute paths before comparison EXECUTABLE_REALPATH="$(cd -- "$(dirname -- "$EXECUTABLE_PATH")" && pwd -P)" EFFECTIVE_BINDIR_REALPATH="$(cd -- "$EFFECTIVE_BINDIR" && pwd -P)" if [ "$EXECUTABLE_REALPATH" != "$EFFECTIVE_BINDIR_REALPATH" ]; then + # $PATH is intentionally a literal in this message. # shellcheck disable=SC2016 echo "An existing $PROJECT_NAME was found at $EXECUTABLE_PATH. Please prepend \"$EFFECTIVE_BINDIR\" to your "'$PATH'" or remove the existing one." fi + else + # $PATH is intentionally a literal in this message. + # shellcheck disable=SC2016 + echo "$PROJECT_NAME not found. You might want to add \"$EFFECTIVE_BINDIR\" to your "'$PATH' fi set -e From 1f641f6f51a5e1dd255bd7d32a073118bbdcb132 Mon Sep 17 00:00:00 2001 From: per1234 Date: Fri, 17 Dec 2021 11:42:45 -0800 Subject: [PATCH 3/3] Check exit status directly in install script The "template" installation script stored in this repository checks for an existing installation in the PATH in order to provide appropriate advice to the user about adding the installation to their their PATH environment variable. This check is done by checking the exit status of `command -v`. The previous use of `$?` for that check was unnecessary since the command itself can be checked. This change is required for compliance with ShellCheck rule SC2181: https://github.com/koalaman/shellcheck/wiki/SC2181 --- other/installation-script/install.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/other/installation-script/install.sh b/other/installation-script/install.sh index be326f5b..213b4878 100755 --- a/other/installation-script/install.sh +++ b/other/installation-script/install.sh @@ -186,8 +186,7 @@ bye() { testVersion() { set +e - EXECUTABLE_PATH="$(command -v $PROJECT_NAME)" - if [ "$?" = "0" ]; then + if EXECUTABLE_PATH="$(command -v $PROJECT_NAME)"; then # Convert to resolved, absolute paths before comparison EXECUTABLE_REALPATH="$(cd -- "$(dirname -- "$EXECUTABLE_PATH")" && pwd -P)" EFFECTIVE_BINDIR_REALPATH="$(cd -- "$EFFECTIVE_BINDIR" && pwd -P)"