Skip to content

Commit 6bf8e1a

Browse files
lqiu96lqiu96
committed
perf(ci): replace mvn help:evaluate with native bash and sed extraction
In Kokoro CI and automation scripts, heavy Maven JVM invocations were used to parse POM values: - In generate_modified_modules_list (.kokoro/common.sh), evaluating project.modules launched a full JVM and evaluated the monorepo POMs, taking 20–30+ seconds on every single CI run. - In downstream-build.sh (.kokoro/presubmit/downstream-build.sh) and showcase-native.sh (sdk-platform-java/.kokoro/presubmit/showcase-native.sh), evaluating gapic-showcase.version launched Maven JVM processes. - In update_javadoc.sh (google-auth-library-java/scripts/update_javadoc.sh), maven-help-plugin:evaluate was called to obtain the project version. Optimizations: - Pure-Bash Module Extraction: Updated generate_modified_modules_list to use extract_pom_modules pom.xml, extracting all 270 modules in ~0.02s without JVM boot overhead. - Sed Showcase Version Extraction: Updated downstream-build.sh and showcase-native.sh to parse <gapic-showcase.version> directly using sed with fail-fast validation, and added --fail to curl. - Sed Javadoc Version Extraction: Updated update_javadoc.sh to parse project version from pom.xml using sed, skipping parent POM declarations. - Unit Tests: Added automated unit tests in .kokoro/common_test.sh for extract_pom_modules and generate_modified_modules_list. - Refactoring & Best Practices: Scoped temporary variables as local, used pure-bash whitespace trimming and string manipulations, and anchored paths.
1 parent 20fc417 commit 6bf8e1a

5 files changed

Lines changed: 190 additions & 49 deletions

File tree

