Skip to content

ci: fix array iteration, variable leaks, and spurious full-reactor lint runs #14227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lqiu96 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ci: fix array iteration, variable leaks, and spurious full-reactor lint runs #14227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 35 additions & 52 deletions .kokoro/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,67 +241,50 @@ case ${JOB_TYPE} in
changed_file_list=$(git diff --name-only "${BASE_SHA}" "${HEAD_SHA}" --relative)
echo "${changed_file_list}"

has_code_change="false"

changed_modules=()
while IFS= read -r changed_file; do
# Checks if the line is not empty AND if it matches a .java file
if [ -n "${changed_file}" ] && [[ "${changed_file}" == *.java ]]; then
echo "Matched: ${changed_file}"
has_code_change="true"
break
fi
done <<< "${changed_file_list}"
[[ -z "${changed_file}" || "${changed_file}" != *.java ]] && continue

if [ "${has_code_change}" == "false" ]; then
echo "No java modules affected. Skipping linter check."
exit 0
fi
# Skip samples and test utilities that have standalone builds:
case "${changed_file}" in
*samples* | *java-showcase* | *test-proxy*)
continue
;;
esac

# Compute list of changed Maven modules from changed Java files.
# We walk each changed .java file up to its nearest pom.xml to find the correct module.
# e.g., if "java-asset/google-cloud-asset/src/main/java/Foo.java" is changed,
# it traverses upward until finding "java-asset/google-cloud-asset/pom.xml" and adds that module.
changed_modules=()
while IFS= read -r changed_file; do
if [ -n "${changed_file}" ] && [[ "${changed_file}" == *.java ]]; then
dir=$(dirname "${changed_file}")
while [ "${dir}" != "." ] && [ ! -f "${dir}/pom.xml" ]; do
dir=$(dirname "${dir}")
done
if [ -f "${dir}/pom.xml" ] && [ "${dir}" != "." ]; then
# Filter out directories not participating in the default formatting reactor:
# - samples are handwritten by developers
# - benchmarks are handwritten by developers
# - proto-*/grpc-* are generated code and should use the compiler format
# - *-bom/parents are POM-only and contain no Java source
if [[ "${dir}" != *"samples"* ]] && \
[[ "${dir}" != *"java-showcase"* ]] && \
[[ "$(basename "${dir}")" != *"benchmark"* ]] && \
[[ "$(basename "${dir}")" != "proto-google-"* ]] && \
[[ "$(basename "${dir}")" != "grpc-google-"* ]] && \
[[ "$(basename "${dir}")" != *"-bom" ]] && \
[[ "$(basename "${dir}")" != "google-cloud-pom-parent" ]] && \
[[ "$(basename "${dir}")" != "dependency-analyzer" ]] && \
[[ "$(basename "${dir}")" != "dependency-convergence-check" ]] && \
[[ "$(basename "${dir}")" != "unmanaged-dependency-check" ]] && \
[[ "$(basename "${dir}")" != *"test-proxy"* ]] && \
[[ "$(basename "${dir}")" != "google-cloud-jar-parent" ]]; then
# Traverse upward to the nearest enclosing pom.xml:
dir=$(dirname "${changed_file}")
while [ "${dir}" != "." ] && [ ! -f "${dir}/pom.xml" ]; do
dir=$(dirname "${dir}")
done

[ -f "${dir}/pom.xml" ] && [ "${dir}" != "." ] || continue

changed_modules+=("${dir}")
fi
fi
fi
# Skip modules not participating in the formatting reactor:
case "${dir##*/}" in
*benchmark* | proto-google-* | grpc-google-* | *-bom | \
google-cloud-pom-parent | google-cloud-jar-parent | \
dependency-analyzer | dependency-convergence-check | unmanaged-dependency-check)
continue
;;
*)
changed_modules+=("${dir}")
;;
esac
done <<< "${changed_file_list}"

echo "Changed Modules: ${changed_modules[*]}"

