Massive DIFF_FILES crashes bash in repo-sync (#23326)

* Massive DIFF_FILES crashes bash in repo-sync

Part of #1304

* gst

* make exception

Author: Peter Bengtsson

.github/workflows/test.yml

@@ -56,7 +56,10 @@ jobs:
 
       - name: Insight into changed files
         run: |
-          echo ${{ steps.get_diff_files.outputs.files }}
+
+          # Must to do this because the list of files can be HUGE. Especially
+          # in a repo-sync when there are lots of translation files involved.
+          echo "${{ steps.get_diff_files.outputs.files }}" > get_diff_files.txt
 
       - name: Setup node
         uses: actions/setup-node@04c56d2f954f1e4c69436aa54cfef261a018f458
@@ -80,5 +83,5 @@ jobs:
 
       - name: Run tests
         env:
-          DIFF_FILES: ${{ steps.get_diff_files.outputs.files }}
+          DIFF_FILE: get_diff_files.txt
         run: npm run test tests/${{ matrix.test-group }}/

tests/linting/lint-files.js

@@ -23,6 +23,10 @@ import { supported, next, nextNext, deprecated } from '../../lib/enterprise-serv
 import { getLiquidConditionals } from '../../script/helpers/get-liquid-conditionals.js'
 import allowedVersionOperators from '../../lib/liquid-tags/ifversion-supported-operators.js'
 import semver from 'semver'
+import { jest } from '@jest/globals'
+import { getDiffFiles } from '../utils.js'
+
+jest.useFakeTimers('legacy')
 
 const __dirname = path.dirname(fileURLToPath(import.meta.url))
 const enterpriseServerVersions = Object.keys(allVersions).filter((v) =>
@@ -357,28 +361,18 @@ function getContent(content) {
   return null
 }
 
-// Filter out entries from an array like this:
-//
-//    [
-//      [relativePath, absolutePath],
-//      ...
-// so it's only the files mentioned in the DIFF_FILES environment
-// variable, but only if it's set and present.
-
-// Setting an environment varible called `DIFF_FILES` is optional.
-// But if and only if it's set, we will respect it.
-// And if it set, turn it into a cleaned up Set so it's made available
-// every time we use it.
-if (process.env.DIFF_FILES) {
-  // Parse and turn that environment variable string into a set.
+const diffFiles = getDiffFiles()
+
+// If present, and not empty, leverage it because in most cases it's empty.
+if (diffFiles.length > 0) {
   // It's faster to do this once and then re-use over and over in the
   // .filter() later on.
   const only = new Set(
     // If the environment variable encodes all the names
     // with quotation marks, strip them.
     // E.g. Turn `"foo" "bar"` into ['foo', 'bar']
     // Note, this assumes no possible file contains a space.
-    process.env.DIFF_FILES.split(/\s+/g).map((name) => {
+    diffFiles.map((name) => {
       if (/^['"]/.test(name) && /['"]$/.test(name)) {
         return name.slice(1, -1)
       }

tests/meta/orphan-tests.js

@@ -5,7 +5,7 @@ import { filter as asyncFilter } from 'async'
 describe('check for orphan tests', () => {
   test('all tests are in sub-directories', async () => {
     // A known list of exceptions that can live outside of directories
-    const EXCEPTIONS = ['README.md', 'package.json']
+    const EXCEPTIONS = ['README.md', 'package.json', 'utils.js']
     const pathToTests = path.join(process.cwd(), 'tests')
 
     // Get a list of files/directories in `/tests`

tests/utils.js

@@ -0,0 +1,29 @@
+import fs from 'fs'
+
+// The reason we're not manually doing a spawned subprocess
+// of `git diff --name-only ...` or something here is because that stuff
+// is unpredictable in GitHub Actions because of how it does `git clone`.
+// So we rely on environment variables instead.
+
+export function getDiffFiles() {
+  // Instead of testing every single file possible, if there's
+  // an environment variable called `DIFF_FILES` or one called
+  // `DIFF_FILE` then use that.
+  // If `DIFF_FILES` is set, it's expected to be a space separated
+  // string. If `DIFF_FILE` is set, it's expected to be a text file
+  // which contains a space separated string.
+  const diffFiles = []
+  // Setting an environment varible called `DIFF_FILES` is optional.
+  // But if and only if it's set, we will respect it.
+  // And if it set, turn it into a cleaned up Set so it's made available
+  // every time we use it.
+  // Alternatively, you can put all the files change changed into a
+  // text file and do `export DIFF_FILE=files-that-changed.txt`
+  if (process.env.DIFF_FILES) {
+    diffFiles.push(...process.env.DIFF_FILES.trim().split(/\s+/g))
+  } else if (process.env.DIFF_FILE) {
+    diffFiles.push(...fs.readFileSync(process.env.DIFF_FILE, 'utf-8').trim().split(/\s+/g))
+  }
+
+  return diffFiles
+}