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.
This commit is contained in:
Martin Atkins 2022-08-25 15:32:52 -07:00
parent 101d0d91af
commit 246686813d

View File

@ -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