fix(release): use cross main branch to fix mdbx-sys build#84
fix(release): use cross main branch to fix mdbx-sys build#84
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:
📝 WalkthroughWalkthroughReworks CI and local build to use cargo-installed cross and Makefile cross targets, removes Cross.toml pre-build APT steps, adds Dockerfile.cross and multi-arch staging, and adjusts Docker workflow tagging and build steps. No exported/public code entities changed. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
102-103: Consider pinning to a specific cross commit for reproducibility.Installing from the main branch (
--git https://github.com/cross-rs/cross) means builds could break unexpectedly if cross introduces breaking changes. While this solves the Ubuntu 16.04 issue, consider pinning to a specific commit or tag once a stable release with Ubuntu 20.04 images is available:run: cargo install cross --git https://github.com/cross-rs/cross --rev <commit-sha>That said, using main is a reasonable approach given the immediate build failure and alignment with reth/scroll-reth practices.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 102 - 103, The workflow step that installs cross from the main branch currently uses "--git https://github.com/cross-rs/cross" which risks non-reproducible builds; change the cargo install invocation to pin to a specific commit or tag by adding "--rev <commit-sha-or-tag>" (replace <commit-sha-or-tag> with the chosen commit SHA or release tag), and update the step name/description if desired (the step labeled "Install cross main") so the intent is clear; pick a stable commit from the cross repo (or a released tag) that includes the needed Ubuntu 20.04 images and use that SHA in the --rev flag to ensure reproducible CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/test-cross-build.yml:
- Around line 11-14: Add a top-level GitHub Actions permissions block to
restrict GITHUB_TOKEN to least privilege (only read access to repository
contents) for this workflow; update the workflow that defines the jobs/build
(look for the jobs: build: name: Cross Build and runs-on: ubuntu-latest entries)
to include a permissions: contents: read stanza so the workflow only has read
access to repository contents.
---
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 102-103: The workflow step that installs cross from the main
branch currently uses "--git https://github.com/cross-rs/cross" which risks
non-reproducible builds; change the cargo install invocation to pin to a
specific commit or tag by adding "--rev <commit-sha-or-tag>" (replace
<commit-sha-or-tag> with the chosen commit SHA or release tag), and update the
step name/description if desired (the step labeled "Install cross main") so the
intent is clear; pick a stable commit from the cross repo (or a released tag)
that includes the needed Ubuntu 20.04 images and use that SHA in the --rev flag
to ensure reproducible CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebc5efd9-1dbe-40e8-bb82-923ce43d1afb
📒 Files selected for processing (3)
.github/workflows/release.yml.github/workflows/test-cross-build.ymlCross.toml
💤 Files with no reviewable changes (1)
- Cross.toml
| jobs: | ||
| build: | ||
| name: Cross Build ${{ matrix.target }} | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
Add permissions block to limit GITHUB_TOKEN scope.
The workflow is missing a permissions block. Even for a temporary test workflow, it's good practice to apply the principle of least privilege. Since this workflow only checks out code and builds, it only needs read access.
🔒 Proposed fix to add permissions
env:
CARGO_TERM_COLOR: always
CARGO_INCREMENTAL: 0
+permissions:
+ contents: read
+
jobs:
build:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/test-cross-build.yml around lines 11 - 14, Add a top-level
GitHub Actions permissions block to restrict GITHUB_TOKEN to least privilege
(only read access to repository contents) for this workflow; update the workflow
that defines the jobs/build (look for the jobs: build: name: Cross Build and
runs-on: ubuntu-latest entries) to include a permissions: contents: read stanza
so the workflow only has read access to repository contents.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
47-47: Add--lockedto keep cross-builds reproducibleLine 47 currently allows dependency resolution drift if
Cargo.lockand index state diverge. For release-oriented targets, prefer locked resolution.Proposed change
- cross build --bin morph-reth --target $* --profile "$(PROFILE)" + cross build --locked --bin morph-reth --target $* --profile "$(PROFILE)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 47, Update the cross build invocation to use Cargo's locked resolution by adding the --locked flag to the existing command that invokes cross: replace the current invocation "cross build --bin morph-reth --target $* --profile \"$(PROFILE)\"" with a version that includes --locked so builds use Cargo.lock and avoid dependency resolution drift for release targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 45-50: The release workflow can bypass Makefile safeguards because
RUSTFLAGS (used in the build-% recipe) and JEMALLOC_SYS_WITH_LG_PAGE (set for
build-aarch64-unknown-linux-gnu) only take effect when invoking make
build-<target>; update the CI workflow to either (A) call make build-${{
matrix.target }} so the Makefile targets and their env exports are honored
(referencing the build-% and build-aarch64-unknown-linux-gnu targets), or (B) if
you must keep invoking cross build directly, inject the same environment
variables into the workflow step (set RUSTFLAGS with "-C link-arg=-lgcc
-Clink-arg=-static-libgcc" and set JEMALLOC_SYS_WITH_LG_PAGE=16 for aarch64) so
the aarch64 jemalloc page-size and RUSTFLAGS are preserved.
---
Nitpick comments:
In `@Makefile`:
- Line 47: Update the cross build invocation to use Cargo's locked resolution by
adding the --locked flag to the existing command that invokes cross: replace the
current invocation "cross build --bin morph-reth --target $* --profile
\"$(PROFILE)\"" with a version that includes --locked so builds use Cargo.lock
and avoid dependency resolution drift for release targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…ependent of Makefile
Replace slow QEMU-based multi-arch Docker build (~200min) with cross-compiled binaries + Dockerfile.cross (~15min), matching reth/scroll-reth approach.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Makefile (1)
46-47: Preserve caller-suppliedRUSTFLAGSin cross builds.Current assignment overwrites any upstream
RUSTFLAGS. Appending keeps this target flexible for CI/local overrides. (Line 46 and Line 47)Suggested patch
- RUSTFLAGS="-C link-arg=-lgcc -Clink-arg=-static-libgcc" \ + RUSTFLAGS="$$RUSTFLAGS -C link-arg=-lgcc -Clink-arg=-static-libgcc" \ cross build --locked --bin morph-reth --target $* --profile "$(PROFILE)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 46 - 47, The Makefile currently overwrites any caller-supplied RUSTFLAGS before invoking cross build for the morph-reth target; change the RUSTFLAGS assignment so it preserves existing RUSTFLAGS (i.e., append the flags -C link-arg=-lgcc and -Clink-arg=-static-libgcc to the existing RUSTFLAGS) and then invoke cross build --locked --bin morph-reth --target $* --profile "$(PROFILE)"; update the line that sets RUSTFLAGS and the subsequent cross build invocation to use the preserved/appended RUSTFLAGS rather than replacing them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile.cross`:
- Around line 3-14: Add a non-root user and switch to it before ENTRYPOINT:
create a dedicated user and group (e.g., "morph" or similar) and chown the
installed binary path (/usr/local/bin/morph-reth) to that user, then switch to
that user via USER before the ENTRYPOINT. Update the Dockerfile around the COPY
and ENTRYPOINT steps that reference ARG TARGETARCH and COPY
./dist/bin/$TARGETARCH/morph-reth so ownership and permissions are set for the
new user and the container runs as that non-root account instead of root.
In `@Makefile`:
- Line 41: Update every occurrence of the unpinned installer invocation "cargo
install cross --git https://github.com/cross-rs/cross" in the CI config to
include a fixed --tag or --rev (e.g., --tag <version> or --rev <commit>) so the
cross tool is pinned for reproducible builds, and mirror that change in the
Makefile comment line that documents the Cross Build instruction so the doc
string reflects the pinned installation. Ensure both CI invocations use the same
pinned value and that the Makefile text mentions the specific tag/revision.
---
Nitpick comments:
In `@Makefile`:
- Around line 46-47: The Makefile currently overwrites any caller-supplied
RUSTFLAGS before invoking cross build for the morph-reth target; change the
RUSTFLAGS assignment so it preserves existing RUSTFLAGS (i.e., append the flags
-C link-arg=-lgcc and -Clink-arg=-static-libgcc to the existing RUSTFLAGS) and
then invoke cross build --locked --bin morph-reth --target $* --profile
"$(PROFILE)"; update the line that sets RUSTFLAGS and the subsequent cross build
invocation to use the preserved/appended RUSTFLAGS rather than replacing them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f2773fbd-a40e-4a7b-a722-c462f4182251
📒 Files selected for processing (4)
.github/workflows/docker.yml.github/workflows/release.ymlDockerfile.crossMakefile
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/release.yml
- .github/workflows/docker.yml
3 participants
Summary
taiki-e/install-action(cross 0.2.5, Ubuntu 16.04) tocargo install cross --git(cross main, Ubuntu 20.04)Cross.toml— cross main images already ship gcc + libclang, no pre-build installs neededtest-cross-build.yml(to be deleted after merge)Root Cause
cross 0.2.5 uses a Ubuntu 16.04 (Xenial) base image. The pre-build
apt-getcommands inCross.tomltried to switch sources to HTTPS, but Xenial lacksapt-transport-httpsby default, causingapt-get updateto fail. reth and scroll-reth both usecargo install cross --git(main branch) which ships Ubuntu 20.04 images with all required toolchain already installed.Test plan
Summary by CodeRabbit