feat: replace source build with binary download for benchx_cli#2249
feat: replace source build with binary download for benchx_cli#2249colinaaa merged 1 commit intolynx-family:mainfrom
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the benchx_cli source build with a release-download workflow: removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/renovate.json5:
- Around line 198-205: The Renovate entry for the benchx_cli github-releases
datasource lacks a versioningTemplate, so non-semver tags like
"benchx_cli-202602132156" may be parsed incorrectly; update the object that
contains "customType", "matchStrings", and "depNameTemplate" to add a
"versioningTemplate" property that either (a) uses a regex to parse the
benchx_cli-YYYYMMDDHHMM style tags (matching year, month, day, time groups) or
(b) sets a loose/non-semver scheme so Renovate can sort and compare those
release tags correctly.
In `@packages/lynx/benchx_cli/scripts/build.sh`:
- Around line 62-69: The curl call that downloads URL built from LOCKED_VERSION
and ASSET_NAME can silently save HTTP error pages and cause tar -xzf to fail;
update the curl invocation that writes to TAR_PATH to include --fail (or -f) so
curl exits non‑zero on 4xx/5xx, and optionally check the curl exit status and
remove the partial TAR_PATH before exiting to avoid passing an invalid file to
tar (locate the line with curl -L --progress-bar -o "$TAR_PATH" "$URL" and add
--fail/-f and a brief exit-on-error handling).
🧹 Nitpick comments (2)
packages/lynx/benchx_cli/scripts/build.sh (2)
62-72: No integrity verification of the downloaded binary.The script downloads and executes a binary from a remote URL without any checksum or signature verification. A compromised release or MITM (on non-HTTPS fallback) could inject a malicious binary. Consider adding a SHA-256 checksum check against a known digest.
37-54: Add a clarifying comment about intentional platform support limits.The script currently supports Darwin/arm64 and Linux/x86_64, matching the published release assets. These are the only platforms with published binaries, which is reflected correctly in the build script. Consider adding a comment (e.g., above line 37) explaining why only these platforms are supported, so future maintainers understand this is intentional rather than an oversight. This will prevent accidental attempts to add support for Linux aarch64 or Darwin x86_64 without corresponding release artifact builds.
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous source-based build for packages/lynx/benchx_cli (which required fetching/cherry-picking from upstream and compiling) with a faster, version-locked binary download approach from GitHub Releases.
Changes:
- Added a Bash build script that downloads a prebuilt
benchx_clitarball for supported platforms and writes it todist/bin. - Removed the Node/zx-based build script and updated
packages/lynx/benchx_cli/package.jsonto run the new shell script. - Updated Renovate configuration to track
LOCKED_VERSIONinbuild.shvia thegithub-releasesdatasource.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/lynx/benchx_cli/scripts/build.sh | New CI-gated build that downloads a locked release asset and extracts it into dist/bin. |
| packages/lynx/benchx_cli/scripts/build.mjs | Removes the previous source build pipeline (git fetch/cherry-pick + GN/Ninja build). |
| packages/lynx/benchx_cli/package.json | Switches build script to the new shell script and removes zx dependency from the package manifest. |
| .github/renovate.json5 | Adjusts custom regex manager to update LOCKED_VERSION from GitHub Releases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hzy
force-pushed
the
feature/benchx-cli-download
branch
3 times, most recently
from
1a45735 to
0e8593d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/lynx/benchx_cli/package.json (1)
9-13:⚠️ Potential issue | 🟡 MinorRemove the unused
zxdependency.The
build.mjsscript that depended onzxhas been removed and replaced withbuild.sh, a pure bash script that does not use Node.js orzx. Thezxdependency independenciesis no longer needed and should be removed.Proposed fix
"dependencies": { - "zx": "^8.8.5" - } + }Or remove the
dependenciesfield entirely if empty.
hzy
force-pushed
the
feature/benchx-cli-download
branch
4 times, most recently
from
0f06e4f to
eb9404c
Compare
This change replaces the complex source build process (which required various dependencies like git, ninja, rust, etc.) with a simple binary download from GitHub Releases. The script now downloads pre-built binaries for macOS and Linux, ensuring faster and more reliable setup. Key changes: - Replaced with for simpler dependency-free execution. - Updated to use the new shell script. - Configured Renovate to monitor for updates via GitHub Releases.
hzy
force-pushed
the
feature/benchx-cli-download
branch
from
eb9404c to
3985e2d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will degrade performance by 14.18%
Performance Changes
Comparing 
Footnotes
|
Web Explorer#7675 Bundle Size — 383.74KiB (0%).3985e2d(current) vs 87682f6 main#7667(baseline) Bundle metrics
|
| Current #7675 |
Baseline #7667 |
|
|---|---|---|
 Initial JS |
154.88KiB |
154.88KiB |
| Initial CSS |
35.06KiB |
35.06KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
8 |
8 |
| Assets |
8 |
8 |
 Modules |
238(+0.42%) |
237 |
| Duplicate Modules |
16 |
16 |
| Duplicate Code |
2.99% |
2.99% |
| Packages |
4 |
4 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #7675 |
Baseline #7667 |
|
|---|---|---|
| JS |
252.83KiB |
252.83KiB |
| Other |
95.85KiB |
95.85KiB |
| CSS |
35.06KiB |
35.06KiB |
Bundle analysis report Branch hzy:feature/benchx-cli-download Project dashboard
Generated by RelativeCI Documentation Report issue
Summary
Replaced the complex source build process for
benchx_cliwith a lightweight binary download script. This change significantly speeds up the setup process and removes build-time dependencies.Changes
packages/lynx/benchx_cli/scripts/build.mjswithbuild.sh:package.json:buildscript to run the new shell script.zxdependency as it's no longer needed for the build script..github/renovate.json5:LOCKED_VERSIONinbuild.shagainst GitHub Releases instead of git tags/commits.Verification
pnpm buildinpackages/lynx/benchx_clicorrectly downloads and extracts the binary.Summary by CodeRabbit
Refactor
Chores