-
Notifications
You must be signed in to change notification settings - Fork 132
fix(l1,l2): make ethrex L1 + L2 + GPU build compile for ubuntu 22.04 #5276
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
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.
Pull Request Overview
This PR fixes a build compilation issue for ethrex L1 + L2 + GPU builds on Ubuntu 22.04 that was introduced by a previous change to the c-kzg crate configuration. The fix ensures continuous validation of key builds by extending the release workflow.
- Reverted
c-kzgdependency to use default features to restore RISC0 compatibility - Extended GitHub Actions release workflow to trigger on pushes to
mainbranch for continuous build validation - Removed hardcoded
RUSTFLAGSoverride to use the default configuration from.cargo/config.toml
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/common/crypto/Cargo.toml | Restores default features for c-kzg dependency to fix Ubuntu 22.04 build compatibility |
| .github/workflows/tag_release.yaml | Adds main branch trigger for continuous validation, removes RUSTFLAGS override, adds conditional logic for Docker tagging and release creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Format name | ||
| run: | | ||
| echo "TAG_VERSION=$(echo ${{ github.ref_name }} | tr -d v)" >> $GITHUB_ENV | ||
| # For branch builds (main) we want docker images tagged as 'main'. For tag pushes |
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.
There's already a CI that builds the docker image for main commits. We should remove either that or this
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.
Does this change replace entirely the tag_latest workflow?
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.
| echo "TAG_VERSION=$(echo ${{ github.ref_name }} | tr -d v)" >> $GITHUB_ENV | ||
| # For branch builds (main) we want docker images tagged as 'main'. For tag pushes | ||
| # use the tag name (stripped of a leading 'v'). | ||
| if [[ "$GITHUB_REF" == "refs/heads/main" ]]; 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.
Suggestion from copilot:
| if [[ "$GITHUB_REF" == "refs/heads/main" ]]; then | |
| if [[ ${{ github.ref_type }} == "branch" ]]; then |
.github/workflows/tag_release.yaml
Outdated
| branches: | ||
| - main | ||
| # FIXME: This is to trigger the job in the PR's CI | ||
| - fix_v7.0.0-rc.1 |
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.
We could add a workflow_dispatch trigger, to make this run without adding patches like this
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.
Yes, good idea. We have something similar in ethrex-replay.
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.
We can do that in another PR.
Motivation
One of the changes in a recent PR disabled the default features of the
c-kzgcrate and manually enabled a subset of them. The goal was to ensure the library imports with only 2 out of its 3 default features enabled. Unfortunately, this broke the ethrex L2 + GPU build for Ubuntu 22.04 (specifically due to a RISC0-related compilation error).This issue went undetected because ethrex builds are only tested during release or release candidate preparation, not on every commit.
Description
To avoid similar regressions in the future, this PR adapts the existing GitHub Actions workflow for releases and pre-releases. The workflow will now also trigger automatically after every push to the main branch, ensuring continuous validation of key builds.
While debugging, I noticed that releases for ethrex L1 and L1 + L2 + GPU on Ubuntu 22.04 are being built with the
RUSTFLAGS='-C target-cpu=x86-64-v2'flag, which overrides the settings in.cargo/config.toml. This PR removes this override to align with our configured defaults.c-kzgfeatures change: Re-enable all default features to restore compatibility without negative impacts.RUSTFLAGSoverride: eliminate the hardcoded flag from the release configuration.