fix(workflows): pass changed files via bash array (mdn#40648)

argl · web-flow · commit e4eb45be84b6 · 2025-08-11T17:52:46.000+02:00
* Use a bash array instead of a string to convey the file list to linters

* refactor xargs usage

* removed debug option from step

* removed commented code

* fix file existence text
diff --git a/.github/workflows/pr-check-lint_content.yml b/.github/workflows/pr-check-lint_content.yml
@@ -34,21 +34,34 @@ jobs:
         run: |
           # Use the GitHub API to get the list of changed files
           # documentation: https://docs.github.com/rest/commits/commits#compare-two-commits
-          DIFF_DOCUMENTS=$(gh api repos/{owner}/{repo}/compare/${BASE_SHA}...${HEAD_SHA} \
-            --jq '.files | .[] | select(.status|IN("added", "modified", "renamed", "copied", "changed")) | .filename')
-          # filter out files that are not markdown
-          DIFF_DOCUMENTS=$(echo "${DIFF_DOCUMENTS}" | egrep -i "^files/.*\.md$" | xargs)
-          echo "DIFF_DOCUMENTS=${DIFF_DOCUMENTS}" >> "$GITHUB_OUTPUT"
+
+          # Get files as newline-separated list
+          FILTERED_FILES=$(gh api repos/{owner}/{repo}/compare/${BASE_SHA}...${HEAD_SHA} \
+            --jq '.files | .[] | select(.status|IN("added", "modified", "renamed", "copied", "changed")) | .filename' | \
+            egrep -i "^files/.*\.md$")
+
+          # Store as multiline output
+          EOF="$(openssl rand -hex 8)"
+          echo "DIFF_DOCUMENTS<<${EOF}" >> "$GITHUB_OUTPUT"
+          echo "${FILTERED_FILES}" >> "$GITHUB_OUTPUT"
+          echo "${EOF}" >> "$GITHUB_OUTPUT"
+
+          # Also set a simple flag for whether we have files
+          if [ -n "${FILTERED_MD_FILES// /}" ]; then  # Remove all spaces and check if anything remains
+            echo "HAS_FILES=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "HAS_FILES=false" >> "$GITHUB_OUTPUT"
+          fi
 
       - name: Checkout HEAD
-        if: steps.check.outputs.DIFF_DOCUMENTS
+        if: steps.check.outputs.HAS_FILES == 'true'
         uses: actions/checkout@v4
         with:
           ref: ${{ github.event.pull_request.head.sha }}
           path: pr_head
 
       - name: Get changed content from HEAD
-        if: steps.check.outputs.DIFF_DOCUMENTS
+        if: steps.check.outputs.HAS_FILES == 'true'
         run: |
           git config --global user.email "108879845+mdn-bot@users.noreply.github.com"
           git config --global user.name "mdn-bot"
@@ -64,63 +77,69 @@ jobs:
           git commit -m "Code from PR head"
 
       - name: Setup Node.js environment
-        if: steps.check.outputs.DIFF_DOCUMENTS
+        if: steps.check.outputs.HAS_FILES == 'true'
         uses: actions/setup-node@v4
         with:
           node-version-file: ".nvmrc"
           cache: yarn
 
       - name: Install all yarn packages
-        if: steps.check.outputs.DIFF_DOCUMENTS
+        if: steps.check.outputs.HAS_FILES == 'true'
         run: yarn --frozen-lockfile
         env:
           # https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Lint and format markdown files
         id: lint
-        if: steps.check.outputs.DIFF_DOCUMENTS
+        if: steps.check.outputs.HAS_FILES == 'true'
         env:
           DIFF_DOCUMENTS: ${{ steps.check.outputs.DIFF_DOCUMENTS }}
         run: |
           # Generate random delimiter
           # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings
           EOF="$(openssl rand -hex 8)"
 
-          files_to_lint="$DIFF_DOCUMENTS"
+          # The DIFF_DOCUMENTS env var contains the clean newline-separated list
+          # Read into array, one filename per line
+          readarray -t files_to_lint <<< "$DIFF_DOCUMENTS"
+
+          # Debug: show what we got
+          printf "Files to process (%d files):\n" "${#files_to_lint[@]}"
+          printf "'%s'\n" "${files_to_lint[@]}"
 
           echo "crlf line ending check"
           CRLF_FAILED=true
-          CRLF_LOG=$(git ls-files --eol ${files_to_lint} | grep -E 'w/(mixed|crlf)') || CRLF_FAILED=false
+          CRLF_LOG=$(git ls-files --eol "${files_to_lint[@]}" | grep -E 'w/(mixed|crlf)') || CRLF_FAILED=false
           echo "CRLF_LOG<<${EOF}" >> "$GITHUB_OUTPUT"
           echo "${CRLF_LOG}" >> "$GITHUB_OUTPUT"
           echo "${EOF}" >> "$GITHUB_OUTPUT"
           echo "CRLF_FAILED=${CRLF_FAILED}" >> "$GITHUB_OUTPUT"
 
           echo "Running markdownlint --fix"
           MD_LINT_FAILED=false
-          MD_LINT_LOG=$(yarn markdownlint-cli2 --fix ${files_to_lint} 2>&1) || MD_LINT_FAILED=true
+          MD_LINT_LOG=$(yarn markdownlint-cli2 --fix "${files_to_lint[@]}" 2>&1) || MD_LINT_FAILED=true
           echo "MD_LINT_LOG<<${EOF}" >> "$GITHUB_OUTPUT"
           echo "${MD_LINT_LOG}" >> "$GITHUB_OUTPUT"
           echo "${EOF}" >> "$GITHUB_OUTPUT"
           echo "MD_LINT_FAILED=${MD_LINT_FAILED}" >> "$GITHUB_OUTPUT"
 
           echo "Linting front-matter"
           FM_LINT_FAILED=false
-          FM_LINT_LOG=$(node scripts/front-matter_linter.js --fix true ${files_to_lint} 2>&1) || FM_LINT_FAILED=true
+          FM_LINT_LOG=$(node scripts/front-matter_linter.js --fix true "${files_to_lint[@]}" 2>&1) || FM_LINT_FAILED=true
           echo "FM_LINT_LOG<<${EOF}" >> "$GITHUB_OUTPUT"
           echo "${FM_LINT_LOG}" >> "$GITHUB_OUTPUT"
           echo "${EOF}" >> "$GITHUB_OUTPUT"
           echo "FM_LINT_FAILED=${FM_LINT_FAILED}" >> "$GITHUB_OUTPUT"
 
           echo "Running Prettier"
           PRETTIER_FAILED=false
-          PRETTIER_LOG=$(yarn prettier --check ${files_to_lint} 2>&1) || PRETTIER_FAILED=true
+          PRETTIER_LOG=$(yarn prettier --check "${files_to_lint[@]}" 2>&1) || PRETTIER_FAILED=true
           echo "PRETTIER_LOG<<${EOF}" >> "$GITHUB_OUTPUT"
           echo "${PRETTIER_LOG}" >> "$GITHUB_OUTPUT"
           echo "${EOF}" >> "$GITHUB_OUTPUT"
           echo "PRETTIER_FAILED=${PRETTIER_FAILED}" >> "$GITHUB_OUTPUT"
-          yarn prettier -w ${files_to_lint}
+          yarn prettier -w "${files_to_lint[@]}"
 
           if [[ -n $(git diff) ]]; then
             echo "FILES_MODIFIED=true" >> "$GITHUB_OUTPUT"
diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml
@@ -35,37 +35,64 @@ jobs:
         id: check
         run: |
           # Use the GitHub API to get the list of changed files
-          # documentation: https://docs.github.com/rest/commits/commits#compare-two-commits
           DIFF_DOCUMENTS=$(gh api repos/{owner}/{repo}/compare/${BASE_SHA}...${HEAD_SHA} \
             --jq '.files | .[] | select(.status|IN("added", "modified", "renamed", "copied", "changed")) | .filename')
 
-          # filter out files that are not markdown files
-          GIT_DIFF_CONTENT=$(echo "${DIFF_DOCUMENTS}" | egrep -i "^files/.*\.(md)$" | xargs)
-          echo "GIT_DIFF_CONTENT=${GIT_DIFF_CONTENT}" >> "$GITHUB_OUTPUT"
-
-          # filter out files that are not attachments
-          GIT_DIFF_FILES=$(echo "${DIFF_DOCUMENTS}" | egrep -i "^files/.*\.(png|jpeg|jpg|gif|svg|webp)$" | xargs)
-          echo "GIT_DIFF_FILES=${GIT_DIFF_FILES}" >> "$GITHUB_OUTPUT"
+          FILTERED_MD_FILES=$(echo "$DIFF_DOCUMENTS" | egrep -i "^files/.*\.md$" || true)
+          FILTERED_IMAGE_FILES=$(echo "$DIFF_DOCUMENTS" | egrep -i "^files/.*\.(png|jpeg|jpg|gif|svg|webp)$" || true)
+
+          HAS_MD_FILES=false
+          HAS_IMAGE_FILES=false
+
+          # Check if we actually have content (not just empty strings)
+          if [ -n "${FILTERED_MD_FILES// /}" ]; then  # Remove all spaces and check if anything remains
+            HAS_MD_FILES=true
+            EOF_MD="$(openssl rand -hex 8)"
+            echo "GIT_DIFF_CONTENT<<${EOF_MD}" >> "$GITHUB_OUTPUT"
+            echo "${FILTERED_MD_FILES}" >> "$GITHUB_OUTPUT"
+            echo "${EOF_MD}" >> "$GITHUB_OUTPUT"
+          else
+            echo "GIT_DIFF_CONTENT=" >> "$GITHUB_OUTPUT"
+          fi
+
+          if [ -n "${FILTERED_IMAGE_FILES// /}" ]; then  # Remove all spaces and check if anything remains
+            HAS_IMAGE_FILES=true
+            EOF_IMG="$(openssl rand -hex 8)"
+            echo "GIT_DIFF_FILES<<${EOF_IMG}" >> "$GITHUB_OUTPUT"
+            echo "${FILTERED_IMAGE_FILES}" >> "$GITHUB_OUTPUT"
+            echo "${EOF_IMG}" >> "$GITHUB_OUTPUT"
+          else
+            echo "GIT_DIFF_FILES=" >> "$GITHUB_OUTPUT"
+          fi
+
+          # Set the boolean outputs
+          echo "HAS_MD_FILES=${HAS_MD_FILES}" >> "$GITHUB_OUTPUT"
+          echo "HAS_IMAGE_FILES=${HAS_IMAGE_FILES}" >> "$GITHUB_OUTPUT"
+          if [ "${HAS_MD_FILES}" = "true" ] || [ "${HAS_IMAGE_FILES}" = "true" ]; then
+            echo "HAS_FILES=true" >> "$GITHUB_OUTPUT"
+          else
+            echo "HAS_FILES=false" >> "$GITHUB_OUTPUT"
+          fi
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Setup Node.js environment
-        if: steps.check.outputs.GIT_DIFF_CONTENT || steps.check.outputs.GIT_DIFF_FILES
+        if: steps.check.outputs.HAS_FILES == 'true'
         uses: actions/setup-node@v4
         with:
           node-version-file: ".nvmrc"
           cache: yarn
 
       - name: Install all yarn packages
-        if: steps.check.outputs.GIT_DIFF_CONTENT || steps.check.outputs.GIT_DIFF_FILES
+        if: steps.check.outputs.HAS_FILES == 'true'
         run: yarn --frozen-lockfile
         env:
           # https://github.com/microsoft/vscode-ripgrep#github-api-limit-note
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
       - name: Build changed content
         id: build-content
-        if: steps.check.outputs.GIT_DIFF_CONTENT
+        if: steps.check.outputs.HAS_MD_FILES == 'true'
         env:
           GIT_DIFF_CONTENT: ${{ steps.check.outputs.GIT_DIFF_CONTENT }}
 
@@ -109,8 +136,16 @@ jobs:
           # you don't need that script as a writer. It's only used in CI
           # and it can't use the default CONTENT_ROOT that gets set in
           # package.json.
-          ARGS=$(echo $GIT_DIFF_CONTENT | sed -E -e "s#(^| )files#\1-f $PWD/files#g")
-          yarn rari build --no-basic --json-issues --data-issues $ARGS
+
+          # Read into array, one filename per line
+          readarray -t files_to_build <<< "$GIT_DIFF_CONTENT"
+
+          ARGS=()
+          for file in "${files_to_build[@]}"; do
+            ARGS+=("-f" "$PWD/$file")
+          done
+
+          yarn rari build --no-basic --json-issues --data-issues "${ARGS[@]}"
           yarn yari-render-html
 
           echo "Disk usage size of the build"
@@ -126,25 +161,25 @@ jobs:
           wget https://github.com/${{ github.repository }}/compare/${BASE_SHA}...${HEAD_SHA}.diff -O ${BUILD_OUT_ROOT}/DIFF
 
       - name: Merge static assets with built documents
-        if: steps.check.outputs.GIT_DIFF_CONTENT
+        if: steps.check.outputs.HAS_MD_FILES == 'true'
         run: |
           # Exclude the .map files, as they're used for debugging JS and CSS.
           rsync -a --exclude "*.map" node_modules/@mdn/yari/client/build/ $BUILD_OUT_ROOT
           # Show the final disk usage size of the build.
           du -sh $BUILD_OUT_ROOT
 
       - uses: actions/upload-artifact@v4
-        if: steps.check.outputs.GIT_DIFF_CONTENT
+        if: steps.check.outputs.HAS_MD_FILES == 'true'
         with:
           name: build
           path: ${{ env.BUILD_OUT_ROOT }}
 
       - name: Check changed files
-        if: steps.check.outputs.GIT_DIFF_FILES
+        if: steps.check.outputs.HAS_IMAGE_FILES == 'true'
         env:
           GIT_DIFF_FILES: ${{ steps.check.outputs.GIT_DIFF_FILES }}
         run: |
-          echo $GIT_DIFF_FILES
+          readarray -t files_to_check <<< "$GIT_DIFF_FILES"
 
           export CONTENT_ROOT=$(pwd)/files
-          yarn filecheck $GIT_DIFF_FILES
+          yarn filecheck "${files_to_check[@]}"