‎.kokoro/common.sh‎

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -90,53 +90,56 @@ function retry_with_backoff {
9090
# and naturally survives single-module components without throwing exit signals.
9191
function extract_pom_modules() {
9292
local pom_file="$1"
93-
local modules_list=""
93+
if [[ ! -f "${pom_file}" ]]; then
94+
return 1
95+
fi
96+
local line module
9497
local in_profiles=false
9598
local in_modules=false
96-
97-
while IFS= read -r line || [ -n "$line" ]; do
98-
if [[ "$line" == *"<profiles>"* ]]; then
99+
local -a modules=()
100+
101+
while IFS= read -r line || [[ -n "${line}" ]]; do
102+
if [[ "${line}" == *"<profiles>"* ]]; then
99103
in_profiles=true
100-
elif [[ "$line" == *"</profiles>"* ]]; then
104+
elif [[ "${line}" == *"</profiles>"* ]]; then
101105
in_profiles=false
102-
elif [[ "$line" == *"<modules>"* ]] && [ "$in_profiles" = false ]; then
106+
elif [[ "${line}" == *"<modules>"* && "${in_profiles}" == "false" ]]; then
103107
in_modules=true
104-
elif [[ "$line" == *"</modules>"* ]] && [ "$in_profiles" = false ]; then
108+
elif [[ "${line}" == *"</modules>"* && "${in_profiles}" == "false" ]]; then
105109
in_modules=false
106110
break
107-
elif [ "$in_modules" = true ] && [[ "$line" == *"<module>"* ]]; then
111+
elif [[ "${in_modules}" == "true" && "${line}" == *"<module>"* ]]; then
108112
# Extract text between tags
109-
local module="${line#*<module>}"
113+
module="${line#*<module>}"
110114
module="${module%</module>*}"
111-
112-
# Trim whitespace natively
115+
116+
# Trim leading/trailing whitespace without spawning external processes
113117
module="${module#"${module%%[![:space:]]*}"}"
114118
module="${module%"${module##*[![:space:]]}"}"
115-
116-
if [ -z "$modules_list" ]; then
117-
modules_list="$module"
118-
else
119-
modules_list="${modules_list} ${module}"
119+
120+
if [[ -n "${module}" ]]; then
121+
modules+=("${module}")
120122
fi
121123
fi
122-
done < "$pom_file"
123-
124-
echo "$modules_list"
124+
done < "${pom_file}"
125+
126+
echo "${modules[*]}"
125127
}
126128

127129
# Given a folder containing a maven multi-module, assign the variable 'submodules' to a
128130
# comma-delimited list of <folder>/<submodule>.
129131
function parse_submodules() {
130132
submodules_array=()
131-
if [ -f "$1/pom.xml" ]; then
133+
if [[ -f "$1/pom.xml" ]]; then
132134
local modules
135+
local submodule
133136

134137
# Use pure Bash extraction to find the modules in the aggregator pom file.
135138
# Faster than invoking mvn help:evaluate to list all the project modules,
136139
# cleanly ignores optional <profiles>, and gracefully skips flat POMs.
137140
modules=$(extract_pom_modules "$1/pom.xml")
138-
if [ -n "$modules" ]; then
139-
for submodule in $modules; do
141+
if [[ -n "${modules}" ]]; then
142+
for submodule in ${modules}; do
140143
# Each entry = <folder>/<submodule>
141144
submodules_array+=("$1/${submodule}")
142145
done
@@ -285,39 +288,54 @@ function generate_modified_modules_list() {
285288
files=$(get_modified_files)
286289
printf "Modified files:\n%s\n" "${files}"
287290

288-
# Generate the list of valid maven modules
289-
maven_modules_list=$(mvn help:evaluate -Dexpression=project.modules | grep '<.*>.*</.*>' | sed -e 's/<.*>\(.*\)<\/.*>/\1/g')
291+
# Extract valid maven modules directly from pom.xml in pure Bash (~0.02s).
292+
# This replaces 'mvn help:evaluate -Dexpression=project.modules' which previously
293+
# spent 20-30+ seconds booting a JVM and evaluating the monorepo POMs on every run.
294+
local root_pom="${commonScriptDir}/../pom.xml"
295+
if [[ ! -f "${root_pom}" ]]; then
296+
root_pom="pom.xml"
297+
fi
298+
local maven_modules_list
299+
maven_modules_list=$(extract_pom_modules "${root_pom}")
290300
maven_modules=()
291301

292-
# If the first argument is "true" (default), then use the module exclusion list
293-
use_exclusion_list=${1:-true}
302+
# Positional parameter $1 specifies whether to apply the exclusion list (defaults to true).
303+
local use_exclusion_list="${1:-true}"
304+
local -a all_modules=()
305+
read -r -a all_modules <<< "${maven_modules_list}"
306+
307+
local module
294308
if [[ "${use_exclusion_list}" == "true" ]]; then
295309
echo "Excluding modules from the global exclusion list"
296-
for module in $maven_modules_list; do
297-
if [[ ! " ${excluded_modules[*]} " =~ " ${module} " ]]; then
310+
for module in "${all_modules[@]}"; do
311+
if [[ ! " ${excluded_modules[*]} " == *" ${module} "* ]]; then
298312
maven_modules+=("${module}")
299313
fi
300314
done
301315
else
302-
maven_modules=(${maven_modules_list[*]})
316+
maven_modules=("${all_modules[@]}")
303317
fi
304318

305319
modified_module_list=()
306320
# If either parent pom.xml or core shared dependency is touched, run ITs on all the modules
307321
if should_test_all_modules; then
308-
modified_module_list=(${maven_modules[*]})
322+
# '("${maven_modules[@]}")' copies the array elements safely.
323+
modified_module_list=("${maven_modules[@]}")
309324
echo "Testing the entire monorepo"
310325
else
311-
modules=$(echo "${files}" | grep -E '(google-auth|java)-.*' || true)
326+
# Extract the top-level directory from each modified file path:
327+
# 'cut -d '/' -f1' takes the first path segment (e.g. 'java-bigquery/src/...' -> 'java-bigquery').
328+
# 'sort -u' sorts and deduplicates the candidate directory names.
329+
local modules
330+
modules=$(cut -d '/' -f1 <<< "${files}" | sort -u)
312331
printf "Files in java modules:\n%s\n" "${modules}"
313-
if [[ -n $modules ]]; then
314-
modules=$(echo "${modules}" | cut -d '/' -f1 | sort -u)
315-
for module in $modules; do
316-
if [[ " ${maven_modules[*]} " =~ " ${module} " ]]; then
317-
modified_module_list+=("${module}")
318-
fi
319-
done
320-
else
332+
for module in ${modules}; do
333+
# If this top-level directory is a recognized Maven module, add it to our list.
334+
if [[ " ${maven_modules[*]} " == *" ${module} "* ]]; then
335+
modified_module_list+=("${module}")
336+
fi
337+
done
338+
if [[ ${#modified_module_list[@]} -eq 0 ]]; then
321339
echo "Found no changes in the java modules"
322340
fi
323341

‎.kokoro/common_test.sh‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,35 @@ function test_parse_pom_version {
8181
popd
8282
}
8383

84+
# Test that extract_pom_modules correctly extracts modules from root pom.xml
85+
# in pure Bash without launching Maven, and handles non-existent files safely.
86+
function test_extract_pom_modules {
87+
local -a modules
88+
read -r -a modules <<< "$(extract_pom_modules "${scriptDir}/../pom.xml")"
89+
if (( ${#modules[@]} < 200 )); then
90+
echo "extract_pom_modules failed: expected at least 200 modules, got ${#modules[@]}"
91+
exit 1
92+
fi
93+
if [[ ! " ${modules[*]} " =~ " java-bigquery " ]]; then
94+
echo "extract_pom_modules missing java-bigquery"
95+
exit 1
96+
fi
97+
if [[ ! " ${modules[*]} " =~ " java-bigquerystorage " ]]; then
98+
echo "extract_pom_modules missing java-bigquerystorage"
99+
exit 1
100+
fi
101+
if [[ ! " ${modules[*]} " =~ " sdk-platform-java " ]]; then
102+
echo "extract_pom_modules missing sdk-platform-java"
103+
exit 1
104+
fi
105+
106+
# Verify non-existent file returns 1 and empty output
107+
if extract_pom_modules "non_existent_pom.xml" &>/dev/null; then
108+
echo "extract_pom_modules should return non-zero for non-existent pom"
109+
exit 1
110+
fi
111+
}
112+
84113
# Tests that is_module_modified strictly matches the module directory prefix,
85114
# preventing prefix collisions (e.g. java-bigquery vs java-bigquerystorage).
86115
function test_is_module_modified {
@@ -308,10 +337,87 @@ function test_mock_get_modified_files {
308337
unset TEST_MODIFIED_FILES
309338
}
310339

340+
# Test that parse_submodules expands multi-module directories to child modules
341+
# and preserves single-module/flat components.
342+
function test_parse_submodules {
343+
pushd "${scriptDir}/.." >/dev/null
344+
345+
parse_submodules "java-bigquery"
346+
if [[ "${submodules}" != "java-bigquery/google-cloud-bigquery,java-bigquery/google-cloud-bigquery-bom" ]]; then
347+
echo "parse_submodules failed for java-bigquery: got ${submodules}"
348+
exit 1
349+
fi
350+
351+
parse_submodules "google-cloud-jar-parent"
352+
if [[ "${submodules}" != "google-cloud-jar-parent" ]]; then
353+
echo "parse_submodules failed for google-cloud-jar-parent: got ${submodules}"
354+
exit 1
355+
fi
356+
357+
popd >/dev/null
358+
}
359+
360+
# Test that generate_modified_modules_list correctly maps modified files to
361+
# their top-level module names without requiring mvn help:evaluate, and verifies
362+
# both exclusion list enabled (default) and disabled modes.
363+
function test_generate_modified_modules_list {
364+
pushd "${scriptDir}/.." >/dev/null
365+
TEST_MODIFIED_FILES="java-bigquery/google-cloud-bigquery/src/main/java/Foo.java
366+
java-asset/google-cloud-asset/pom.xml"
367+
generate_modified_modules_list false >/dev/null
368+
369+
local has_bigquery="false"
370+
local has_asset="false"
371+
local has_bigquerystorage="false"
372+
373+
if [[ " ${modified_module_list[*]} " =~ " java-bigquery " ]]; then
374+
has_bigquery="true"
375+
fi
376+
if [[ " ${modified_module_list[*]} " =~ " java-asset " ]]; then
377+
has_asset="true"
378+
fi
379+
if [[ " ${modified_module_list[*]} " =~ " java-bigquerystorage " ]]; then
380+
has_bigquerystorage="true"
381+
fi
382+
383+
if [[ "${has_bigquery}" != "true" ]]; then
384+
echo "generate_modified_modules_list missing java-bigquery"
385+
exit 1
386+
fi
387+
if [[ "${has_asset}" != "true" ]]; then
388+
echo "generate_modified_modules_list missing java-asset"
389+
exit 1
390+
fi
391+
if [[ "${has_bigquerystorage}" == "true" ]]; then
392+
echo "generate_modified_modules_list incorrectly included java-bigquerystorage"
393+
exit 1
394+
fi
395+
396+
# Test default exclusion list behavior: java-bigquery is excluded, while java-asset is included
397+
TEST_MODIFIED_FILES="java-asset/google-cloud-asset/Foo.java
398+
java-bigquery/google-cloud-bigquery/Bar.java"
399+
generate_modified_modules_list true >/dev/null
400+
401+
if [[ " ${modified_module_list[*]} " =~ " java-bigquery " ]]; then
402+
echo "generate_modified_modules_list should exclude java-bigquery when exclusion list is true"
403+
exit 1
404+
fi
405+
if [[ ! " ${modified_module_list[*]} " =~ " java-asset " ]]; then
406+
echo "generate_modified_modules_list missing java-asset when exclusion list is true"
407+
exit 1
408+
fi
409+
410+
popd >/dev/null
411+
unset TEST_MODIFIED_FILES
412+
}
413+
311414
test_find_all_poms_with_versioned_dependency
312415
test_update_pom_dependency
313416
test_parse_pom_version
314417
test_mock_get_modified_files
418+
test_extract_pom_modules
419+
test_parse_submodules
315420
test_should_test_all_modules
316421
test_is_module_modified
317422
test_is_upstream_module_modified
423+
test_generate_modified_modules_list

‎.kokoro/presubmit/downstream-build.sh‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,23 @@ pushd java-showcase
4747
modify_shared_config
4848
popd
4949

50-
# Parse showcase version from the local directory
51-
pushd java-showcase/gapic-showcase
52-
SHOWCASE_VERSION=$(mvn help:evaluate -Dexpression=gapic-showcase.version -q -DforceStdout)
53-
popd
50+
# Extract the showcase version directly from pom.xml using sed:
51+
# - 'sed -n': suppresses default line printing.
52+
# - 's:...[[:space:]]*\([^<[:space:]]*\).*:\1:p': captures non-whitespace version text and prints it.
53+
# - '/.../q': quits immediately on first match, avoiding trailing passes and external pipe utilities.
54+
# This replaces 'mvn help:evaluate' which previously took 15+ seconds to boot Maven.
55+
SHOWCASE_VERSION=$(sed -n 's:.*<gapic-showcase\.version>[[:space:]]*\([^<[:space:]]*\).*:\1:p; /<gapic-showcase\.version>/q' java-showcase/gapic-showcase/pom.xml)
56+
57+
# Fail fast with a clear error message if the version could not be parsed,
58+
# preventing malformed curl URLs and ambiguous downstream failures.
59+
if [[ -z "${SHOWCASE_VERSION}" ]]; then
60+
echo "Error: Failed to parse gapic-showcase.version from java-showcase/gapic-showcase/pom.xml" >&2
61+
exit 1
62+
fi
5463

5564
# Start showcase server
5665
mkdir -p /usr/src/showcase
57-
curl --location https://github.com/googleapis/gapic-showcase/releases/download/v"${SHOWCASE_VERSION}"/gapic-showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz --output /usr/src/showcase/showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz
66+
curl --fail --location https://github.com/googleapis/gapic-showcase/releases/download/v"${SHOWCASE_VERSION}"/gapic-showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz --output /usr/src/showcase/showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz
5867
pushd /usr/src/showcase/
5968
tar -xf showcase-*
6069
./gapic-showcase run &

‎google-auth-library-java/scripts/update_javadoc.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@
3131

3232
set -e
3333

34-
VERSION=$(mvn org.apache.maven.plugins:maven-help-plugin:2.1.1:evaluate -Dexpression=project.version | grep -Ev '(^\[|\w+:)')
34+
VERSION=$(sed -n '/<parent>/,/<\/parent>/d; /<version>/{s:.*<version>[[:space:]]*\([^<[:space:]]*\).*:\1:p; q;}' pom.xml)
3535

3636
if [ -z "$VERSION" ]; then
37-
echo "Error updating Javadoc: could not obtain version number from maven-help-plugin."
37+
echo "Error updating Javadoc: could not obtain version number from pom.xml."
3838
exit 1
3939
fi
4040

‎sdk-platform-java/.kokoro/presubmit/showcase-native.sh‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,20 @@ mvn install --projects '!gapic-generator-java' \
3838
SHARED_DEPS_VERSION=$(parse_pom_version java-shared-dependencies)
3939

4040
# Run showcase integration tests in GraalVM
41-
pushd java-showcase/gapic-showcase
42-
SHOWCASE_VERSION=$(mvn help:evaluate -Dexpression=gapic-showcase.version -q -DforceStdout)
43-
popd
41+
SHOWCASE_POM="java-showcase/gapic-showcase/pom.xml"
42+
if [[ ! -f "${SHOWCASE_POM}" && -f "../java-showcase/gapic-showcase/pom.xml" ]]; then
43+
SHOWCASE_POM="../java-showcase/gapic-showcase/pom.xml"
44+
fi
45+
SHOWCASE_VERSION=$(sed -n 's:.*<gapic-showcase\.version>[[:space:]]*\([^<[:space:]]*\).*:\1:p; /<gapic-showcase\.version>/q' "${SHOWCASE_POM}")
46+
if [[ -z "${SHOWCASE_VERSION}" ]]; then
47+
echo "Error: Failed to parse gapic-showcase.version from ${SHOWCASE_POM}" >&2
48+
exit 1
49+
fi
50+
4451
# Start showcase server
4552
mkdir -p /usr/src/showcase
4653
curl \
54+
--fail \
4755
--location https://github.com/googleapis/gapic-showcase/releases/download/v"${SHOWCASE_VERSION}"/gapic-showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz \
4856
--output /usr/src/showcase/showcase-"${SHOWCASE_VERSION}"-linux-amd64.tar.gz
4957
pushd /usr/src/showcase/

0 commit comments

Comments
 (0)