ci: optimize release version checks and version caching - #14229
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request optimizes several shell scripts by replacing slow operations (such as mvn help:evaluate and full git clone commands) with faster alternatives like local XML parsing, git ls-remote, and associative array caching. However, the introduction of a sed-based XML parser in check_status.sh is fragile and can incorrectly extract parent POM versions instead of the project's own version. It is recommended to use xmllint with a robust XPath query to safely parse the POM files.
ff9e1c8 to
65b2bf0
Compare
lqiu96
changed the title
d975f64 to
b0c5b5f
Compare
| local tag=$1 | ||
| local file=$2 | ||
| [[ ! -f "${file}" ]] && return 0 | ||
| sed -n "/<parent>/,/<\/parent>/d; \|<${tag}[ >]|{s|.*<${tag}[^>]*>[[:space:]]*\([^<[:space:]]*\).*|\1|p; q;}" "${file}" |
There was a problem hiding this comment.
XML format is pretty flexible and I feel this may miss some cases. For example, group_id might be inherited from parent and not exist in the pom. I think we should use other xml tools (e.g. built-in Python XML parser) instead of using sed.
There was a problem hiding this comment.
For example, group_id might be inherited from parent and not exist in the pom. I think we should use other xml tools (e.g. built-in Python XML parser) instead of using sed.
I believe Python's default XML parser uses XPath (same as Xmllint). XPath runs into the issue where parses the structure of a single XML file and has no knowledge of Maven pom.xml hierarchy (parent -> child inheritance)
A more robust structure for parsing may be needed in the future (ideally fast enough without having to invoke mvn help:effective-pom as that's a relatively costly call). I think the current bash call should be roughly 1:1 with existing functionality for the script. I'll move it away from common.sh and into the specific calling script so other scripts don't build reliance on this non-perfect solution.
There was a problem hiding this comment.
I like the changes in this file. Can we repurpose this PR to only include versions_map changes and remove the xml parsing part?
b0c5b5f to
5726a4a
Compare
In release generation scripts and nightly protobuf compatibility jobs:
- In check_existing_release_versions.sh (generation/check_existing_release_versions.sh):
* Replaced external xmllint calls with local extract_xml_tag helper, eliminating
the xmllint / libxml2-utils dependency completely while avoiding parent-tag collisions.
* Replaced memory-buffered $(find ... | sort) with streaming process substitution.
* Replaced complex chained if condition with clean case pattern matching.
- In downstream-protobuf-binary-compatibility.sh (sdk-platform-java/.kokoro/nightly/downstream-protobuf-binary-compatibility.sh):
* Pre-cached versions.txt into associative array declare -A versions_map, eliminating ~120
cat/grep/cut subprocesses and 40 redundant file reads.
* Added --depth 1 shallow clone for cloud-opensource-java, saving 98.5% of git objects and ~10MB.
* Replaced space-substitution word-splitting with idiomatic IFS=, read -ra array operations.
* Added explanatory comments for bash parameter expansion artifactId extraction.
5726a4a to
075de09
Compare
lqiu96
changed the title
|
|
2 participants
Problem
In release generation scripts and nightly protobuf compatibility jobs:
generation/check_existing_release_versions.sh,xmllintwas invoked 3 separate times per POM across all 1,600+ POMs to extract coordinates, introducing a brittle external dependency onxmllint / libxml2-utils.sdk-platform-java/.kokoro/nightly/downstream-protobuf-binary-compatibility.sh,cat,grep, andcutwere repeatedly executed againstversions.txtinside a loop for each artifact (~120 subprocesses), and full repository history was cloned forcloud-opensource-java.Changes
generation/check_existing_release_versions.sh):xmllintinvocations with a localextract_xml_taghelper (ignoring<parent>blocks), removing the externalxmllint / libxml2-utilsdependency completely.$(find ... | sort)with streaming process substitution.ifconditions with cleancasepattern matching.sdk-platform-java/.kokoro/nightly/downstream-protobuf-binary-compatibility.sh):versions.txtinto an associative array (declare -A versions_map), eliminating ~120cat/grep/cutsubprocesses and 40 redundant file reads.--depth 1shallow clone forcloud-opensource-java, saving ~98% of git objects and ~10MB.IFS=, read -raarray operations.