chore(op-program): migrate Makefile to justfile#19476
Conversation
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
7f06f05 to
f29f080
Compare
707214d to
bc8a396
Compare
f29f080 to
ac12acf
Compare
bc8a396 to
ee44bd6
Compare
ac12acf to
e97849d
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## aj/migrate-op-e2e-makefile-to-just #19476 +/- ##
===================================================================
Coverage 75.8% 75.8%
===================================================================
Files 477 477
Lines 59985 59985
===================================================================
Hits 45510 45510
Misses 14475 14475
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
1e19f45 to
cf8a75d
Compare
ee44bd6 to
38cf3bd
Compare
cf8a75d to
ccc870a
Compare
50c0f17 to
b9a8200
Compare
ccc870a to
ef6564d
Compare
b9a8200 to
8a674d6
Compare
ef6564d to
945b5e3
Compare
sebastianst
left a comment
There was a problem hiding this comment.
Review: op-program Makefile → justfile migration
Overall this is a solid, faithful migration. All 33 targets are preserved with correct behavior. A few items worth flagging:
1. PR description vs Dockerfile.vmcompat
The description says "Updates Dockerfile.vmcompat to use just instead of make for the analyze target", but line 34 still calls make "analyze-op-program-client-${VM_TARGET}". The only actual change is adding just to apk add so the deprecated Make shim can delegate to it. Not a code issue, just a misleading description.
2. VERSION is no longer env-overridable
Original Makefile: VERSION ?= v0.0.0 — accepts env/cmdline override, defaults to v0.0.0.
New git.just: computes VERSION from git tags, falling back to "untagged". No env('VERSION', ...) wrapper.
This means:
- Default changed from
v0.0.0to tag-computed /"untagged"for the host binary ldflags make op-program-host VERSION=x.y.zthrough the deprecated shim won't forward — the env var is ignored bygit.just- Client binaries are unaffected (
_CLIENT_LDFLAGShardcodesv0.0.0) - The repro pipeline only builds client binaries, so it's fine there
Worth checking if any CI job passes VERSION=... to op-program host builds.
3. TODO comment dropped from run-vm-compat
The original Makefile had # TODO(#18334): Uncomment once vm-compat supports go1.25 with commented-out docker build lines for the next target. The justfile drops this entirely, losing the issue reference.
Things that look good
repro.justfilecorrectly removes the ineffectiveGOOS/GOARCH/GOMIPSoverrides (the mips64 target hardcodes these in env, so Make variable overrides were never used)- Dockerignore files correctly add
!justfiles/ output-prestate-hashswitches fromecho "\n..."toprintf "\n..."— more portable- Host binary correctly uses
go_buildwithCGO_ENABLED=0; client binaries correctly omit it _CLIENT_LDFLAGSfaithfully reproducesPC_LDFLAGSSTRING(hardcodedv0.0.0, empty Meta)- Capture targets correctly use
$VARinstead of Make's$$VAR - Deprecated shim list is more comprehensive than the original
.PHONYdeclarations
Review generated by Claude Code
0ad8729 to
c01848c
Compare
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>
Migrate op-program 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>
The Makefile shim includes ../justfiles/deprecated.mk which delegates to just. Docker builds for vmcompat and repro excluded justfiles/ from the build context, causing "No such file or directory" errors in CI. - Add !justfiles/ to both .dockerignore files - Install just in Dockerfile.vmcompat (Dockerfile.repro already has it) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…o.justfile The op-program-client-mips target hardcodes GOOS/GOARCH/GOMIPS64 in its build commands, so passing GOOS=linux GOARCH=mips GOMIPS=softfloat from repro.justfile was always redundant. With the Make-to-just migration, the deprecated.mk shim forwards these as just variables, and just rejects GOMIPS since it's not defined in the justfile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Update Dockerfile.vmcompat to call just directly for the analyze target instead of going through the deprecated make shim. - Restore TODO(#18334) comment for go1.25 vm-compat support that was dropped during migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert repro.justfile to call just directly instead of going through make's deprecated shim. GOOS/GOARCH are passed as env vars for cannon (read by go.just), while VERSION/GITCOMMIT/GITDATE are passed as just variable overrides. The op-program mips targets hardcode their own GOOS/GOARCH/GOMIPS so they don't need to be passed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c01848c to
c2b4af4
Compare
|
Thanks for the thorough review! All three items have been addressed:
|
Summary
justwith a deprecation warningreproducible-prestatetarget is kept as an explicit Make target (not in the deprecated shim) since it's called directly by external toolingDockerfile.vmcompatto calljustdirectly for the analyze target instead of going through the deprecated Make shimStack
Test plan
just --listshows all 33 targetsjust --dry-run op-program-hostconfirms CGO_ENABLED=0 and dynamic VERSION ldflagsjust --dry-run op-program-clientconfirms hardcoded v0.0.0 ldflagsjust --dry-run op-program-client-mips64confirms GOOS=linux GOARCH=mips64 GOMIPS64=softfloatmake op-program-hostdelegates to just with deprecation warning🤖 Generated with Claude Code