Skip to content

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

Open
lqiu96 wants to merge 1 commit into
googleapis:mainfrom
lqiu96:fix-ci-bugs
Open

ci: fix array iteration, variable leaks, and spurious full-reactor lint runs#14227
lqiu96 wants to merge 1 commit into
googleapis:mainfrom
lqiu96:fix-ci-bugs

Conversation

@lqiu96

@lqiu96 lqiu96 commented Sep 1, 2026

Copy link
Copy Markdown
Member

Summary

Fixes several correctness bugs and edge cases in Kokoro CI build and helper scripts:

  1. Fix Array Iteration in update_all_poms_dependency (.kokoro/common.sh):

    • for pom in $POMS; do only iterated over the first element of the array.
    • Updated to for pom in "${POMS[@]}"; do.
    • In find_all_poms_with_versioned_dependency, reset POMS=(), scoped local arrays, and pre-filtered candidate files with grep -rl "<artifactId>${1}</artifactId>" --include="pom.xml" . before running xmllint.
  2. Fix Variable Leaks in generate_graalvm_modules_list (.kokoro/common.sh):

    • Fixed typo where modules_assigned_list+=("${module}") appended a leaked ${module} variable instead of the loop variable ${maven_module}.
  3. Prevent Spurious Full-Reactor Lint Scans (.kokoro/build.sh):

    • When only excluded files (such as samples or benchmarks) were modified, has_code_change="true" was set, but changed_modules was empty after exclusions.
    • This left MODULE_FILTER="", causing Maven to run format/checkstyle without -pl across the entire 280+ module reactor.
    • If changed_modules is empty after exclusions, cleanly exit 0 and skip linting.

Verification

  • bash -n .kokoro/common.sh .kokoro/build.sh
  • git diff --check
  • bash .kokoro/common_test.sh

@lqiu96
lqiu96 requested review from a team as code owners September 1, 2026 14:43

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request optimizes and fixes issues in the Kokoro build scripts. Specifically, it skips the linter check early in .kokoro/build.sh if no relevant Java modules are affected, fixes a module assignment bug in .kokoro/common.sh, and optimizes the POM file search by using grep instead of find. The review feedback suggests making the grep pattern in find_all_poms_with_versioned_dependency more robust by allowing optional whitespace around the artifact ID to prevent silently skipping files with non-standard formatting.

Comment thread .kokoro/common.sh Outdated
@lqiu96
lqiu96 marked this pull request as draft September 1, 2026 16:50
@lqiu96
lqiu96 force-pushed the fix-ci-bugs branch 2 times, most recently from 712c532 to 28ae25b Compare September 1, 2026 20:01
…or lint runs

- In .kokoro/common.sh (update_all_poms_dependency): fix array iteration
  where `for pom in $POMS; do` only processed the first POM element. Update to
  `for pom in "${POMS[@]}"`, and use pure-Bash `${pom%/*}` instead of `$(dirname "$pom")`.
- In .kokoro/common.sh (find_all_poms_with_versioned_dependency): reset POMS=(),
  use local arrays, and pre-filter candidates with grep -rl before executing xmllint.
- In .kokoro/common.sh (generate_graalvm_modules_list): fix variable typo where
  modules_assigned_list appended leaked ${module} instead of loop variable ${maven_module},
  and parse MAVEN_MODULES using pure-Bash `IFS=, read -ra`.
- In .kokoro/build.sh: streamline changed Java module resolution into a single
  while loop with guard clauses, case pattern matching for exclusions, and
  cleanly skip linter checks when changed_modules is empty after exclusions
  (preventing spurious full-reactor lint runs without -pl).
@lqiu96
lqiu96 requested review from blakeli0 and whowes September 1, 2026 21:05
@lqiu96
lqiu96 marked this pull request as ready for review September 1, 2026 21:06
@lqiu96 lqiu96 changed the title fix(ci): fix array iteration, variable leaks, and spurious full-reactor lint runs ci: fix array iteration, variable leaks, and spurious full-reactor lint runs Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant