chore: migrate from Makefile to justfile#143
Conversation
Replace Makefile with justfile for local development. CI workflows inline cargo/cross/docker commands directly to avoid adding just as a CI dependency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR removes the Makefile, adds a comprehensive Justfile for local and CI automation, updates docs and examples to use Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Pipeline
participant Just as Justfile Runner
participant Cross as cross (cross-compiler)
participant Dist as Dist Output
participant Buildx as Docker Buildx
participant Registry as Image Registry
CI->>Just: invoke docker-build-push
Just->>Cross: build target x86_64-unknown-linux-gnu (FEATURES/PROFILE)
Cross-->>Dist: write dist/bin/linux/amd64/<binary>
Just->>Cross: build target aarch64-unknown-linux-gnu (FEATURES/PROFILE)
Cross-->>Dist: write dist/bin/linux/arm64/<binary>
Just->>Buildx: docker buildx build --platform linux/amd64,linux/arm64 --file Dockerfile.cross --push
Buildx->>Registry: push multi-arch image (amd64 + arm64)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
Claude finished @tac0turtle's task —— View job Pull Request ReviewI've completed my review of this Makefile to justfile migration. Overall, this is a well-executed modernization that improves the development experience while maintaining functionality. Todo List:
Review Summary✅ StrengthsJustfile Implementation
CI/CD Improvements
Documentation Updates
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
justfile (1)
135-136: Remove duplicate image tag argument indocker-build-push.The same tag is provided twice, which is redundant.
♻️ Proposed cleanup
docker buildx build --file ./Dockerfile.cross . \ --platform linux/amd64,linux/arm64 \ --tag {{docker_image}}:{{docker_tag}} \ - --tag {{docker_image}}:{{docker_tag}} \ --provenance=false \ --sbom=false \ --push🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 135 - 136, In the docker-build-push target remove the duplicated --tag argument: the lines that pass --tag {{docker_image}}:{{docker_tag}} twice should only include a single --tag; update the docker-build-push recipe so it only adds one --tag using the existing variables (docker_image and docker_tag) and remove the redundant duplicate --tag entry.
🤖 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/release.yml:
- Line 44: The cargo build step currently runs RUSTFLAGS="-C target-cpu=native"
cargo build --profile maxperf --features jemalloc,asm-keccak --bin ev-reth which
omits the cross-compile target and uses the wrong profile; change that
invocation to include --target ${{ matrix.target }} and use --profile release
(or match whatever profile the packaging expects) so artifacts go to the matrix
target directory and the release profile path, and then update the subsequent
packaging steps that create the tarball/zip to point to the correct artifact
paths produced by the modified cargo build.
---
Nitpick comments:
In `@justfile`:
- Around line 135-136: In the docker-build-push target remove the duplicated
--tag argument: the lines that pass --tag {{docker_image}}:{{docker_tag}} twice
should only include a single --tag; update the docker-build-push recipe so it
only adds one --tag using the existing variables (docker_image and docker_tag)
and remove the redundant duplicate --tag entry.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.claude/skills/ev-reth-core.md.claude/skills/ev-reth-evolve.md.claude/skills/ev-reth-testing.md.github/workflows/docker.yml.github/workflows/release.ymlCLAUDE.mdMakefileREADME.mdjustfile
💤 Files with no reviewable changes (1)
- Makefile
440019e to
086eda8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
justfile (1)
128-152: Extract duplicated Docker staging/buildx steps.Lines 128-152 duplicate the same copy/buildx flow in two recipes. A shared private recipe would reduce drift and maintenance overhead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 128 - 152, The two justfile recipes docker-build-push and docker-build-push-latest duplicate the same staging/copy and docker buildx invocation; extract the duplicated block (mkdir/cp for {{bin_dir}}/linux/amd64 and /arm64 and the docker buildx build ... --platform ... --provenance/--sbom/--push invocation) into a single private recipe (e.g., _docker-buildx-staging) that accepts or references the same variables ({{bin_dir}}, {{target_dir}}, {{profile}}, {{binary}}, {{docker_image}}, tag), then call that private recipe from both docker-build-push and docker-build-push-latest (passing the appropriate tag variables like {{docker_tag}} or {{git_tag}}/latest) so both top-level recipes delegate to the shared implementation and eliminate the duplication.
🤖 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/docker.yml:
- Around line 84-89: The workflow currently always runs docker buildx build with
the --push flag which causes permission failures for pull_request events where
GITHUB_TOKEN is read-only; change the step that runs the docker buildx build
command to conditionally include/publish the image only for non-pull_request
events (e.g. add an if: github.event_name != 'pull_request' on the step or gate
the --push by an input/env like PUSH=true and only append --push when PUSH is
true), keep the build command otherwise without --push (or use --load for local
testing), and ensure any references to GITHUB_TOKEN or packages: write remain
unchanged but are only relied upon when pushing is allowed.
In @.github/workflows/release.yml:
- Line 44: Replace the inline bash-style env assignment and host-specific CPU
tuning by moving RUSTFLAGS into the job/step env and set CPU tuning to a
portable value: set env: RUSTFLAGS: "-C target-cpu=generic" and leave the run
command as cargo build --target ${{ matrix.target }} --profile release
--features jemalloc,asm-keccak --bin ev-reth (i.e., update the step that
currently contains the run string with RUSTFLAGS="..." to instead declare
RUSTFLAGS in the step's env and change target-cpu=native to target-cpu=generic).
In `@justfile`:
- Line 11: The docker_image default parsing (variable docker_image in justfile)
fails for remote URLs without a .git suffix because the sed regex requires
".git" and extracts "https:" for URLs like https://github.com/org/repo; update
the sed expression used in the docker_image assignment to robustly extract the
owner segment from both SSH and HTTPS remotes and to accept an optional ".git"
suffix (so it matches patterns like git@github.com:org/repo,
https://github.com/org/repo, and https://github.com/org/repo.git) and return
only the owner/org portion instead of protocol text.
---
Nitpick comments:
In `@justfile`:
- Around line 128-152: The two justfile recipes docker-build-push and
docker-build-push-latest duplicate the same staging/copy and docker buildx
invocation; extract the duplicated block (mkdir/cp for {{bin_dir}}/linux/amd64
and /arm64 and the docker buildx build ... --platform ...
--provenance/--sbom/--push invocation) into a single private recipe (e.g.,
_docker-buildx-staging) that accepts or references the same variables
({{bin_dir}}, {{target_dir}}, {{profile}}, {{binary}}, {{docker_image}}, tag),
then call that private recipe from both docker-build-push and
docker-build-push-latest (passing the appropriate tag variables like
{{docker_tag}} or {{git_tag}}/latest) so both top-level recipes delegate to the
shared implementation and eliminate the duplication.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.claude/skills/ev-reth-core.md.claude/skills/ev-reth-evolve.md.claude/skills/ev-reth-testing.md.github/workflows/docker.yml.github/workflows/release.ymlCLAUDE.mdMakefileREADME.mdjustfile
💤 Files with no reviewable changes (1)
- Makefile
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/skills/ev-reth-core.md
- .claude/skills/ev-reth-testing.md
| docker buildx build --file ./Dockerfile.cross . \ | ||
| --platform linux/amd64,linux/arm64 \ | ||
| --tag "$DOCKER_IMAGE_NAME:$DOCKER_TAG" \ | ||
| --provenance=false \ | ||
| --sbom=false \ | ||
| --push |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/docker.ymlRepository: evstack/ev-reth
Length of output: 4074
Avoid unconditional image pushes during PR runs.
Lines 12-13 allow pull_request triggers, but lines 84-89 always use --push. For fork PRs, the GITHUB_TOKEN is read-only despite the explicit packages: write permission request (line 27), causing the push to fail with permission errors.
🔧 Suggested adjustment
docker buildx build --file ./Dockerfile.cross . \
--platform linux/amd64,linux/arm64 \
+ --push
- --pushOr conditionally load/push based on event type:
+ if [ "${{ github.event_name }}" = "pull_request" ]; then
+ docker buildx build --file ./Dockerfile.cross . \
+ --platform linux/amd64 \
+ --tag "$DOCKER_IMAGE_NAME:$DOCKER_TAG" \
+ --provenance=false \
+ --sbom=false \
+ --load
+ else
docker buildx build --file ./Dockerfile.cross . \
--platform linux/amd64,linux/arm64 \
--tag "$DOCKER_IMAGE_NAME:$DOCKER_TAG" \
--provenance=false \
--sbom=false \
--push
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker.yml around lines 84 - 89, The workflow currently
always runs docker buildx build with the --push flag which causes permission
failures for pull_request events where GITHUB_TOKEN is read-only; change the
step that runs the docker buildx build command to conditionally include/publish
the image only for non-pull_request events (e.g. add an if: github.event_name !=
'pull_request' on the step or gate the --push by an input/env like PUSH=true and
only append --push when PUSH is true), keep the build command otherwise without
--push (or use --load for local testing), and ensure any references to
GITHUB_TOKEN or packages: write remain unchanged but are only relied upon when
pushing is allowed.
|
|
||
| - name: Build release binary | ||
| run: make build-maxperf | ||
| run: RUSTFLAGS="-C target-cpu=native" cargo build --target ${{ matrix.target }} --profile release --features jemalloc,asm-keccak --bin ev-reth |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=".github/workflows/release.yml"
echo "=== [1] Checking file exists and reading context around line 44 ==="
if [ -f "$f" ]; then
echo "File found. Reading lines 20-50 for context:"
sed -n '20,50p' "$f" | cat -n
else
echo "File not found at $f"
fi
echo
echo "=== [2] Checking matrix configuration for OS targets ==="
sed -n '1,60p' "$f" | grep -A 20 'matrix:' | head -25Repository: evstack/ev-reth
Length of output: 2188
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== [1] Checking for explicit shell configuration in release workflow ==="
grep -n "shell:" .github/workflows/release.yml || echo "No explicit 'shell:' key found"
echo
echo "=== [2] Checking full 'Build release binary' step ==="
sed -n '24,26p' .github/workflows/release.yml | cat -n
echo
echo "=== [3] Checking if any global defaults set shell ==="
head -40 .github/workflows/release.yml | grep -A2 -B2 "defaults:" || echo "No defaults section found at top"Repository: evstack/ev-reth
Length of output: 422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking context around line 55 to understand shell scope ==="
sed -n '40,65p' .github/workflows/release.yml | cat -n
echo
echo "=== Checking GitHub Actions default shell for windows-latest ==="
echo "GitHub Actions default shell behavior:"
echo "- ubuntu/macos runners: bash"
echo "- windows-latest runner: PowerShell"
echo "- Inline VAR=value syntax is bash-specific and will fail on PowerShell"Repository: evstack/ev-reth
Length of output: 1512
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking if 'Build release binary' step has any implicit shell override ==="
sed -n '1,30p' .github/workflows/release.yml | cat -n
echo
echo "=== Verification of the issue ==="
echo "1. Build release binary step (line 24-25): NO explicit 'shell:' specification"
echo "2. matrix.os includes 'windows-latest' which defaults to PowerShell"
echo "3. Create zip step (line 55-56): HAS 'shell: pwsh' but ONLY applies to that step"
echo "4. Bash syntax 'VAR=value command' is INCOMPATIBLE with PowerShell default"
echo
echo "Result: The build step WILL FAIL on windows-latest due to shell syntax incompatibility"Repository: evstack/ev-reth
Length of output: 1432
Fix cross-platform build portability on line 44.
The build command uses bash-style inline env assignment (RUSTFLAGS="..." cargo build), which will fail on windows-latest runners that default to PowerShell. Additionally, target-cpu=native bakes host-specific CPU tuning into release artifacts, making them non-portable across machines with different CPUs.
Proposed fix
- - name: Build release binary
- run: RUSTFLAGS="-C target-cpu=native" cargo build --target ${{ matrix.target }} --profile release --features jemalloc,asm-keccak --bin ev-reth
+ - name: Build release binary
+ run: cargo build --target ${{ matrix.target }} --release --features jemalloc,asm-keccak --bin ev-reth🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 44, Replace the inline bash-style env
assignment and host-specific CPU tuning by moving RUSTFLAGS into the job/step
env and set CPU tuning to a portable value: set env: RUSTFLAGS: "-C
target-cpu=generic" and leave the run command as cargo build --target ${{
matrix.target }} --profile release --features jemalloc,asm-keccak --bin ev-reth
(i.e., update the step that currently contains the run string with
RUSTFLAGS="..." to instead declare RUSTFLAGS in the step's env and change
target-cpu=native to target-cpu=generic).
| git_commit := `git rev-parse --short HEAD 2>/dev/null || echo "unknown"` | ||
| docker_tag := env("DOCKER_TAG", git_commit) | ||
| bin_dir := "dist/bin" | ||
| docker_image := env("DOCKER_IMAGE_NAME", `echo "ghcr.io/$(git config --get remote.origin.url | sed 's/.*github.com[:/]\(.*\)\.git/\1/' | cut -d'/' -f1)/ev-reth"`) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "justfile|Justfile" | head -20Repository: evstack/ev-reth
Length of output: 67
🏁 Script executed:
cat -n justfile | head -15Repository: evstack/ev-reth
Length of output: 756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pattern='s/.*github.com[:/]\(.*\)\.git/\1/'
for url in \
'git@github.com:evstack/ev-reth.git' \
'https://github.com/evstack/ev-reth.git' \
'https://github.com/evstack/ev-reth'
do
owner=$(echo "$url" | sed "$pattern" | cut -d'/' -f1)
printf '%s -> %s\n' "$url" "$owner"
doneRepository: evstack/ev-reth
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Test the proposed fix logic
for url in \
'git@github.com:evstack/ev-reth.git' \
'https://github.com/evstack/ev-reth.git' \
'https://github.com/evstack/ev-reth'
do
owner="$(printf "%s" "$url" | sed -E 's#^(git@github.com:|https://github.com/)##; s#\.git$##' | cut -d'/' -f1)"
[ -n "$owner" ] && result="ghcr.io/$owner/ev-reth" || result="ghcr.io/evstack/ev-reth"
printf '%s -> %s\n' "$url" "$result"
doneRepository: evstack/ev-reth
Length of output: 248
Harden default Docker image owner parsing on Line 11.
The current sed expression requires a .git suffix and fails for remotes like https://github.com/org/repo (without .git), returning https: instead of the actual owner. This causes invalid Docker image names when .git is absent from the remote URL.
🛠️ Proposed fix
-docker_image := env("DOCKER_IMAGE_NAME", `echo "ghcr.io/$(git config --get remote.origin.url | sed 's/.*github.com[:/]\(.*\)\.git/\1/' | cut -d'/' -f1)/ev-reth"`)
+docker_image := env("DOCKER_IMAGE_NAME", `url="$(git remote get-url origin 2>/dev/null || true)"; owner="$(printf "%s" "$url" | sed -E 's#^(git@github.com:|https://github.com/)##; s#\.git$##' | cut -d'/' -f1)"; [ -n "$owner" ] && echo "ghcr.io/$owner/ev-reth" || echo "ghcr.io/evstack/ev-reth"`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@justfile` at line 11, The docker_image default parsing (variable docker_image
in justfile) fails for remote URLs without a .git suffix because the sed regex
requires ".git" and extracts "https:" for URLs like https://github.com/org/repo;
update the sed expression used in the docker_image assignment to robustly
extract the owner segment from both SSH and HTTPS remotes and to accept an
optional ".git" suffix (so it matches patterns like git@github.com:org/repo,
https://github.com/org/repo, and https://github.com/org/repo.git) and return
only the owner/org portion instead of protocol text.
Makefile was deleted on main (replaced by justfile in #143). Added ev-dev recipes (build-ev-dev, dev-chain) to justfile instead.
Replace Makefile with justfile for local development. CI workflows inline cargo/cross/docker commands directly to avoid adding just as a CI dependency.
Description
Type of Change
Related Issues
Fixes #(issue)
Checklist
Testing
Additional Notes
Summary by CodeRabbit
Documentation
Chores