Skip to content

fix(release): support generic XML version tags in apply_versions.sh#13791

Draft
jinseopkim0 wants to merge 1 commit into
mainfrom
fix-apply-versions-regex
Draft

fix(release): support generic XML version tags in apply_versions.sh#13791
jinseopkim0 wants to merge 1 commit into
mainfrom
fix-apply-versions-regex

Conversation

@jinseopkim0

Copy link
Copy Markdown
Contributor

Updates apply_versions.sh to use a generic XML tag replacement regex with backreferences. This allows the script to update dependency versions defined in custom XML properties (such as first-party-dependencies.version in libraries-bom/pom.xml), rather than only matching standard tags.

@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 updates the version replacement logic in generation/apply_versions.sh to support arbitrary XML tags instead of only <version>. However, the current regular expression matches the first leaf XML tag on a line, which can cause XML corruption if a line contains multiple tags (such as both groupId and version). To prevent this, the reviewer suggests anchoring the match to the specific x-version-update comment on the same line.

K=${KV%:*}; V=${KV#*:}
echo Key:$K, Value:$V;
echo "/x-version-update:$K:current/{s|<version>.*<\/version>|<version>$V<\/version>|;}" >> "$SED_SCRIPT_FILE"
echo '/x-version-update:'"$K"':current/{s|<\([^>]*\)>[^<]*</\1>|<\1>'"$V"'</\1>|;}' >> "$SED_SCRIPT_FILE"

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.

high

Issue: Potential XML Corruption on Lines with Multiple Tags

The current regular expression <\([^>]*\)>[^<]*</\1> matches the first leaf XML tag on the line, regardless of whether it is the tag intended for the version update.

If a line contains multiple tags (for example, a dependency definition on a single line or multiple properties), like:

<groupId>com.google.cloud</groupId> <version>1.0.0</version> <!-- x-version-update:my-dep:current -->

The regex will match <groupId>com.google.cloud</groupId> instead of <version>1.0.0</version>, resulting in the corruption of the groupId tag:

<groupId>1.2.3</groupId> <version>1.0.0</version> <!-- x-version-update:my-dep:current -->

Solution: Anchor the Match to the Comment

We can prevent this by anchoring the match to the specific x-version-update comment on the same line, ensuring that no other tags (i.e., no < characters) exist between the closing tag and the comment. This guarantees that only the tag immediately preceding the comment is updated.

Suggested change
echo '/x-version-update:'"$K"':current/{s|<\([^>]*\)>[^<]*</\1>|<\1>'"$V"'</\1>|;}' >> "$SED_SCRIPT_FILE"
echo '/x-version-update:'"$K"':current/{s|<\([^>]*\)>[^<]*</\1>\([^<]*x-version-update:'"$K"':current\)|<\1>'"$V"'</\1>\2|;}' >> "$SED_SCRIPT_FILE"

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