# Deduplicate the modules using sort -u to pass a concise list of unique modules
# via the Maven `-pl` argument.
if [ ${#changed_modules[@]} -gt 0 ]; then
unique_modules=$(printf '%s\n' "${changed_modules[@]}" | sort -u | paste -sd ',' -)
MODULE_FILTER="-pl ${unique_modules}"
echo "Formatting only changed modules: ${unique_modules}"
# If only non-reactor files or no Java files changed, skip cleanly without running full reactor:
if [ ${#changed_modules[@]} -eq 0 ]; then
echo "No relevant java modules affected after exclusions. Skipping linter check."
exit 0
fi

# Pass a concise comma-separated list of unique modules via the Maven `-pl` argument.
unique_modules=$(printf '%s\n' "${changed_modules[@]}" | sort -u | paste -sd ',' -)
MODULE_FILTER="-pl ${unique_modules}"
echo "Formatting only changed modules: ${unique_modules}"
else
echo "BASE_SHA or HEAD_SHA is empty. Cannot continue linting."
exit 1
Expand Down
45 changes: 33 additions & 12 deletions .kokoro/common.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,15 +498,14 @@ function generate_graalvm_modules_list() {
num=$((num + 1))
done
elif [[ ${#modified_module_list[@]} -gt 0 ]]; then
# MAVEN_MODULES ENV_VAR is expecting comma delimited string (similar to mvn -pl)
# This will get all the modules and put all the elements into an array
maven_modules_list=($(echo "${MAVEN_MODULES}" | tr ',' ' '))
# Parse comma-delimited MAVEN_MODULES into array using pure Bash:
IFS=',' read -ra maven_modules_list <<< "${MAVEN_MODULES}"
for maven_module in "${maven_modules_list[@]}"; do
# Check that the modified_module_list contains a module from MAVEN_MODULES
# Spaces are intentionally added -- Query is regex and array elements are space separated
# It tries to match the *exact* `maven_module` text
if [[ " ${modified_module_list[*]} " =~ " ${maven_module} " ]]; then
modules_assigned_list+=("${module}")
modules_assigned_list+=("${maven_module}")
fi
done
fi
Expand Down Expand Up @@ -624,16 +623,36 @@ EOF
popd || exit 1
}

# Find all pom.xml files that declare a specific version for the given artifact ($1)
# Find all pom.xml files that declare a specific version for the given artifact ($1).
# Pre-filters candidate files with grep -rl to avoid executing xmllint across hundreds
# of unrelated POM files in the repository.
function find_all_poms_with_versioned_dependency {
poms=($(find . -name pom.xml))
for pom in "${poms[@]}"; do
POMS=()
local found=()
local pom

# Stream matching pom.xml paths line-by-line via process substitution '< <(...)',
# which executes the while loop in the current shell process so 'found' array mutations persist:
while IFS= read -r pom; do
[[ -z "${pom}" ]] && continue
# Verify the POM declares an explicit <version> tag following the target <artifactId>:
if xmllint --xpath "//*[local-name()='artifactId' and text()='$1']/following-sibling::*[local-name()='version']" "$pom" &>/dev/null; then
found+=("$pom")
fi
done
POMS=(${found[@]})
unset found
done < <(
# Fast pre-filter using 'grep' to avoid parsing hundreds of unrelated POMs:
# - '-r': recursively scans directories.
# - '-l': prints each matching file path once (stops scanning a file on first match,
# preventing duplicate paths in the stream and saving I/O on large POMs).
# - '--include="pom.xml"': scopes search exclusively to POMs, ignoring non-POM files.
# - '<artifactId>...${1}...</artifactId>': matches tags around $1 with optional whitespace.
# - '2>/dev/null || true': silences errors and prevents 'set -e' failure when 0 files match.
grep -rlE "<artifactId>[[:space:]]*${1}[[:space:]]*</artifactId>" \
--include="pom.xml" \
. 2>/dev/null || true
)

POMS=("${found[@]}")
export POMS
}

Expand All @@ -643,8 +662,10 @@ function find_all_poms_with_versioned_dependency {
function update_all_poms_dependency {
pushd "$1" || exit 1
find_all_poms_with_versioned_dependency "$2"
for pom in $POMS; do
update_pom_dependency "$(dirname "$pom")" "$2" "$3"
# Quote "${POMS[@]}" so the loop iterates over each array element safely:
for pom in "${POMS[@]}"; do
# Use '${pom%/*}' to extract the parent directory in pure Bash without a 'dirname' subshell:
update_pom_dependency "${pom%/*}" "$2" "$3"
done
git diff
popd || exit 1
Expand Down
Loading