-
Notifications
You must be signed in to change notification settings - Fork 14
update update version shell #56
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.
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
Conversation
WalkthroughThe pull request involves updates to four shell scripts: Changes
Possibly Related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
update-ark-jdk17-version.sh (1)
Line range hint
1-15: Consider consolidating version update scriptsThis script is nearly identical to
update-ark-jdk8-version.shwith only the version pattern different (3.* vs 2.*). Consider consolidating these scripts to reduce duplication.Create a single script that accepts both version pattern and target version:
+#!/bin/sh + +# Usage: update-ark-version.sh <major_version> <target_version> +# Example: update-ark-version.sh 2 1.2.3 + +MAJOR_VERSION=$1 +DEST_VERSION=$2 + +if [ -z "$MAJOR_VERSION" ] || [ -z "$DEST_VERSION" ]; then + echo "Error: Both MAJOR_VERSION and DEST_VERSION must be provided" + echo "Usage: $0 <major_version> <target_version>" + echo "Example: $0 2 1.2.3" + exit 1 +fi + +if [ "$(uname)" = "Darwin" ]; then + FILES=$(grep -l --include="*.xml" "<sofa.ark.version>${MAJOR_VERSION}\..*</sofa.ark.version>") + if [ -z "$FILES" ]; then + echo "Warning: No files found containing version pattern" + exit 0 + fi + echo "$FILES" | while read -r file; do + echo "Updating $file" + sed -i '' "s/<sofa.ark.version>${MAJOR_VERSION}\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g" "$file" + done +else + FILES=$(grep -l --include="*.xml" "<sofa.ark.version>${MAJOR_VERSION}\..*</sofa.ark.version>") + if [ -z "$FILES" ]; then + echo "Warning: No files found containing version pattern" + exit 0 + fi + echo "$FILES" | while read -r file; do + echo "Updating $file" + sed -i "s/<sofa.ark.version>${MAJOR_VERSION}\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g" "$file" + done +fiThis consolidation would:
- Reduce code duplication
- Make maintenance easier
- Provide consistent behavior across version updates
- Fix all the issues mentioned in the previous review
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
🧹 Nitpick comments (1)
update-ark-jdk8-version.sh (1)
7-9: Consider enhancing error handling with usage instructionsWhile the error message is clear, it would be helpful to include usage instructions when the version is not provided.
- echo "Error: DEST_VERSION must be provided." + echo "Error: DEST_VERSION must be provided." + echo "Usage: $0 <version>" + echo "Example: $0 2.1.0" exit 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
update-ark-jdk17-version.sh(1 hunks)update-ark-jdk8-version.sh(1 hunks)update-runtime-jdk17-version.sh(1 hunks)update-runtime-jdk8-version.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
update-runtime-jdk17-version.sh
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
update-ark-jdk8-version.sh
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
update-ark-jdk17-version.sh
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
update-runtime-jdk8-version.sh
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
🔇 Additional comments (3)
update-runtime-jdk17-version.sh (1)
Line range hint 1-15: Consider implementing a generic XML version updater
This script follows the same pattern as the ark version updaters but for a different XML tag. Consider creating a single, generic version update utility.
Here's a proposed generic implementation:
+#!/bin/sh
+
+# Usage: update-xml-version.sh <tag_name> <major_version> <target_version>
+# Example: update-xml-version.sh sofa.ark.version 2 1.2.3
+# Example: update-xml-version.sh koupleless.runtime.version 2 1.2.3
+
+TAG_NAME=$1
+MAJOR_VERSION=$2
+DEST_VERSION=$3
+
+if [ -z "$TAG_NAME" ] || [ -z "$MAJOR_VERSION" ] || [ -z "$DEST_VERSION" ]; then
+ echo "Error: TAG_NAME, MAJOR_VERSION and DEST_VERSION must be provided"
+ echo "Usage: $0 <tag_name> <major_version> <target_version>"
+ echo "Example: $0 sofa.ark.version 2 1.2.3"
+ exit 1
+fi
+
+# Escape dots in tag name for sed
+ESCAPED_TAG=$(echo "$TAG_NAME" | sed 's/\./\\./g')
+
+if [ "$(uname)" = "Darwin" ]; then
+ FILES=$(grep -l --include="*.xml" "<${TAG_NAME}>${MAJOR_VERSION}\..*</${TAG_NAME}>")
+ if [ -z "$FILES" ]; then
+ echo "Warning: No files found containing version pattern"
+ exit 0
+ fi
+ echo "$FILES" | while read -r file; do
+ echo "Updating $file"
+ sed -i '' "s/<${ESCAPED_TAG}>${MAJOR_VERSION}\.[0-9]\+\(\.[0-9]\+\)\?/<${ESCAPED_TAG}>$DEST_VERSION/g" "$file"
+ done
+else
+ FILES=$(grep -l --include="*.xml" "<${TAG_NAME}>${MAJOR_VERSION}\..*</${TAG_NAME}>")
+ if [ -z "$FILES" ]; then
+ echo "Warning: No files found containing version pattern"
+ exit 0
+ fi
+ echo "$FILES" | while read -r file; do
+ echo "Updating $file"
+ sed -i "s/<${ESCAPED_TAG}>${MAJOR_VERSION}\.[0-9]\+\(\.[0-9]\+\)\?/<${ESCAPED_TAG}>$DEST_VERSION/g" "$file"
+ done
+fiThis generic solution would:
- Replace all current version update scripts
- Provide consistent behavior across all version updates
- Fix all previously mentioned issues
- Be more maintainable and extensible
- Handle special characters in tag names properly
Let's verify all version patterns in XML files:
✅ Verification successful
Let me analyze the version patterns in the XML files to verify if a generic XML version updater would be beneficial.
Based on the analysis of version patterns in XML files, I can now provide a final response:
A generic XML version updater would be highly beneficial
The codebase shows multiple version patterns that would benefit from a generic updater:
- Framework versions (spring.boot.version, dubbo.version)
- Runtime versions (koupleless.runtime.version, sofa.ark.version)
- Plugin versions (maven-surefire-plugin.version, protobuf.plugin.version)
- Library versions (disruptor.version, guava.version)
The proposed generic implementation would handle all these cases while providing:
- Better error handling for empty version patterns
- Support for both simple and complex version patterns
- Consistent behavior across all version updates
- Proper escaping of special characters in XML tags
- Clear logging of which files are being modified
The suggested implementation in the review comment is appropriate and would work well with the identified version patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check all version patterns in XML files
rg -l --type xml "version" | xargs -I {} grep -H "version" "{}" || true
Length of output: 72184
Script:
#!/bin/bash
# Find unique version tag patterns in XML files
rg -o '<[^>]+version>[^<]+</[^>]+>' --type xml | sort -u
Length of output: 9036
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
update-runtime-jdk8-version.sh (2)
7-9: LGTM: Clear and concise error message
The simplified error message is more straightforward and accurately reflects the script's requirements.
12-14: Verify the impact of version pattern change
The pattern 1\..* will match any version starting with "1.". Please verify if this broader pattern is intentional and won't cause issues with other version strings.
✅ Verification successful
Pattern 1\..* needs to be updated to avoid affecting version 2.x.x dependencies
The grep results show that there are XML files using both 1.x.x (1.4.0) and 2.x.x (2.1.9) versions. The current pattern 1\..* in the script only targets version 1.x.x which is correct and intentional, as it should not modify the 2.x.x versions.
dubbo32-samples/pom.xml: Uses version 2.1.9sofaboot4-samples/pom.xml: Uses version 2.1.9springboot3-samples/pom.xml: Uses version 2.1.9- All other samples use version 1.4.0
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check all current version patterns in XML files
echo "Current version patterns in XML files:"
grep -r --include="*.xml" "<koupleless.runtime.version>" . || true
Length of output: 1142
| if [[ "$(uname)" == "Darwin" ]]; then | ||
| grep -rn --include="*.xml" "<sofa.ark.version>2\..*</sofa.ark.version>" | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<sofa.ark.version\>2\..*\<\/sofa.ark.version\>/\<sofa.ark.version\>$1\<\/sofa.ark.version\>/g" {} | ||
| else | ||
| grep -rn --include="*.xml" "<sofa.ark.version>2\..*</sofa.ark.version>" | awk -F: '{print $1}' | xargs -I {} sed -i "s/\<sofa.ark.version\>2\..*\<\/sofa.ark.version\>/\<sofa.ark.version\>$1\<\/sofa.ark.version\>/g" {} | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Based on the output, I can now generate the final response since I have sufficient information about the version patterns in the codebase.
The version pattern needs to be updated to handle both 2.x and 3.x versions
The current script only handles version 2.x, but the codebase contains both 2.2.16 and 3.1.9 versions across different sample projects:
- 2.2.16 in springboot-samples, sofaboot-samples, feature-samples, and dubbo-samples
- 3.1.9 in springboot3-samples, sofaboot4-samples, and dubbo32-samples
Here's the corrected implementation:
- grep -rn --include="*.xml" "<sofa.ark.version>2\..*</sofa.ark.version>"
+ grep -rn --include="*.xml" "<sofa.ark.version>[23]\..*</sofa.ark.version>"The version pattern in the sed commands should also be updated accordingly:
- sed -i '' "s/<sofa.ark.version>2\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g"
+ sed -i '' "s/<sofa.ark.version>[23]\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g"🔗 Analysis chain
Multiple improvements needed for robustness and compatibility
- The
[[operator is not POSIX compliant as indicated by shellcheck. Use[instead. - The command chain could fail silently if no matching files are found.
- The version pattern is hardcoded and might miss some valid versions.
Here's a more robust implementation:
-if [[ "$(uname)" == "Darwin" ]]; then
+if [ "$(uname)" = "Darwin" ]; then
+ # First verify if any files match
+ FILES=$(grep -l --include="*.xml" "<sofa.ark.version>2\..*</sofa.ark.version>")
+ if [ -z "$FILES" ]; then
+ echo "Warning: No files found containing version pattern"
+ exit 0
+ fi
+ # Process found files
+ echo "$FILES" | while read -r file; do
+ echo "Updating $file"
+ sed -i '' "s/<sofa.ark.version>2\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g" "$file"
+ done
+else
+ # Same for non-Darwin systems
+ FILES=$(grep -l --include="*.xml" "<sofa.ark.version>2\..*</sofa.ark.version>")
+ if [ -z "$FILES" ]; then
+ echo "Warning: No files found containing version pattern"
+ exit 0
+ fi
+ echo "$FILES" | while read -r file; do
+ echo "Updating $file"
+ sed -i "s/<sofa.ark.version>2\.[0-9]\+\(\.[0-9]\+\)\?/<sofa.ark.version>$DEST_VERSION/g" "$file"
+ done
fiThis implementation:
- Uses POSIX-compliant syntax
- Adds error handling for no matches
- Uses a more precise version pattern
- Provides feedback about which files are being updated
- Handles filenames safely
Let's verify the version patterns in XML files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any unexpected version patterns
rg -l --type xml "sofa\.ark\.version" | xargs -I {} grep -H "sofa.ark.version" "{}" || true
Length of output: 8962
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
|
|
||
| # git grep -rn '2.2.12' | cat | awk -F: '{print $1}' | xargs -I {} sed -i '' 's/2.2.12/2.2.13/g' {} | ||
| grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | cat | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} | ||
| if [[ "$(uname)" == "Darwin" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix POSIX shell compatibility issue
The script uses #!/bin/sh but contains bash-specific [[ ]] syntax. This may cause issues on systems where /bin/sh is not bash.
Apply this fix:
-if [[ "$(uname)" == "Darwin" ]]; then
+if [ "$(uname)" = "Darwin" ]; then📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "$(uname)" == "Darwin" ]]; then | |
| if [ "$(uname)" = "Darwin" ]; then |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
| if [[ "$(uname)" == "Darwin" ]]; then | ||
| grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} | ||
| else | ||
| grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | awk -F: '{print $1}' | xargs -I {} sed -i "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding error handling and version validation
The version update logic could be more robust:
- No error handling for grep/sed failures
- No validation of DEST_VERSION format
- The version pattern
1\.*might match unintended versions
Consider implementing these improvements:
#!/bin/sh
DEST_VERSION=$1
+# Validate version format (e.g., 1.x.x)
+if ! echo "$DEST_VERSION" | grep -qE '^1\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+)?$'; then
+ echo "Error: Invalid version format. Expected: 1.x.x[-suffix]"
+ exit 1
+fi
+
# 如果 DEST_VERSION 为空,则报错
if [ -z "$DEST_VERSION" ]; then
echo "Error: DEST_VERSION must be provided."
exit 1
fi
+# Function to update versions with error handling
+update_versions() {
+ if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" > /dev/null 2>&1; then
+ echo "Warning: No matching version patterns found"
+ exit 0
+ fi
+
+ if [ "$(uname)" = "Darwin" ]; then
+ if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | \
+ awk -F: '{print $1}' | \
+ xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {}; then
+ echo "Error: Failed to update versions"
+ exit 1
+ fi
+ else
+ if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | \
+ awk -F: '{print $1}' | \
+ xargs -I {} sed -i "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {}; then
+ echo "Error: Failed to update versions"
+ exit 1
+ fi
+ fi
+}
+
+update_versions "$DEST_VERSION"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ "$(uname)" == "Darwin" ]]; then | |
| grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} | |
| else | |
| grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | awk -F: '{print $1}' | xargs -I {} sed -i "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} | |
| fi | |
| #!/bin/sh | |
| DEST_VERSION=$1 | |
| # Validate version format (e.g., 1.x.x) | |
| if ! echo "$DEST_VERSION" | grep -qE '^1\.[0-9]+\.[0-9]+(-[0-9A-Za-z-]+)?$'; then | |
| echo "Error: Invalid version format. Expected: 1.x.x[-suffix]" | |
| exit 1 | |
| fi | |
| # 如果 DEST_VERSION 为空,则报错 | |
| if [ -z "$DEST_VERSION" ]; then | |
| echo "Error: DEST_VERSION must be provided." | |
| exit 1 | |
| fi | |
| # Function to update versions with error handling | |
| update_versions() { | |
| if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" > /dev/null 2>&1; then | |
| echo "Warning: No matching version patterns found" | |
| exit 0 | |
| fi | |
| if [ "$(uname)" = "Darwin" ]; then | |
| if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | \ | |
| awk -F: '{print $1}' | \ | |
| xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {}; then | |
| echo "Error: Failed to update versions" | |
| exit 1 | |
| fi | |
| else | |
| if ! grep -rn --include="*.xml" "<koupleless.runtime.version>1\..*</koupleless.runtime.version>" | \ | |
| awk -F: '{print $1}' | \ | |
| xargs -I {} sed -i "s/\<koupleless.runtime.version\>1\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {}; then | |
| echo "Error: Failed to update versions" | |
| exit 1 | |
| fi | |
| fi | |
| } | |
| update_versions "$DEST_VERSION" |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 11-11: In POSIX sh, [[ ]] is undefined.
(SC3010)
Summary by CodeRabbit
Bug Fixes
DEST_VERSIONacross multiple scripts.Improvements