Conversation
|
| Branch | Total Count |
|---|---|
| main | 2784 |
| This PR | 3539 |
| Difference | +755 (27.12%) |
📁 Changes by file type:
| File Type | Change |
|---|---|
| Go files (.go) | ❌ +328 |
| Documentation (.md) | ❌ +354 |
| Earthfiles | ❌ +2 |
Keep up the great work migrating from Earthly to Earthbuild! 🚀
💡 Tips for finding more occurrences
Run locally to see detailed breakdown:
./.github/scripts/count-earthly.shNote that the goal is not to reach 0.
There is anticipated to be at least some occurrences of earthly in the source code due to backwards compatibility with config files and language constructs.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| return "", fmt.Errorf("open %s: %w", procs, err) | ||
| } | ||
|
|
||
| defer f.Close() |
| return fmt.Errorf("create %s: %w", dst, err) | ||
| } | ||
|
|
||
| defer out.Close() |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
gilescope
changed the title
| return Digest{}, fmt.Errorf("create %q: %w", h.Name, err) | ||
| } | ||
|
|
||
| defer f.Close() |
| relaxed := map[string]os.FileMode{} | ||
|
|
||
| for { | ||
| h, err := tr.Next() |
| return fmt.Errorf("read the layer archive: %w", err) | ||
| } | ||
|
|
||
| target, err := res.path(h.Name) |
| // legitimate - /bin/sh -> /busybox is resolved inside the step's own root | ||
| // at run time. What must never happen is this unpacker following it, and | ||
| // safePath is what prevents that. | ||
| err := os.Symlink(h.Linkname, target) |
| return fmt.Errorf("create the blob %s: %w", at, err) | ||
| } | ||
|
|
||
| defer f.Close() |
| return fmt.Errorf("open the blob %s to fill it: %w", at, err) | ||
| } | ||
|
|
||
| defer f.Close() |
Author
|
Currently working on improving earth in earth support. |
| return fmt.Errorf("create %s: %w", dst, err) | ||
| } | ||
|
|
||
| defer out.Close() |
Author
|
Marker: We're green pretty much on CI! |
| tr := tar.NewReader(r) | ||
|
|
||
| for { | ||
| hdr, err := tr.Next() |
| return fmt.Errorf("read the archive: %w", err) | ||
| } | ||
|
|
||
| at, err := within(into, hdr.Name) |
| // the previous build's answer, and `Symlink` refuses to replace. | ||
| _ = os.Remove(at) | ||
|
|
||
| err = os.Symlink(hdr.Linkname, at) |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
An earlier draft called for the guest to keep a map of image references to stacks. That would have been a second copy of what FROM already does, kept in the one place that cannot fetch anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
A sandbox stops itself when nobody has wanted it for a while, and idleness is measured by when a *host* last spoke. A client inside a step is not the host, so without a hold the machine stops itself while it is busiest. The HTTP handler took one from the beginning; nothing on the gRPC path did - which is the half that runs actions, and running an action is the longest thing this service does. A client would see its connection close saying nothing, part-way through. R5 said this was worth a test rather than a comment and it was right: I passed Hold into the Cache, saw it in the struct literal, and did not check that anything called it. Taken in the unary adapter rather than in each method, because a method that forgot would let the machine stop underneath it with no way to notice from inside. The test asserts the release count matches, because a hold taken and not given back is a sandbox that outlives every build on the host - the same failure from the other side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
A client decides how many actions to ask for and this one is inside a sandbox: Buck2 sizes its parallelism from the machine it thinks it is on, so a step given this service could ask for as many as it liked, and every one is a process on a machine already running the step that asked. Nothing bounded them. Their own bound, never the build's. The deadlock R5 warned about does not arise here - the scheduler's pool is host-side and an action never touches it - but its conclusion holds for the other reason: two pools can oversubscribe a machine, which is slow, and prefer slow. A step holding a slot while the actions it spawned wait for slots of their own is a build waiting for itself, which needs a person and a stack dump. Waits rather than refusing. A client told RESOURCE_EXHAUSTED has to decide what to do and what it should do is wait, so waiting here is the same answer with nobody having to implement it - and on the client's context, so one that gave up stops waiting with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
The shape the remote-execution work is for: buck2 inside an `earth` target is supported and buck2 outside one is not, so nothing is installed on a machine - the target's FROM brings the client in. The execution platform is remote-only on purpose. Hybrid would let buck2 quietly run the action locally and report success, which is the one answer this experiment must not be able to give. No prelude rules and no toolchain: what is being tested is whether an action reaches this engine and comes back, and a real toolchain would put a compiler's problems between the question and the answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Buck2's remote-execution client rejects a `unix://` address outright - `Invalid address: invalid format` - so a service reachable only that way is one it cannot use, whatever else works. Found by running it: buck2 got as far as computing an REAPI action digest and then could not build a Capabilities client. The socket stays, because Bazel does take one. The same gRPC server answers both, so there is one service however it is reached. A fixed port rather than one chosen at bind time: the client is configured before the listener exists, so a port it must be told about is one more thing to plumb. 8980 is what REAPI implementations conventionally use. Also splits the example's target platform from its execution platform, which buck2 is strict about and I had conflated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Buck2's remote-execution client speaks TLS unconditionally and has no
plaintext setting: a bare address and a `grpc://` one both handshake,
and `http://` is rejected outright ("you should omit it"). So a service
without a certificate is one it reports as a corrupt message and cannot
use, however correct the protocol underneath.
Not because there is anything to protect - the listener is reachable
only from inside a sandbox this engine started, which is the whole
authorisation model - but because the client insists.
Made per step and never written anywhere durable: the certificate goes
beside the socket, on the ephemeral tmpfs a WITH RE step is given, so it
dies with the step and cannot reach a layer. A key that outlived the
thing it authenticates would be a credential left lying in a build.
The listener is wrapped rather than the server given credentials, so one
server still answers both: the socket has nothing to protect and the TCP
port has a client that will not speak without it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
A client handed a single self-signed certificate as its trust anchor refuses it the moment the server presents that same certificate as its own. rustls calls it `CaUsedAsEndEntity` and buck2 reports it as a connection error, which reads like a network problem and is not one. A CA and a leaf it signed, which is the shape buildkitd/certificates.go already builds for talking to buildkit over gRPC - the same reason, in about a tenth of the code, because none of this is durable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
FindMissingBlobs echoes the digests a client asked about. Ours echoed the hash and wrote nothing for `size_bytes`, which proto3 omits when zero - so every digest of a non-empty blob came back as one the client had never asked about. Buck2 requested twelve, recognised the single empty blob, and reported twenty-three. The golden vector could not catch it, and said so in its own comment: "a client is told which blobs to send, not how big they are", with the comparison written to tolerate the difference. That sentence is not true, and the test was shaped around it - both halves agreeing about a message neither had to justify to a peer. It now compares against protoc's bytes exactly, which it never did. `Blob` carries the pair, because keeping the size in a separate variable from the hash is what made dropping it possible to write. Found by pointing buck2 at the service and reading what it said. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Buck2 refuses a result whose `execution_metadata` is unset - "The execution metadata are not defined" - before it looks at anything inside it. So a service that omitted the message because it had nothing interesting to put there is a service that cannot answer, however correct the rest of the result. Always emitted, never nil, including on a cache hit where there is no worker to name and the timestamps are of a run that happened on another day. Naming the engine makes the message present, which is what is actually being required. An empty message is not the same bytes as no message - the distinction this encoding is careful about everywhere else, pointing the other way for once. The fixtures are regenerated with protoc rather than the comparison loosened, so the goldens still say what a conforming peer writes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
A client reads `Operation.metadata` before it reads the response beside it, and buck2 refuses an Operation without one - "The execution metadata are not defined" - however complete the result. We emitted name, done and response, and nothing for field 2. COMPLETED is the honest stage for the only kind this service sends: one that is already done when it is first delivered. The action digest is quoted back with its size, because a client compares the whole digest. The golden's inner `Any` bytes are protoc's own, generated rather than hand-encoded - a hand-written length is exactly the sort of thing these vectors exist to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Field 6 is `stdout_digest`, so a metadata message written there is read by a peer as a malformed digest and the metadata it requires is simply absent. Buck2 says "The execution metadata are not defined" and is right. The number came from testdata/reapi/reapi_min.proto, which is this repository's own hand transcription of the schema - so the golden agreed with the mistake, byte for byte, and said nothing. A vector generated from a transcription checks that we and the transcription agree; it cannot check the transcription. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Buck2 reads OutputDirectory.tree_digest and refuses a result without one - "Tree digest not defined" - where this engine wrote only root_directory_digest. The plan chose the latter deliberately, because a Tree holds every child inline and says the same thing at greater length; that reasoning stands and does not help, because a client that reads only the older field cannot be argued with. So both. A peer that fetches nodes it lacks reads the reference; one that wants everything at once reads the Tree. Saying it twice costs a blob nobody fetches; saying it once costs a client. The children are sorted, because a map is not: two runs producing the same tree must produce the same Tree message, or its digest is a different name for one filesystem on every build. On the cache path it is best effort - a peer that reads the reference needs nothing there, and failing the whole lookup because the inline form could not be built would deny both kinds of client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Six findings, every one from the client and none from this repository's own tests. Three of them the golden vectors could not have caught: they are generated from our own transcription of the schema, so they prove the encoder and the transcription agree and cannot prove the transcription. Two field numbers and one omitted field were wrong in both at once. Also records that the last gap is confirmed by reaching it: buck2 now gets through the whole protocol and fails in extract_artifacts, wanting one entry per declared output where it gets the delta entire. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update earthbuild/dind docker tag to ubuntu-26.04-docker-29.8.1-1 (#941) Line 48 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(earthfile): rename default installation name to earth-dev (#944) Line 49 of docs-internals/plan-merge-main.md, flagged `check` on the grounds that the native engine reads an installation name for its store and config paths. Checked: it does not, and the answer is nothing owed. The installation name reaches `defaultConfigPath`, `buildkitd. ContainerName`, `VolumeName` and `config.PortOffset` - all of them the old engine's. `engine/` contains no reference to an installation name or a config path at all; its whole settings surface is EARTH_* environment variables, and `storeDir` is EARTH_CACHE_DIR, then $XDG_CACHE_HOME/ earthbuild, then ~/.cache/earthbuild - a fixed string that this rename cannot move. So a dev binary's config directory becomes ~/.earth-dev, and a developer holding ~/.earthly-dev/config.yml will need to move it. That is main's change and lands on main's users identically; the native engine's store is untouched and no cache goes cold. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update dependency earthbuild/earthbuild to v0.8.19 (#950) Line 50 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore: retire the earthly-ci-runner-arg feature flag Line 51 of docs-internals/plan-merge-main.md, flagged `port`. This merge takes main's half; the native engine's equivalent removal follows in its own commit, so that a bisect can tell the two apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
The native half of line 51 of docs-internals/plan-merge-main.md, whose main-side merge is the commit before this one. Main retired `VERSION --earthly-ci-runner-arg` and both builtins it gated, `EARTHLY_CI_RUNNER` and `EARTH_CI_RUNNER` - the product it named was discontinued, so the argument answered a question about a machine nobody runs. The native engine had its own implementation of all three: a `ciRunner` feature bit, an `addCIRunner` that read the environment, and two names pinned into `builtinNameSet` precisely because the call was conditional. All gone. The test is replaced rather than deleted. A retirement has two halves and only one of them shows up in a diff - the builtin stops being supplied, *and* the dialect stops accepting the flag - so `TestTheRetiredCIRunnerFlagIsRefused` asserts both: the flag is refused by name, and each of the two argument names is now an ordinary argument nobody supplied. An engine that quietly ignored an unknown VERSION flag would pass the first half while silently accepting files no other engine accepts, which is how a compatible implementation stops being one. Both old tests were run against the removed code first and both failed, so they were testing this and not passing vacuously. The corpus ratchet drops by one on each platform because main's commit deletes the `earthly-ci-runner-builtin-arg-test` target from tests/builtin-args.earth. darwin 259 -> 258 measured here; linux 258 -> 257 measured in a container rather than inferred, because ratchetSlice fails in both directions and the two platforms already disagreed by one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update alpine docker tag to v3.24.2 (#951) Line 52 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update public.ecr.aws/amazonlinux/amazonlinux docker tag to v2023.12.20260917.1 (#952) Line 53 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update public.ecr.aws/amazonlinux/amazonlinux docker tag to v2027.0.20260914.0 (#953) Line 54 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update jdkato/vale docker tag to v3.22.0 (#955) Line 55 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
fix(deps): update module github.com/dustin/go-humanize to v1.1.0 (#956) Line 56 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
fix(deps): update module google.golang.org/grpc to v1.84.0 (#957) Line 57 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
perf: skip work that ls, doc, init and config never use (#958) Line 58 of docs-internals/plan-merge-main.md, where the ledger said `done` - this branch is where the idea came from, and `92cde118a` and `fcb82ec05` are its native half. Both are ancestors here, but main's own commit still had to be merged or the branch stays behind it, and it does not resolve to "take ours" everywhere. Two implementations of one idea met, and each had something the other lacked: - `cmd/earth/app/before.go` keeps this branch's gate. It answers a wider question - `needsContainerFrontend` also skips detection for the native engine, which is most builds - and main's `needsFrontend` only covers the subcommand half of it. - `cmd/earth/subcmd/ls_cmds.go` takes main's, which is the strict superset: it has this branch's parse-rather-than-resolve path *and* `findBuildFile`/`nearestEarthfileDir`, the ancestor walk that also stops on `build.earth`. Keeping ours would have dropped that and left main's tests for it dangling. - `earthfile2llb` keeps this branch's `TargetsIn`, which main has no equivalent of, and renames `TargetArgsIn` to main's `TargetArgs`. One function under two names across a merge is how the next one conflicts for no reason. Main's `TestNeedsFrontend` is not carried, because the function it tests is not the one this branch kept. It documents a real defect that this branch's gate still has - a global flag's value read as the subcommand - and that is fixed in the commit after this one, where it can be a change rather than a merge artefact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
The native half of main's #958 fix, whose merge is the commit before this one. Main found the shape and fixed its own gate; this branch's gate had it too and was not covered by that merge, because the two engines gate different things and only main's function was replaced. `needsContainerFrontend` decided whether to probe for a Docker or Podman daemon, and found the subcommand by scanning the raw arguments for anything matching a command name, first match winning. A global flag takes a value, and a value can read like a command: earth --git-username ls prune answers for `ls`, which touches no daemon, and hands `prune` - which is entirely about the daemon - a stub frontend. It then prunes nothing and says so in neither direction. Main's own repro was `--git-username doc build +all`. Root flags are parsed by the time `before` runs, so the subcommand now comes from the parser as `cmd.Args().First()` rather than from a scan. The engine half still reads raw arguments and still must: `--engine` is a *build subcommand* flag, so it genuinely is not parsed yet, and guessing wrong there costs the 116ms that was always being spent. Narrower here than upstream, because the engine check runs first and a non-native engine short-circuits to "detect" - so only a native invocation whose global flag value reads as `build` or `ls` could reach it. It reaches it. Two failing cases were added first and both failed against the old scan; they pass now, and state the parsed subcommand per case rather than deriving it, since deriving it would reimplement the defect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
refactor(telemetry): remove redundant stdr logger dependency (#959) Line 59 of docs-internals/plan-merge-main.md, flagged `check` on the grounds that the native engine links its own telemetry path. Checked: nothing owed. `telemetry.Setup` stops calling `otel.SetLogger`, so OpenTelemetry's own internal diagnostics go to its default handler. `cmd/earth/main.go` is the only caller and runs for both engines, but `engine/` imports no opentelemetry package at all - not one file - so the native engine emits no spans and has no internal diagnostics to lose. Its own instrumentation is the EARTH_TIMINGS phase log, which shares nothing with otel but the word. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv # Conflicts: # go.sum
chore: rename Earthfile build workdir from /earthly to /earth (#943) Line 60 of docs-internals/plan-merge-main.md, flagged `check`. No executable reference in Go depends on the workdir being /earthly: every occurrence under engine/ is prose about this repository's own Earthfile, and the remaining matches are the github.com/earthly/earthly URL and conslogging's prefix fixtures, which name a repository rather than a path. Nor does any script or workflow - all their /earthly hits are release-download URLs. The Earthfile itself is not so lucky. `+code` now saves `/earth`, and this branch has targets main does not, which still said `COPY --dir +code/earthly /` - an artifact that no longer exists. Those would have failed at this commit, so they move here rather than later: two COPY lines, the guest test invocation that runs from `/earthly/engine/guest`, the corpus directory handed to the build test, and the mutation sweep's `cd`. Deliberately *not* renamed: `./earthly`, `+earthly-linux-amd64/earthly` and `./build/<os>/<arch>/earthly`. Those are the script, a target and the binary - the tool's name, not the working directory - and main left them alone too. Both sides of the two conflicts are kept: this branch's `go telemetry off` and its `examples/` lint exclusion sit on lines adjacent to the ones main renamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
Follows the merge of main's #943, which moved the Earthfile's WORKDIR from /earthly to /earth. No code read that path, but six comments named it, and they are comments that exist to explain why particular tests are shaped the way they are - a reader who greps the Earthfile for the path they name now finds nothing. `engine/cli/corpusvanish_test.go` keeps its /earthly deliberately. That one is a verbatim capture of a failure from a real run, down to the building directory's pid, and a captured log that has been edited to match today's paths is no longer evidence of anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
feat: add --no-image-output to skip loading images locally (#858) Line 61 of docs-internals/plan-merge-main.md, flagged `port` - the last of the three. The native half is in this commit rather than the next one, because `TestEveryFlagIsClassified` will not let a flag arrive undecided: "a flag nobody decided about is one that is accepted and does nothing". Merging alone leaves that red, so the decision lands with the merge. The decision is to honour it. Upstream's knob skips *loading an image into a local daemon*; this engine writes an OCI layout instead, which is the same write to the same filesystem and the same thing a build might not want done to the machine it runs on. `Options.NoImageOutput` guards `writeImages` exactly as `NoOutput` guards `exportAll` - before anything is looked up, so the steps still run and the cache is still filled. `TestNoImageOutputWritesNothing` asserts it the way its neighbour does, with a nil executor: without the guard the call reaches `e.Sandbox()` and panics, so returning cleanly proves nothing was consulted. It was written first and failed to compile, then failed on the nil, then passed. Three more guards fired on the way through and all three were right: `TestEveryOptionIsAccountedFor` wanted the new field exercised, and the corpus ratchet rose because main ships two new test Earthfiles with thirteen targets between them, five of which plan. darwin 523 -> 528 measured here, linux 515 -> 520 measured in a container. Also removed two mutation anchors pointing at the CI-runner builtin, retired at line 51. They were missed there because that commit was verified with ./engine/... and tools/mutate is not under it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
chore(deps): update amazonlinux (#963) Line 62 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
fix(deps): update dependency joda-time:joda-time to v2.14.4 (#964) Line 63 of docs-internals/plan-merge-main.md. Nothing under engine/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
`git rev-list --count HEAD..origin/main` is 0. Build green for linux/amd64, seventy-nine packages green. Records the two things the ledger got wrong, because both are the kind of mistake that repeats. All three `check` verdicts turned out to owe nothing - each for a reason about what engine/ does not import, which is worth having written down. And the single line marked `done` was the only genuinely wrong entry: it meant "we had this idea first", which is not the same as "nothing to merge", and merging it anyway surfaced a live defect in this branch's frontend gate. Also records the four guards that caught things reading did not, including the two mutation anchors that line 51 left dangling because it was verified with ./engine/... and tools/mutate is not under it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
| tr := tar.NewReader(r) | ||
|
|
||
| for { | ||
| hdr, err := tr.Next() |
`go test -race ./engine/helper/` fails: wazero v1.12.0's `internal/version.GetWazeroVersion` memoises the module version into a package-level variable with no synchronisation, and `NewRuntimeWithConfig` reaches it on every call. Two goroutines opening a helper at once read and write it concurrently, and the race detector fails the whole binary - which is how one upstream global took four packages red in CI. Reproduced here before and after: red on the same command, green now. Not serialised in general, which would be the obvious fix and the wrong one: a helper is opened per cache mount, and making that a global bottleneck to work around somebody else's unsynchronised variable trades a real property for a borrowed bug. A `sync.Once` around the first construction is enough - the variable is written once and only read afterwards, and Once gives every later caller the happens-before it needs to read it safely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
`TestAnActionMayNameTheImageItsStepStandsOn` failed in CI with `mount /proc for the step: operation not permitted`, while every sibling that needs the same privilege skipped with a named reason. Two of its five cases actually run the action; the other three never reach a mount, which is why only those two went red. Guarded with `guest.CanIsolate`, the engine's own probe, rather than by matching the kernel's message. "operation not permitted" is what this kernel says today, and a harness that recognises one phrasing turns every other into a false failure - the argument `nstest.Unstartable` already makes in its own doc comment. Proven in both directions, because a skip that fires everywhere is a deleted test: it skips in an unprivileged container, naming the reason, and still runs all five cases on a machine that can mount. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
fleet-e2e triggers on engine/, cmd/earth-worker/, cmd/earth-native/ and
tests/fleet/ - our own code. The transport is mostly not our own code: a
worker reaches its driver over QUIC and fetches blobs over a TLS session
whose identity checking lives in x/crypto.
Today's run failed on exactly that, `CRYPTO_ERROR 0x12a` - QUIC's
0x100 + TLS alert 42, bad_certificate - on the blob connection, with no
file in the trigger list touched by the commit that moved x/crypto. It
ran only because unrelated engine/ changes rode in beside it.
The comment above the list already made this argument about our own
packages ("a trigger list that names only the obvious package tests the
obvious package"). go.mod and go.sum are the same argument one step out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
**Revert this once CI has answered.** It is a single-variable experiment, not a decision about the dependency. fleet-e2e was green on 0ec18db (20 Sep) and failed on c51b3b3 with `CRYPTO_ERROR 0x12a` - bad_certificate - when a worker tried to fetch a delegated step's inputs. Between those two commits the only changes in the transport stack were x/crypto v0.55.0 -> v0.57.0 and x/sys v0.47.0 -> v0.48.0; go-iroh and quic-go did not move. x/crypto is the larger jump and the one that owns TLS material, so it is the variable to move first. If this run is green, the bump is the cause and the real question is whether v0.57 is right to reject what the blob endpoint presents - the plan's own gate 7 says that endpoint's key is ephemeral and no allowlist is consulted, so a stricter stack refusing it would be correct behaviour exposing a known gap rather than a regression to paper over. If this run is still red, x/crypto is exonerated and x/sys is next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
`ee34673d9` guarded `NewRuntimeWithConfig` and left `NewCompilationCacheWithDir` racing on the identical variable, eleven lines above it. `engine/helper` and `engine/guest` went green and `engine/cacheshare` and `engine/exec` stayed red, for the same upstream global and a stack that differed in one frame. A guard placed at a call site is only as good as the enumeration of call sites behind it, which is the mistake and not the typo. So this warms wazero's version global once under a `sync.Once` at the top of `Open`, before anything that reads it, instead of guarding a particular constructor - which holds whatever entry point a future wazero adds. Enumerated rather than assumed this time: every `wazero.NewRuntime*`/`NewCompilationCache*` in the tree is inside helper.go, and every one that a caller reaches is inside `Open` after the warm-up. `engine/exec` never constructs one itself; it raced through `cacheshare` -> `helper.Open`. Verified with `go test -race` on all four packages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
…ading `go test -race ./engine/exec/` on linux reports a data race between `os.(*File).Close` and `os.(*File).Fd` on the same file: the test closes the descriptor it handed to `dupOf` while the goroutine inside `serveNetFDs` is still duplicating it. The ordering is the whole bug. A test's `defer` runs when the test function returns; its `t.Cleanup` functions run after that. So `defer held.Close()` fired first, and `listenAt`'s cleanup - which closes the listener and waits for the serving goroutine - had not run yet. The window is exactly as long as a request in flight, which is why it needed the race detector and a loaded runner to show. Fixed by closing through `t.Cleanup` registered *before* `listenAt`: cleanups are LIFO, so the listener stops and the goroutine is waited for before the file it was reading goes away. Both sites in the file had it. Verified on linux in a container, where it reproduced: two races before, none after, same command. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
…failure" This reverts commit cced0ea. The experiment answered, and the answer was not the one the first green run suggested. **x/crypto is exonerated by inspection.** Between v0.55.0 and v0.57.0 the only packages that change are `acme` and `ssh`, plus its own go.mod; `cryptobyte`, `hkdf`, `chacha20`, `chacha20poly1305` and `curve25519` are byte-identical, and those are every x/crypto package go-iroh's TLS path imports. The transitive bumps in that go.mod do not move this build either - tidying after the downgrade changed two lines of go.sum. So there is no mechanism by which this version could reject a peer's raw public key. **And the bisect was not sound.** fleet-e2e has failed 8 times in 40 runs on this branch - 20% - so one red run against one green run is not evidence, which is a rule I hold and did not apply. Retracted. What is true, and is the part worth keeping: today's symptom is novel. Every earlier failure was "both workers did not join", a rendezvous timing flake. `CRYPTO_ERROR 0x12a` - bad_certificate on the blob endpoint - has never appeared before. That deserves an explanation and has not got one yet. So the pin goes, and the next run on v0.57.0 is a real second data point rather than a confirmation of a guess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
fleet-e2e has failed 8 times in 40 runs on this branch - 20% - and seven of those were "both workers did not join". That flakiness is not a nuisance: it is why today's novel `CRYPTO_ERROR` could not be read as signal, because one red run against a 20% base rate says nothing. The cause is an asymmetry between two numbers that were never related. A driver waits `EARTH_FLEET_WAIT` for workers - 8 minutes in CI. A worker waited `DefaultPatience`, a constant 2 minutes, for the driver. Nothing tied them together, and the driver is systematically the slower side to appear, because it builds the engine *and* the guest before it can listen. Run 7178389 is the shape in full: the worker announced itself at 08:25:31 and gave up around 08:27:31; the driver began listening at 08:29:37 and waited, alone, until 08:37:37. Two processes waiting for each other, four minutes apart, neither at fault. So a worker now takes its patience from the driver's own declared willingness to wait, never shortening below the old default, and `EARTH_FLEET_WAIT` moves to workflow scope so both sides read one number. A fleet with nothing configured is unchanged. `DefaultPatience`'s own comment already said "a worker started before its driver is the normal case in CI, where the jobs start together" - the observation was right and the constant beside it was two minutes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ2c9xLfQvSY8YGLr7o5tv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
NOT FOR USE YET. SECURITY NOT YET LOCKED DOWN.
A native build engine, built alongside the existing buildkit one rather than in
place of it. Nothing here changes what
earthlydoes today: the engine ships asseparate binaries (
earth-native,earth-guestd,earth-worker) and theexisting CLI is untouched, so this is additive and safe to land incrementally.
What it is
A build engine that plans, schedules and executes Earthfiles without buildkit:
an interpreter, a content-addressed layer store, a two-tier cache (Κ₁ on
declared inputs, Κ₂ on observed ones), and a sandbox backed by Apple's
containeron darwin and namespaces on linux. The specification it conforms tois
docs-internals/green-paper.md; the plan isplan-native-engine.md; everymeasurement quoted below is written up in
experiments-adversarial.md.Where it is
+earthlyand+all-binariesbuild correctly, the latter producing fiveplatform binaries in three executable formats.
+unit-testruns.+allstopsat
+all-buildkitd, which needs to run amd64 steps on an arm64 machine - thatis emulation this engine does not have, and it says so.
Head to head on this repository:
+earthly, warm+unit-test, coldBoth numbers are honest and neither is the whole story: a warm
+earthlyis ano-op and measures the cache, while
+unit-testmeasures execution and isdominated by two packages that hang or run long under this engine. The gap is
being worked through with measurements rather than intuition, and the write-ups
include the several times an obvious explanation turned out to be wrong.
Why the CI signal is wanted
This has been developed on one machine, on darwin, on arm64. CI runs linux on
amd64 with a different filesystem, a different sandbox and a real fleet of
runners - which is exactly the set of assumptions a single developer's machine
cannot test.
The engine is wired into the shipped CLI and is the default there, and that is
deliberate and temporary.
--enginetakesnativeorbuildkit, dispatchesbefore either engine's setup runs, and defaults to
native- so every suite jobthat invokes the built binary exercises the new engine instead of describing it.
That is the whole point: a suite that has been run against buildkit for years is
a better differential than any test written on purpose, and what it reports is
where the two engines disagree. The default goes back to
buildkitonce CI isfully green; until then, a red job here is the signal being asked for rather
than a regression in the shipped path.
--engine=buildkitrestores the old behaviour exactly, and the native pathrefuses rather than guesses where it does not yet reach: a remote target, an
artifact reference, or more than one
--platform.Known open items, all written up in
docs-internals/experiments-adversarial.md:engine/traceexits non-zero as a package with every one of its tests passing(E626, E627 - nine seccomp-filtered threads that never ended, now fixed and
awaiting a CI verdict); a seccomp deadlock seen twice under concurrency; no guard
on the platforms the release ships; and roughly 1,650 lint findings in the new
code, being worked down package by package -
Fast Checkis red on those and onnothing else that is understood.
Draft, because the diff is large and the author has not read it end to end.