Skip to content

chore(cannon): migrate testdata Makefiles to justfiles#19483

Merged
ajsutton merged 14 commits intodevelopfrom
aj/migrate-cannon-testdata-to-just
Mar 13, 2026
Merged

chore(cannon): migrate testdata Makefiles to justfiles#19483
ajsutton merged 14 commits intodevelopfrom
aj/migrate-cannon-testdata-to-just

Conversation

@ajsutton
Copy link
Contributor

@ajsutton ajsutton commented Mar 11, 2026

Summary

  • Migrates cannon/testdata/, cannon/testdata/go-1-24/, and cannon/testdata/go-1-25/ Makefiles to justfiles
  • The Make pattern rules (bin/%.64.elf) for building ELF binaries from go.mod directories are replaced with shell loops that discover and build all directories dynamically
  • Updates cannon/justfile to call just instead of make -C for testdata targets
  • Updates CircleCI to use just elf instead of make elf

Stack

  1. optimism#19473 — linter migration + shared infrastructure
  2. optimism#19474 — cannon migration
  3. optimism#19475 — op-e2e migration
  4. optimism#19476 — op-program migration
  5. optimism#19477 — root Makefile migration
  6. optimism#19482 — CI config update
  7. This PR — cannon testdata migration

Test plan

  • just elf in cannon/testdata/ builds all ELF binaries
  • just elf in cannon/testdata/go-1-24/ builds go-1-24 ELF binaries
  • CI cannon tests pass

🤖 Generated with Claude Code

@ajsutton ajsutton requested a review from a team as a code owner March 11, 2026 03:54
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 981017d to 9f85a7c Compare March 11, 2026 03:58
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch 2 times, most recently from 5c7a2c5 to e0ed02c Compare March 11, 2026 04:11
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch 2 times, most recently from 6f81e0e to 5e144e5 Compare March 11, 2026 04:16
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from e0ed02c to 1261c27 Compare March 11, 2026 04:16
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 5e144e5 to 57be692 Compare March 11, 2026 04:21
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 1261c27 to c3b9c61 Compare March 11, 2026 04:21
@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.7%. Comparing base (9a50c60) to head (8e9e604).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #19483      +/-   ##
===========================================
+ Coverage     75.3%    75.7%    +0.4%     
===========================================
  Files          194      671     +477     
  Lines        11285    71270   +59985     
===========================================
+ Hits          8502    54017   +45515     
- Misses        2639    17109   +14470     
  Partials       144      144              
Flag Coverage Δ
cannon-go-tests-64 66.4% <ø> (ø)
contracts-bedrock-tests 80.2% <ø> (ø)
unit 75.8% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 477 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from c3b9c61 to e4c158a Compare March 11, 2026 15:37
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 57be692 to cb2c2f7 Compare March 11, 2026 15:37
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from e4c158a to a5226fc Compare March 11, 2026 16:20
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from cb2c2f7 to 4781beb Compare March 11, 2026 16:20
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from a5226fc to 7edf4ca Compare March 12, 2026 16:12
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 4781beb to e349dce Compare March 12, 2026 16:12
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 7edf4ca to 84a660a Compare March 12, 2026 18:25
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from e349dce to 096e0c4 Compare March 12, 2026 18:25
Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: cannon testdata Makefiles → justfiles

Small, clean migration. The Make pattern rules are replaced with shell loops that discover and build ELFs dynamically. A few notes:

1. Lost parallel builds

The original Make pattern rules (bin/%.64.elf) could build all ELF binaries in parallel with make -j. The just version uses a sequential for mod in */go.mod loop. For testdata binaries this probably doesn't matter much, but worth noting.

2. dump target changed discovery mechanism

Original: $(patsubst %/go.mod,bin/%.dump,$(wildcard */go.mod)) — dumps only ELFs that correspond to */go.mod directories.
New: for elf in bin/*.64.elf — dumps any .64.elf in bin/, regardless of whether a matching go.mod exists.

Unlikely to matter in practice (stale ELFs in bin/ would be an unusual scenario), but it's a subtle behavioral difference.

3. dump lost a TODO comment

Original had # TODO: currently have the little-endian toolchain, but should use the big-endian one. The -EB compat flag works though. — dropped from the justfile.

Everything else looks good

  • elf64: elf reverses the dependency direction from the original (elf: elf64) but produces the same result — elf does the work, elf64 is an alias
  • clean correctly adds || true to handle missing bin/ directory (needed since just fails on non-zero exit, unlike Make with @)
  • cannon/justfile correctly updated from make -C to just ./testdata/elf syntax
  • CircleCI change is straightforward make elfjust elf
  • Deprecated shim order puts elf first, so bare make (no target) still defaults to building ELFs

Review generated by Claude Code

@sebastianst
Copy link
Member

Follow-up: recovering lost parallelism

A few patterns to recover the make -j parallelism where it matters:

ELF build loops — replace the sequential for with parallel:

mkdir -p bin
parallel --tag 'cd {} && GOOS=linux GOARCH=mips64 GOMIPS64=softfloat go build -o "../bin/{}.64.elf" .' ::: $(ls -d */go.mod | xargs -n1 dirname)

