refactor: unify release validation script and add local path support#19
refactor: unify release validation script and add local path support#19imbajin merged 6 commits intohugegraph:masterfrom
Conversation
Bumped default release version to 1.7.0 and updated default GPG user in the release workflow. Added installation step for subversion on Ubuntu. Appended WARP.md to .gitignore.
Deleted validate-release-in-local.sh and enhanced validate-release.sh to support both SVN and local directory validation. Added color-coded output, improved argument handling, and included Java version checks for better usability and error reporting.
Major rewrite of validate-release.sh for Apache HugeGraph, adding modular structure, improved logging, error/warning collection, colorized output, and comprehensive validation steps for source and binary packages. New features include dependency checks, GPG key management, license compliance, file size and binary checks, version consistency, and automated server/toolchain testing. Usage instructions and help output are expanded for clarity.
…s V3 Updated CI workflow to support additional OS and architectures (arm64, macOS 14). Improved documentation and script usage instructions. The license header check now covers more file types and excludes generated/vendor files. Maven build commands in docs and scripts now use '-DskipTests' and '-Dcheckstyle.skip=true' for consistency. Added a detailed README for the release validation script.
Adds contextual error and warning reporting with step and package information, enhances license category and header checks, improves version consistency logic, and refines summary output with execution time and clearer formatting. These changes make validation results more actionable and easier to interpret, especially for multi-package and multi-step validations.
|
@codecov-ai-reviewer review |
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough更新 CI 发布验证工作流及文档:调整 workflow_dispatch 默认值(release_version、gpg_user)、扩展 OS 矩阵并为 Ubuntu 添加 SVN 安装步骤,扩展验证筛选(包含 JSON),并更改文档中 Maven 跳过测试的标志;同时将 Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as 开发者 / Trigger
participant GH as GitHub Actions
participant Runner as Runner (Ubuntu / macOS)
rect rgb(220,240,255)
Dev->>GH: 手动触发 validate-release (release_version, gpg_user)
GH->>Runner: 启动 job(选择 matrix OS)
end
rect rgb(240,255,230)
Runner->>Runner: Checkout sources
alt OS == ubuntu-*
Runner->>Runner: apt-get install subversion
end
Runner->>Runner: 下载 SVN 源
Runner->>Runner: 构建/验证:检查 source/binary 包(CATEGORY_X 含 JSON)
Runner->>GH: 发布验证结果(成功 / 失败)
end
note right of GH: 新增 OS 矩阵项与 Ubuntu 特定安装步骤为关键变更点
估计代码审查工作量🎯 3 (中等) | ⏱️ ~20分钟
可能相关的PR
诗
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @imbajin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly overhauls the release validation process by consolidating and enhancing the existing validation scripts into a single, powerful Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, unifying the release validation scripts and adding support for local paths. The new validate-release.sh script is much more comprehensive and robust, with excellent features like detailed logging, step-by-step progress, and a final summary report. The new README.md is also very helpful.
I've reviewed the new script and have a few suggestions to further improve its robustness and portability. These are mainly related to argument parsing, command portability, and making some checks less fragile. Overall, this is a very solid contribution.
| local found=$(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null | head -n1) | ||
|
|
||
| if [[ -z "$found" ]]; then | ||
| collect_error "Could not find directory matching pattern: $pattern" | ||
| return 1 | ||
| fi | ||
|
|
||
| echo "$found" |
There was a problem hiding this comment.
The find_package_dir function uses head -n1 to select the first directory that matches a pattern. If multiple directories match (e.g., from previous failed runs), the script will silently pick the first one, which could be incorrect and lead to validating the wrong package. It would be more robust to detect multiple matches and fail with an error, guiding the user to clean up the directory.
| local found=$(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null | head -n1) | |
| if [[ -z "$found" ]]; then | |
| collect_error "Could not find directory matching pattern: $pattern" | |
| return 1 | |
| fi | |
| echo "$found" | |
| local found_dirs | |
| mapfile -t found_dirs < <(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null) | |
| local num_found=${#found_dirs[@]} | |
| if [[ $num_found -eq 0 ]]; then | |
| collect_error "Could not find directory matching pattern: $pattern" | |
| return 1 | |
| fi | |
| if [[ $num_found -gt 1 ]]; then | |
| collect_error "Found multiple directories matching pattern '$pattern'. Please clean up the directory." | |
| printf ' %s\n' "${found_dirs[@]}" >&2 | |
| return 1 | |
| fi | |
| echo "${found_dirs[0]}" |
| RELEASE_VERSION=${1:-} | ||
| USER=${2:-} | ||
| LOCAL_DIST_PATH=${3:-} | ||
| JAVA_VERSION=${4:-11} |
There was a problem hiding this comment.
The current positional argument parsing is fragile. If a user wants to specify JAVA_VERSION but not LOCAL_DIST_PATH, they must provide an empty string ("") as the third argument. If they omit it and pass the Java version as the third argument (e.g., ./validate-release.sh 1.7.0 myuser 11), the script will incorrectly interpret 11 as the LOCAL_DIST_PATH, leading to failure. This is not user-friendly. Consider using getopts for parsing arguments, or at least adding a check to handle this common misuse scenario.
|
|
||
| # Check SHA512 | ||
| TOTAL_CHECKS=$((TOTAL_CHECKS + 1)) | ||
| if shasum -a 512 --check "${pkg}.sha512"; then |
There was a problem hiding this comment.
The script uses shasum -a 512 to verify checksums. While this works on macOS and some Linux systems, the more common command on Linux is sha512sum (from GNU Coreutils). To improve portability for all contributors, it would be better to prefer sha512sum and fall back to shasum if it's not available. This would involve updating the dependency check and this line.
|
|
||
| # Check GPG signature | ||
| TOTAL_CHECKS=$((TOTAL_CHECKS + 1)) | ||
| if gpg --verify "${pkg}.asc" "$pkg" 2>&1 | grep -q "Good signature"; then |
There was a problem hiding this comment.
The GPG signature verification relies on parsing the string "Good signature" from the command's output. This approach can be brittle, as the output message might change with different GPG versions or system locales (languages). A more robust method is to check the exit code of gpg --verify, which is 0 for a valid signature, and non-zero otherwise.
| if gpg --verify "${pkg}.asc" "$pkg" 2>&1 | grep -q "Good signature"; then | |
| if gpg --verify "${pkg}.asc" "$pkg"; then |
The JSON license was added to the CATEGORY_X regex in the binary package validation step to ensure packages with this license are properly flagged during release validation.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/validate-release.yml (1)
108-108: CATEGORY_X许可证过滤规则中的JSON模式缺少单词边界。在第108行和第247行添加
|JSON时,建议使用\bJSON\b来避免误匹配包含"JSON"字样但实际是其他许可证名称的文本。当前模式"JSON"可能在LICENSE或NOTICE文件中捕获如"Parsing JSON"这样的误匹配。建议应用以下变更以提高模式精确性:
-CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial|JSON" +CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial|\bJSON\b"此变更应应用于第108行和第247行。
Also applies to: 247-247
🧹 Nitpick comments (1)
content/cn/docs/contribution-guidelines/validate-release.md (1)
128-130: Maven标志更新正确,但建议对齐英文版的标志顺序。中文版的Maven命令已正确更新为
-DskipTests。为了便于维护和文档一致性,建议调整标志顺序以匹配英文版:mvn clean package -P stage -DskipTests -Dcheckstyle.skip=true。建议应用以下变更:
-mvn clean package -DskipTests -Dcheckstyle.skip=true -P stage +mvn clean package -P stage -DskipTests -Dcheckstyle.skip=true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
dist/README.mdis excluded by!**/dist/**dist/validate-release-in-local.shis excluded by!**/dist/**dist/validate-release.shis excluded by!**/dist/**
📒 Files selected for processing (4)
.github/workflows/validate-release.yml(4 hunks).gitignore(1 hunks)content/cn/docs/contribution-guidelines/validate-release.md(1 hunks)content/en/docs/contribution-guidelines/validate-release.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (5)
.gitignore (1)
31-31: 变更符合现有的文件忽略规则模式。添加WARP.md与其他AI Agent配置文件(CLAUDE.md、GEMINI.md等)的忽略规则保持一致。
content/en/docs/contribution-guidelines/validate-release.md (1)
139-141: Maven标志更新遵循最佳实践。从
-Dmaven.test.skip=true更改为-DskipTests是现代Maven的推荐做法。命令标志组合正确,注释也准确。.github/workflows/validate-release.yml (3)
9-9: 默认值更新合理。将
release_version更新至1.7.0和gpg_user更新至Junzhi Peng反映了当前发版配置的需要。Also applies to: 13-13
70-72: SVN安装步骤必要且正确。添加Ubuntu特定的SVN安装命令确保了Linux CI环境的完整性,与macOS的
brew install svn相对应。
353-354: OS矩阵扩展增强了测试覆盖范围。扩展支持ubuntu-24.04-arm(ARM架构)和macos-14(最新macOS)提高了跨架构和跨平台的验证覆盖。
There was a problem hiding this comment.
Pull Request Overview
This PR successfully unifies two separate release validation scripts into a single, more robust solution with enhanced error handling, comprehensive reporting, and support for both SVN download and local directory validation modes.
Key Changes:
- Merged
validate-release.shandvalidate-release-in-local.shinto a single unified script - Added optional local path parameter for validating pre-downloaded release files
- Implemented comprehensive error collection and reporting with detailed summaries
- Added extensive logging, progress tracking, and colorized output
Reviewed Changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| dist/validate-release.sh | Complete rewrite (~340→1384 lines) with modular functions, improved error handling, colored output, comprehensive validation checks including ASF license headers, version consistency, and detailed reporting |
| dist/validate-release-in-local.sh | Removed - functionality merged into unified validate-release.sh with local path support via optional parameter |
| dist/README.md | New comprehensive Chinese documentation covering features, usage, validation steps, output formats, troubleshooting, and references |
| content/en/docs/contribution-guidelines/validate-release.md | Updated Maven command to use -DskipTests instead of -Dmaven.test.skip=true with note that -P flag is optional |
| content/cn/docs/contribution-guidelines/validate-release.md | Updated Maven command to use -DskipTests and reordered -P stage flag to end of command |
| .gitignore | Added WARP.md to ignore list |
| .github/workflows/validate-release.yml | Updated default version to 1.7.0, changed default GPG user, added ARM architecture support (ubuntu-24.04-arm, macos-14), fixed incomplete CATEGORY_X regex pattern, added subversion installation for Ubuntu |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| cd dist/${{ inputs.release_version }} || exit | ||
| CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial" | ||
| CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial|JSON" |
There was a problem hiding this comment.
Incomplete regex pattern: The pattern ends with |JSON but should be |JSON\.org to match the source script. The backslash and .org are missing, which could result in false positives (matching any occurrence of "JSON" instead of specifically "JSON.org" license references).
| CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial|JSON" | |
| CATEGORY_X="\bGPL|\bLGPL|Sleepycat License|BSD-4-Clause|\bBCL\b|JSR-275|Amazon Software License|\bRSAL\b|\bQPL\b|\bSSPL|\bCPOL|\bNPL1|Creative Commons Non-Commercial|JSON\.org" |
| # TODO: support windows-latest or other OS in future | ||
| os: [ubuntu-latest, macos-latest] | ||
| # Support multiple OS and architectures (x64 and arm64) | ||
| os: [ubuntu-latest, ubuntu-24.04-arm, macos-latest, macos-14] |
There was a problem hiding this comment.
The matrix configuration adds ARM architecture support (ubuntu-24.04-arm, macos-14) but there's no verification that these runners are available or that the validation script works correctly on ARM architectures. The comment mentions "Support multiple OS and architectures" but:
ubuntu-24.04-armmay not be a valid GitHub-hosted runner (GitHub typically usesubuntu-latestfor both x64 and ARM)macos-14is an M1/ARM Mac, which is good, but the combination should be tested
Consider verifying these runner labels are correct and available in your GitHub environment.
| os: [ubuntu-latest, ubuntu-24.04-arm, macos-latest, macos-14] | |
| os: [ubuntu-latest, ubuntu-24.04, macos-latest, macos-14] |
|
|
||
| # 尝试在 Unix 环境下编译测试是否正常 (stage 表示从 stage 仓库拉取依赖) | ||
| mvn clean package -P stage -Dmaven.test.skip=true -Dcheckstyle.skip=true | ||
| # 尝试在 Unix 环境下编译测试是否正常 |
There was a problem hiding this comment.
Same documentation inconsistency as the English version: The command includes -P stage but the comment changed from explicitly mentioning stage repository to a generic compilation comment. The -P stage flag appears at the end now, which is better, but the purpose should still be documented.
| # 尝试在 Unix 环境下编译测试是否正常 | |
| # 尝试在 Unix 环境下编译测试是否正常,注意使用 `-P stage` 以便使用 stage 仓库进行编译 |
|
|
||
| # try to compile in the Unix env to check if it works well | ||
| mvn clean package -P stage -Dmaven.test.skip=true -Dcheckstyle.skip=true | ||
| # try to compile in the Unix env to check if it works well (-P is optional) |
There was a problem hiding this comment.
Documentation improvement: The comment change mentions "(-P is optional)" but the -P stage flag is still included in the example command. This could be confusing.
Consider either:
- Moving
-P stageto the end and noting it's optional:mvn clean package -DskipTests -Dcheckstyle.skip=true [-P stage] - Showing two examples: one with and one without the
-P stageflag - Removing the optional note if
-P stageis actually required for validation
The same issue exists in the Chinese documentation at line 129.
| # try to compile in the Unix env to check if it works well (-P is optional) | |
| # try to compile in the Unix env to check if it works well (-P is optional) | |
| mvn clean package -DskipTests -Dcheckstyle.skip=true | |
| # or, if you want to use the 'stage' profile: |
Purpose of the PR
Summary by CodeRabbit
发布说明
文档
Chores
其他