From 246686813df8c46750421010aae4007ce31bd351 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 25 Aug 2022 15:32:52 -0700
Subject: [PATCH] build: goimports check supports any number of changed files

The previous implementation of this check tried to accumulate all of the
changed files into a single big string and then run goimports once with
all of them, but that approach ran into problems for changesets of a
certain (platform-specific) size due to limits on maximum command line
length.

This new version instead uses bash arrays and runs goimports separately
for each of the files which appear to have changed relative to the
base branch. This is likely to be slower to complete for changesets that
have many different changed files, but it's better for it to be slow than
to return an error and fail to check some of the files.
---
 scripts/goimportscheck.sh | 56 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 7 deletions(-)

diff --git a/scripts/goimportscheck.sh b/scripts/goimportscheck.sh
index f80e4a68fe..1e8c2f560d 100755
--- a/scripts/goimportscheck.sh
+++ b/scripts/goimportscheck.sh
@@ -1,19 +1,61 @@
 #!/usr/bin/env bash
 
+set -euo pipefail
+
 # Check goimports
 echo "==> Checking the code complies with goimports requirements..."
-target_files=$(git diff --name-only origin/main --diff-filter=MA | grep "\.go")
 
-if [[ -z ${target_files}  ]]; then
+# We only require goimports to have been run on files that were changed
+# relative to the main branch, so that we can gradually create more consistency
+# rather than bulk-changing everything at once.
+
+declare -a target_files
+# "readarray" will return an "unbound variable" error if there isn't already
+# at least one element in the target array. "readarray" will overwrite this
+# item, though.
+target_files[0]=""
+
+base_branch="origin/main"
+readarray -t target_files < <(git diff --name-only ${base_branch} --diff-filter=MA | grep "\.go")
+
+if [[ "${#target_files[@]}" -eq 0 ]]; then
+  echo "No files have changed relative to branch ${base_branch}, so there's nothing to check!"
   exit 0
 fi
 
-goimports_files=$(goimports -w -l "${target_files}")
-if [[ -n ${goimports_files} ]]; then
-  echo 'goimports needs running on the following files:'
-  echo "${goimports_files}"
-  echo "You can use the command and flags \`goimports -w -l\` to reformat the code"
+declare -a incorrect_files
+# Array must have at least one item before we can append to it. Code below must
+# work around this extra empty-string element at the beginning of the array.
+incorrect_files[0]=""
+
+for filename in "${target_files[@]}"; do
+  if [[ -z "$filename" ]]; then
+    continue
+  fi
+
+  output=$(go run golang.org/x/tools/cmd/goimports -l "${filename}")
+  if [[ $? -ne 0 ]]; then
+    echo >&2 goimports failed for "$filename"
+    exit 1
+  fi
+
+  if [[ -n "$output" ]]; then
+    incorrect_files+=("$filename")
+  fi
+done
+
+if [[ "${#incorrect_files[@]}" -gt 1 ]]; then
+  echo >&2 'The following files have import statements that disagree with "goimports"':
+  for filename in "${incorrect_files[@]}"; do
+    if [[ -z "$filename" ]]; then
+      continue
+    fi
+
+    echo >&2 ' - ' "${filename}"
+  done
+  echo >&2 'Use `go run golang.org/x/tools/cmd/goimports -w -l` on each of these files to update these files.'
   exit 1
 fi
 
+echo 'All of the changed files look good!'
 exit 0