Recipe-level deps (e.g. reproducible-prestate) — just always runs deps sequentially, but you can parallelize in the recipe body with & + wait:

[script('bash')]
reproducible-prestate:
    set -euo pipefail
    make -C ./op-program build-reproducible-prestate &
    (cd rust && just build-kona-reproducible-prestate) &
    wait
    make -C ./op-program output-prestate-hash
    cd rust && just output-kona-prestate-hash

There's already a MAP_JUST variable in justfiles/default.just that wires up GNU parallel for just targets — it's just not used anywhere yet.

That said, parallel is a heavier dependency than make -j. For testdata ELFs it's probably not worth it. The reproducible-prestate case (two Docker builds) is where it matters most, and & + wait is simple enough.


Comment generated by Claude Code

@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 84a660a to c0b0e5e Compare March 12, 2026 22:16
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 1095540 to d403c42 Compare March 12, 2026 22:16
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from c0b0e5e to 8f998b1 Compare March 12, 2026 22:29
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from d403c42 to 9f10990 Compare March 12, 2026 22:29
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 8f998b1 to f2f76e2 Compare March 12, 2026 22:50
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 9f10990 to 0961fba Compare March 12, 2026 22:50
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from f2f76e2 to 46b6245 Compare March 12, 2026 22:55
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 0961fba to e59b0b4 Compare March 12, 2026 22:55
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 46b6245 to 3f01ec5 Compare March 12, 2026 23:27
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from e59b0b4 to 879f06d Compare March 12, 2026 23:27
ajsutton and others added 9 commits March 13, 2026 00:08
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d shim

The deprecated.mk shim was changed to pass JUSTFLAGS as just CLI
variable overrides (`just VAR=val target`), but just rejects overrides
for variables not declared in the justfile. This broke CI jobs where
Make variable assignments propagate through sub-makes (e.g.
GO_TEST_FLAGS, GUEST_PROGRAM).

Revert to passing them as environment variables via `env`, which is
how the shim originally worked in the cannon migration PR.

Fixes: go-tests-short, sanitize-op-program CI failures

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrate root build targets from Make to Just. The Makefile now
delegates to just with a deprecation warning, preserving backwards
compatibility for existing make invocations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
All subdirectories now have justfiles with deprecated Make shims,
so convert remaining make -C calls to cd && just. Also restores
the explanatory comment on mod-tidy's GOPRIVATE usage.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
just doesn't parallelize dependencies like make -j does. Use background
processes with wait to run op-program and kona prestate builds concurrently.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The cd into cannon/ was changing the CWD for the rest of the script,
causing gotestsum to run from cannon/ instead of the repo root. The
original Makefile used $(MAKE) -C which spawns a subprocess. Use a
subshell to match that behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The bare `cd op-program` on line 180 changed cwd persistently, so the
following `cd rust` tried to resolve `op-program/rust/` which doesn't
exist. Wrap both in subshells to preserve the original working directory.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 3f01ec5 to 7f50e24 Compare March 13, 2026 00:09
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from f386241 to 207928f Compare March 13, 2026 00:10
ajsutton and others added 2 commits March 13, 2026 00:32
These directories were removed in #19506 but the justfile still
referenced them, causing go-tests-short-ci to fail with
"lstat ./devnet-sdk/: no such file or directory".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Migrates all CircleCI make invocations to just for targets that have
been migrated to justfiles. Remaining make calls are for packages not
yet migrated (op-challenger, op-node, op-service, op-chain-ops fuzz
targets) and cannon/testdata which has its own Makefile.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ajsutton ajsutton force-pushed the aj/ci-make-to-just branch from 7f50e24 to dc80630 Compare March 13, 2026 00:32
ajsutton and others added 3 commits March 13, 2026 00:32
Migrates cannon/testdata/, cannon/testdata/go-1-24/, and
cannon/testdata/go-1-25/ Makefiles to justfiles. The Make pattern rules
for building ELF binaries from go.mod directories are replaced with
shell loops that discover and build all directories dynamically.

Also updates cannon/justfile to call just instead of make -C for
testdata targets, and updates the CI config accordingly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The generic deprecated.mk shim converts make variables to env vars, but
diff-cannon VM: is a positional parameter in just. Write a manual shim
that passes VM correctly. Also add deprecation warning to the diff-%-cannon
pattern target.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Match the original Makefile behavior by iterating */go.mod directories
instead of bin/*.64.elf to avoid dumping stale ELFs. Restore the TODO
about the little-endian vs big-endian toolchain.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ajsutton ajsutton force-pushed the aj/migrate-cannon-testdata-to-just branch from 207928f to 8e9e604 Compare March 13, 2026 00:32
Base automatically changed from aj/ci-make-to-just to develop March 13, 2026 01:55
@ajsutton ajsutton added this pull request to the merge queue Mar 13, 2026
Merged via the queue into develop with commit ccb6aa1 Mar 13, 2026
127 checks passed
@ajsutton ajsutton deleted the aj/migrate-cannon-testdata-to-just branch March 13, 2026 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants