From 27a08b33247fed3102ab4a0fbb26ceed600fcdd0 Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 07:35:37 -0400 Subject: [PATCH 1/8] Use GITHUB_ACTION_PATH variable instead of ${{ github.action_path }} --- action.yaml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/action.yaml b/action.yaml index a31ec82..a7638c4 100644 --- a/action.yaml +++ b/action.yaml @@ -58,7 +58,7 @@ runs: - name: Enable problem-matcher shell: bash run: | - problem_matcher_file="${{ github.action_path }}/.github/problem-matcher-${{ inputs.format }}.json" + problem_matcher_file="$GITHUB_ACTION_PATH/.github/problem-matcher-${{ inputs.format }}.json" if [[ ${{ inputs.disable_matcher }} != "true" && -f "$problem_matcher_file" ]]; then echo "::add-matcher::$problem_matcher_file" fi @@ -75,17 +75,17 @@ runs: scversion="${{ inputs.version }}" baseurl="https://github.com/koalaman/shellcheck/releases/download" - curl -Lso "${{ github.action_path }}/sc.tar.xz" \ + curl -Lso "$GITHUB_ACTION_PATH/sc.tar.xz" \ "${baseurl}/${scversion}/shellcheck-${scversion}.${osvariant}.x86_64.tar.xz" - tar -xf "${{ github.action_path }}/sc.tar.xz" -C "${{ github.action_path }}" - mv "${{ github.action_path }}/shellcheck-${scversion}/shellcheck" \ - "${{ github.action_path }}/shellcheck" + tar -xf "$GITHUB_ACTION_PATH/sc.tar.xz" -C "$GITHUB_ACTION_PATH" + mv "$GITHUB_ACTION_PATH/shellcheck-${scversion}/shellcheck" \ + "$GITHUB_ACTION_PATH/shellcheck" - name: Display shellcheck version shell: bash run: | - "${{ github.action_path }}/shellcheck" --version + "$GITHUB_ACTION_PATH/shellcheck" --version - name: Set options shell: bash @@ -193,13 +193,13 @@ runs: statuscode=0 if [[ -n "${{ inputs.check_together }}" ]]; then - "${{ github.action_path }}/shellcheck" \ + "$GITHUB_ACTION_PATH/shellcheck" \ ${{ steps.options.outputs.options }} \ ${{ steps.filepaths.outputs.filepaths }} || statuscode=$? else for file in ${{ steps.filepaths.outputs.filepaths }}; do echo "::debug::Checking $file" - "${{ github.action_path }}/shellcheck" \ + "$GITHUB_ACTION_PATH/shellcheck" \ ${{ steps.options.outputs.options }} \ "$file" || statuscode=$?; done From 7bc235630b35155fb2e2b6b9d9f816c2d5ebee80 Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 07:39:50 -0400 Subject: [PATCH 2/8] Use $RUNNER_OS instead of ${{ runner.os }} --- action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/action.yaml b/action.yaml index a7638c4..c80923e 100644 --- a/action.yaml +++ b/action.yaml @@ -66,7 +66,7 @@ runs: - name: Download shellcheck shell: bash run: | - if [[ "${{ runner.os }}" == "macOS" ]]; then + if [[ "$RUNNER_OS" == "macOS" ]]; then osvariant="darwin" else osvariant="linux" From 046cc62c783a843c83886c3d71e4a573d3086665 Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 08:21:46 -0400 Subject: [PATCH 3/8] Use ${...} syntax for env vars --- action.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/action.yaml b/action.yaml index c80923e..05b8081 100644 --- a/action.yaml +++ b/action.yaml @@ -58,7 +58,7 @@ runs: - name: Enable problem-matcher shell: bash run: | - problem_matcher_file="$GITHUB_ACTION_PATH/.github/problem-matcher-${{ inputs.format }}.json" + problem_matcher_file="${GITHUB_ACTION_PATH}/.github/problem-matcher-${{ inputs.format }}.json" if [[ ${{ inputs.disable_matcher }} != "true" && -f "$problem_matcher_file" ]]; then echo "::add-matcher::$problem_matcher_file" fi @@ -66,7 +66,7 @@ runs: - name: Download shellcheck shell: bash run: | - if [[ "$RUNNER_OS" == "macOS" ]]; then + if [[ "${RUNNER_OS}" == "macOS" ]]; then osvariant="darwin" else osvariant="linux" @@ -75,17 +75,17 @@ runs: scversion="${{ inputs.version }}" baseurl="https://github.com/koalaman/shellcheck/releases/download" - curl -Lso "$GITHUB_ACTION_PATH/sc.tar.xz" \ + curl -Lso "${GITHUB_ACTION_PATH}/sc.tar.xz" \ "${baseurl}/${scversion}/shellcheck-${scversion}.${osvariant}.x86_64.tar.xz" - tar -xf "$GITHUB_ACTION_PATH/sc.tar.xz" -C "$GITHUB_ACTION_PATH" - mv "$GITHUB_ACTION_PATH/shellcheck-${scversion}/shellcheck" \ - "$GITHUB_ACTION_PATH/shellcheck" + tar -xf "${GITHUB_ACTION_PATH}/sc.tar.xz" -C "${GITHUB_ACTION_PATH}" + mv "${GITHUB_ACTION_PATH}/shellcheck-${scversion}/shellcheck" \ + "${GITHUB_ACTION_PATH}/shellcheck" - name: Display shellcheck version shell: bash run: | - "$GITHUB_ACTION_PATH/shellcheck" --version + "${GITHUB_ACTION_PATH}/shellcheck" --version - name: Set options shell: bash @@ -193,13 +193,13 @@ runs: statuscode=0 if [[ -n "${{ inputs.check_together }}" ]]; then - "$GITHUB_ACTION_PATH/shellcheck" \ + "${GITHUB_ACTION_PATH}/shellcheck" \ ${{ steps.options.outputs.options }} \ ${{ steps.filepaths.outputs.filepaths }} || statuscode=$? else for file in ${{ steps.filepaths.outputs.filepaths }}; do echo "::debug::Checking $file" - "$GITHUB_ACTION_PATH/shellcheck" \ + "${GITHUB_ACTION_PATH}/shellcheck" \ ${{ steps.options.outputs.options }} \ "$file" || statuscode=$?; done From 1abc20e2b7d5b3ba17a0b3db8b6bf39ad3e7fc7e Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 08:30:24 -0400 Subject: [PATCH 4/8] Inject all inputs through environment variables --- action.yaml | 45 +++++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/action.yaml b/action.yaml index 05b8081..5795dfd 100644 --- a/action.yaml +++ b/action.yaml @@ -57,14 +57,19 @@ runs: steps: - name: Enable problem-matcher shell: bash + env: + format: ${{ inputs.format }} + disable_matcher: ${{ inputs.disable_matcher }} run: | - problem_matcher_file="${GITHUB_ACTION_PATH}/.github/problem-matcher-${{ inputs.format }}.json" - if [[ ${{ inputs.disable_matcher }} != "true" && -f "$problem_matcher_file" ]]; then + problem_matcher_file="${GITHUB_ACTION_PATH}/.github/problem-matcher-${format}.json" + if [[ "${disable_matcher}" != "true" && -f "$problem_matcher_file" ]]; then echo "::add-matcher::$problem_matcher_file" fi - name: Download shellcheck shell: bash + env: + scversion: ${{ inputs.version }} run: | if [[ "${RUNNER_OS}" == "macOS" ]]; then osvariant="darwin" @@ -72,7 +77,6 @@ runs: osvariant="linux" fi - scversion="${{ inputs.version }}" baseurl="https://github.com/koalaman/shellcheck/releases/download" curl -Lso "${GITHUB_ACTION_PATH}/sc.tar.xz" \ @@ -90,38 +94,45 @@ runs: - name: Set options shell: bash id: options + env: + severity: ${{ inputs.severity }} + format: ${{ inputs.format }} run: | declare -a options - if [[ -n "${{ inputs.severity }}" ]]; then - options+=("-S ${{ inputs.severity }}") + if [[ -n "${severity}" ]]; then + options+=("-S ${severity}") fi - options+=("--format=${{ inputs.format }}") + options+=("--format=${format}") echo "::set-output name=options::${options[@]}" - name: Gather excluded paths shell: bash id: exclude + env: + ignore: ${{ inputs.ignore }} + ignore_paths: ${{ inputs.ignore_paths }} + ignore_names: ${{ inputs.ignore_names }} run: | declare -a excludes excludes+=("! -path \"*./.git/*\"") excludes+=("! -path \"*.go\"") excludes+=("! -path \"*/mvnw\"") - if [[ -n "${{ inputs.ignore }}" ]]; then + if [[ -n "${ignore}" ]]; then echo "::warning::ignore is deprecated. Please use ignore_paths instead" - for path in ${{ inputs.ignore }}; do + for path in ${ignore}; do echo "::debug:: Adding "$path" to excludes" excludes+=("! -path \"*./$path/*\"") excludes+=("! -path \"*/$path/*\"") done else - for path in ${{ inputs.ignore_paths }}; do + for path in ${ignore_paths}; do echo "::debug:: Adding "$path" to excludes" excludes+=("! -path \"*./$path/*\"") excludes+=("! -path \"*/$path/*\"") done fi - for name in ${{ inputs.ignore_names }}; do + for name in ${ignore_names}; do echo "::debug:: Adding "$name" to excludes" excludes+=("! -name $name") done @@ -130,9 +141,11 @@ runs: - name: Gather additional files shell: bash id: additional + env: + additional_files: ${{ inputs.additional_files }} run: | declare -a files - for file in ${{ inputs.additional_files }}; do + for file in ${additional_files}; do echo "::debug:: Adding "$file" to excludes" files+=("-o -name \"*$file\"") done @@ -141,11 +154,13 @@ runs: - name: Gather base file paths shell: bash id: filepaths + env: + scandir: ${{ inputs.scandir }} run: | declare -a filepaths shebangregex="^#! */[^ ]*/(env *)?[abk]*sh" - for path in $(find "${{ inputs.scandir }}" \ + for path in $(find "${scandir}" \ -type f -type f ${{ steps.exclude.outputs.excludes }} \ '(' \ -name '*.bash' \ @@ -180,7 +195,7 @@ runs: filepaths+=("$path"); done - for file in $(find "${{ inputs.scandir }}" ${{ steps.exclude.outputs.excludes }} -type f ! -name '*.*' -perm /111 -print); do + for file in $(find "${scandir}" ${{ steps.exclude.outputs.excludes }} -type f ! -name '*.*' -perm /111 -print); do head -n1 "$file" | grep -Eqs "$shebangregex" || continue filepaths+=("$file"); done @@ -189,10 +204,12 @@ runs: - name: Run the file check id: check shell: bash + env: + check_together: ${{ inputs.check_together }} run: | statuscode=0 - if [[ -n "${{ inputs.check_together }}" ]]; then + if [[ -n "${check_together}" ]]; then "${GITHUB_ACTION_PATH}/shellcheck" \ ${{ steps.options.outputs.options }} \ ${{ steps.filepaths.outputs.filepaths }} || statuscode=$? From 9efe230ff6caa84acde04a61fc235c4ebabe009a Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 08:38:13 -0400 Subject: [PATCH 5/8] Pass all step outputs as env vars --- action.yaml | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/action.yaml b/action.yaml index 5795dfd..29ae572 100644 --- a/action.yaml +++ b/action.yaml @@ -156,12 +156,14 @@ runs: id: filepaths env: scandir: ${{ inputs.scandir }} + exclude_args: ${{ steps.exclude.outputs.excludes }} + additional_file_args: ${{ steps.additional.outputs.files }} run: | declare -a filepaths shebangregex="^#! */[^ ]*/(env *)?[abk]*sh" for path in $(find "${scandir}" \ - -type f -type f ${{ steps.exclude.outputs.excludes }} \ + -type f -type f ${exclude_args} \ '(' \ -name '*.bash' \ -o -name '.bashrc' \ @@ -189,13 +191,13 @@ runs: -o -path '*/.profile' \ -o -path '*/profile' \ -o -name '*.shlib' \ - ${{ steps.additional.outputs.files }} \ + ${additional_file_args} \ ')'\ -print); do filepaths+=("$path"); done - for file in $(find "${scandir}" ${{ steps.exclude.outputs.excludes }} -type f ! -name '*.*' -perm /111 -print); do + for file in $(find "${scandir}" ${exclude_args} -type f ! -name '*.*' -perm /111 -print); do head -n1 "$file" | grep -Eqs "$shebangregex" || continue filepaths+=("$file"); done @@ -206,18 +208,20 @@ runs: shell: bash env: check_together: ${{ inputs.check_together }} + shellcheck_options: ${{ steps.options.outputs.options }} + filepaths: ${{ steps.filepaths.outputs.filepaths }} run: | statuscode=0 if [[ -n "${check_together}" ]]; then "${GITHUB_ACTION_PATH}/shellcheck" \ - ${{ steps.options.outputs.options }} \ - ${{ steps.filepaths.outputs.filepaths }} || statuscode=$? + ${shellcheck_options} \ + ${filepaths} || statuscode=$? else - for file in ${{ steps.filepaths.outputs.filepaths }}; do + for file in ${filepaths}; do echo "::debug::Checking $file" "${GITHUB_ACTION_PATH}/shellcheck" \ - ${{ steps.options.outputs.options }} \ + ${shellcheck_options} \ "$file" || statuscode=$?; done fi @@ -226,10 +230,15 @@ runs: - name: Print information shell: bash + env: + filepaths: ${{ steps.filepaths.outputs.filepaths }} + exclude_args: ${{ steps.exclude.outputs.excludes }} + shellcheck_options: ${{ steps.options.outputs.options }} + statuscode: ${{ steps.check.outputs.statuscode }} run: | - echo "Files: ${{steps.filepaths.outputs.filepaths}}" - echo "Excluded: ${{ steps.exclude.outputs.excludes }}" - echo "Options: ${{ steps.options.outputs.options }}" - echo "Status code: ${{steps.check.outputs.statuscode}}" + echo "Files: ${filepaths}" + echo "Excluded: ${exclude_args}" + echo "Options: ${shellcheck_options}" + echo "Status code: ${statuscode}" - exit ${{steps.check.outputs.statuscode}} + exit "${statuscode}" From 1b56179db781ac272f174b4aa2fbd554f78d9940 Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 1 Sep 2022 11:10:26 -0400 Subject: [PATCH 6/8] Revert to using github.action_path and runner.os expr --- action.yaml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/action.yaml b/action.yaml index 29ae572..f2f26e8 100644 --- a/action.yaml +++ b/action.yaml @@ -61,7 +61,7 @@ runs: format: ${{ inputs.format }} disable_matcher: ${{ inputs.disable_matcher }} run: | - problem_matcher_file="${GITHUB_ACTION_PATH}/.github/problem-matcher-${format}.json" + problem_matcher_file="${{ github.action_path }}/.github/problem-matcher-${format}.json" if [[ "${disable_matcher}" != "true" && -f "$problem_matcher_file" ]]; then echo "::add-matcher::$problem_matcher_file" fi @@ -71,7 +71,7 @@ runs: env: scversion: ${{ inputs.version }} run: | - if [[ "${RUNNER_OS}" == "macOS" ]]; then + if [[ "${{ runner.os }}" == "macOS" ]]; then osvariant="darwin" else osvariant="linux" @@ -79,17 +79,17 @@ runs: baseurl="https://github.com/koalaman/shellcheck/releases/download" - curl -Lso "${GITHUB_ACTION_PATH}/sc.tar.xz" \ + curl -Lso "${{ github.action_path }}/sc.tar.xz" \ "${baseurl}/${scversion}/shellcheck-${scversion}.${osvariant}.x86_64.tar.xz" - tar -xf "${GITHUB_ACTION_PATH}/sc.tar.xz" -C "${GITHUB_ACTION_PATH}" - mv "${GITHUB_ACTION_PATH}/shellcheck-${scversion}/shellcheck" \ - "${GITHUB_ACTION_PATH}/shellcheck" + tar -xf "${{ github.action_path }}/sc.tar.xz" -C "${{ github.action_path }}" + mv "${{ github.action_path }}/shellcheck-${scversion}/shellcheck" \ + "${{ github.action_path }}/shellcheck" - name: Display shellcheck version shell: bash run: | - "${GITHUB_ACTION_PATH}/shellcheck" --version + "${{ github.action_path }}/shellcheck" --version - name: Set options shell: bash @@ -214,13 +214,13 @@ runs: statuscode=0 if [[ -n "${check_together}" ]]; then - "${GITHUB_ACTION_PATH}/shellcheck" \ + "${{ github.action_path }}/shellcheck" \ ${shellcheck_options} \ ${filepaths} || statuscode=$? else for file in ${filepaths}; do echo "::debug::Checking $file" - "${GITHUB_ACTION_PATH}/shellcheck" \ + "${{ github.action_path }}/shellcheck" \ ${shellcheck_options} \ "$file" || statuscode=$?; done From 3c8c1f0b6fa46d2e37e709e0b3d28ebcad1a4820 Mon Sep 17 00:00:00 2001 From: Boris Bera Date: Thu, 8 Sep 2022 09:47:25 -0400 Subject: [PATCH 7/8] Use output directly for status code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Joakim Sørensen --- action.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/action.yaml b/action.yaml index 280cb72..59567a2 100644 --- a/action.yaml +++ b/action.yaml @@ -234,6 +234,4 @@ runs: - name: Exit action shell: bash - env: - statuscode: ${{ steps.check.outputs.statuscode }} - run: exit "${statuscode}" + run: exit ${{steps.check.outputs.statuscode}} From f2b01109f4af83af2b6e050294956f112c6d4209 Mon Sep 17 00:00:00 2001 From: "bbera@coveo.com" Date: Thu, 8 Sep 2022 12:38:31 -0400 Subject: [PATCH 8/8] Adjust escaping and globbing to work with env vars passing Now that we're passing in values through environment variables, we don't need the extra layer of escaping that was present in the code because the shell script was getting templated by GH actions. Also, since this layer is now gone, we stop the shell from globbing in the `check` step to avoid globbing the find arguments. --- action.yaml | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/action.yaml b/action.yaml index 59567a2..0374f4d 100644 --- a/action.yaml +++ b/action.yaml @@ -117,23 +117,23 @@ runs: declare -a excludes set -f # temporarily disable globbing so that globs in input aren't expanded - excludes+=("! -path \"*./.git/*\"") - excludes+=("! -path \"*.go\"") - excludes+=("! -path \"*/mvnw\"") + excludes+=("! -path *./.git/*") + excludes+=("! -path *.go") + excludes+=("! -path */mvnw") if [[ -n "${ignore}" ]]; then echo "::warning::ignore is deprecated. Please use ignore_paths instead" for path in ${ignore}; do echo "::debug:: Adding '$path' to excludes" - excludes+=("! -path \"*./$path/*\"") - excludes+=("! -path \"*/$path/*\"") - excludes+=("! -path \"$path\"") + excludes+=("! -path *./$path/*") + excludes+=("! -path */$path/*") + excludes+=("! -path $path") done else for path in ${ignore_paths}; do echo "::debug:: Adding '$path' to excludes" - excludes+=("! -path \"*./$path/*\"") - excludes+=("! -path \"*/$path/*\"") - excludes+=("! -path \"$path\"") + excludes+=("! -path *./$path/*") + excludes+=("! -path */$path/*") + excludes+=("! -path $path") done fi @@ -153,8 +153,8 @@ runs: run: | declare -a files for file in ${additional_files}; do - echo "::debug:: Adding '$file' to excludes" - files+=("-o -name \"*$file\"") + echo "::debug:: Adding '$file' to additional files" + files+=("-o -name *$file") done echo "::set-output name=files::${files[@]}" @@ -172,6 +172,8 @@ runs: declare -a filepaths shebangregex="^#! */[^ ]*/(env *)?[abk]*sh" + set -f # temporarily disable globbing so that globs in inputs aren't expanded + while IFS= read -r -d '' file; do filepaths+=("$file") done < <(find "${scandir}" \ @@ -232,6 +234,8 @@ runs: echo "::set-output name=filepaths::${filepaths[@]}" echo "::set-output name=statuscode::$statuscode" + set +f # re-enable globbing + - name: Exit action shell: bash run: exit ${{steps.check.outputs.statuscode}}