feat: add interactive build-docker#682
Conversation
➖ Are we earthbuild yet?No change in "earthly" occurrences 📈 Overall Progress
Keep up the great work migrating from Earthly to Earthbuild! 🚀 💡 Tips for finding more occurrencesRun locally to see detailed breakdown: ./.github/scripts/count-earthly.shNote that the goal is not to reach 0. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for in-memory Earthfile AST generation and resolution, allowing Dockerfiles to be translated and debugged interactively without writing temporary files to disk. Key changes include adding an in-memory Earthfile registry to the resolver, implementing a Dockerfile-to-Earthfile AST compiler with source line mapping in docker2earth, and updating the build command to utilize this in-memory flow during interactive debugging. Feedback on these changes highlights opportunities to clean up dead code in docker2earth/ast.go (such as the unused lineWriter and redundant first parsing loop), simplify identical nested if-else branches, and avoid using an if statement with an initializer in buildcontext/resolver.go to prevent potential linting issues.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| lw := &lineWriter{ | ||
| buf: &bytes.Buffer{}, | ||
| currentLine: 0, | ||
| lineMap: []int{0}, | ||
| } | ||
|
|
||
| // Header | ||
| lw.WriteLine(fmt.Sprintf("VERSION %s", earthCurrentVersion), 1) | ||
|
|
||
| names := map[string]int{} | ||
|
|
||
| for i, stage := range stages { | ||
| stageLine := 1 | ||
| if len(stage.Location) > 0 { | ||
| stageLine = stage.Location[0].Start.Line | ||
| } | ||
|
|
||
| lw.WriteLine(fmt.Sprintf("subbuild%d:", i+1), stageLine) | ||
|
|
||
| // Initial ARGs | ||
| if i == 0 && len(initialArgs) > 0 { | ||
| for _, arg := range initialArgs { | ||
| argLine := 1 | ||
| if len(arg.Location()) > 0 { | ||
| argLine = arg.Location()[0].Start.Line | ||
| } | ||
| lw.WriteLine(" "+arg.String(), argLine) | ||
| } | ||
| } | ||
|
|
||
| // FROM command | ||
| fromLine := fmt.Sprintf(" FROM %s", stage.BaseName) | ||
| if stage.Platform != "" { | ||
| fromLine = fmt.Sprintf(" FROM --platform=%s %s", stage.Platform, stage.BaseName) | ||
| } | ||
| lw.WriteLine(fromLine, stageLine) | ||
|
|
||
| if stage.Name == "" { | ||
| names[strconv.Itoa(i)] = i | ||
| } else { | ||
| names[stage.Name] = i | ||
| } | ||
|
|
||
| for _, cmd := range stage.Commands { | ||
| cmdLine := 1 | ||
| if len(cmd.Location()) > 0 { | ||
| cmdLine = cmd.Location()[0].Start.Line | ||
| } | ||
|
|
||
| l := fmt.Sprintf("%v", cmd) | ||
| if strings.HasPrefix(l, "COPY ") && strings.Contains(l, "--from") { | ||
| parts := strings.Split(l, " ") | ||
| if len(parts) == 4 { | ||
| kv := strings.Split(parts[1], "=") | ||
| if len(kv) == 2 { | ||
| fromStageName := kv[1] | ||
| n, ok := names[fromStageName] | ||
| if ok { | ||
| artifactName := getArtifactName(parts[2]) | ||
| lw.WriteLine(fmt.Sprintf(" COPY +subbuild%d/%s %s", n+1, artifactName, parts[3]), cmdLine) | ||
| // Insert SAVE ARTIFACT in the source stage | ||
| // Note: to simplify in-memory representation, we just append it in the generated string. | ||
| // The Earthfile parser handles this. | ||
| // Wait, we need to append SAVE ARTIFACT line-mapped to the current COPY line so it's clean. | ||
| // Since it's in the target target, let's map it to the stage's target recipe. | ||
| // But wait, the SAVE ARTIFACT must be executed in target n+1 (which is target i). | ||
| // Since we are iterating, target n+1 was generated previously, so we cannot easily write to its recipe | ||
| // here if it's already written. | ||
| // Wait, the original convert.go writes targets by appending to a targets slice targets[n+1]. | ||
| // Let's replicate that logic to ensure COPY --from works! | ||
| } | ||
| } | ||
| } | ||
| } else if strings.HasPrefix(l, "ADD ") { | ||
| return earthfile.Tree{}, errors.Errorf("earth does not support ADD, please convert to COPY instead") | ||
| } else { | ||
| lw.WriteLine(" "+l, cmdLine) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // We need to re-implement the targets slice generation so that we can support SAVE ARTIFACT appending dynamically! | ||
| // Let's rewrite the target generation block to match Docker2Earth exactly but with line mapping. | ||
| return generateASTFromStages(stages, initialArgs, names, absDockerfilePath, imageTag) |
There was a problem hiding this comment.
The first loop in GenerateNativeEarthfileAST is completely redundant and its output lw is never used. All AST generation and line mapping are handled by generateASTFromStages. This dead code can be safely removed.
names := make(map[string]int)
return generateASTFromStages(stages, initialArgs, names, absDockerfilePath, imageTag)|
|
||
| if isInMemory { | ||
| localDirs := make(map[string]string) | ||
| if _, isTarget := ref.(domain.Target); isTarget { |
There was a problem hiding this comment.
Avoid using an if statement with an initializer in Go to prevent potential conflicts with project linting rules. Declare the variable on a preceding line instead.
| if _, isTarget := ref.(domain.Target); isTarget { | |
| _, isTarget := ref.(domain.Target) | |
| if isTarget { |
References
- In Go, avoid using an if statement with an initializer. Instead, declare the variable on a preceding line to prevent potential conflicts with project linting rules.
| type lineWriter struct { | ||
| buf *bytes.Buffer | ||
| currentLine int | ||
| lineMap []int | ||
| } | ||
|
|
||
| func (lw *lineWriter) WriteLine(content string, dockerfileLine int) { | ||
| lw.buf.WriteString(content) | ||
| lw.buf.WriteByte('\n') | ||
| lw.currentLine++ | ||
| for len(lw.lineMap) <= lw.currentLine { | ||
| lw.lineMap = append(lw.lineMap, 0) | ||
| } | ||
| lw.lineMap[lw.currentLine] = dockerfileLine | ||
| } |
There was a problem hiding this comment.
The lineWriter struct and its WriteLine method are unused and can be safely removed.
| for i, targetLines := range targets { | ||
| for j, tl := range targetLines { | ||
| if i == 0 { | ||
| buf.WriteString(tl.content) | ||
| buf.WriteByte('\n') | ||
| currentLine++ | ||
| lineMap = append(lineMap, tl.line) | ||
| } else { | ||
| if j == 0 { | ||
| buf.WriteString(tl.content) | ||
| buf.WriteByte('\n') | ||
| currentLine++ | ||
| lineMap = append(lineMap, tl.line) | ||
| } else { | ||
| buf.WriteString(tl.content) | ||
| buf.WriteByte('\n') | ||
| currentLine++ | ||
| lineMap = append(lineMap, tl.line) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The nested if-else branches inside the loop perform identical operations. This can be simplified to a single block without checking the indices i and j.
for _, targetLines := range targets {
for _, tl := range targetLines {
buf.WriteString(tl.content)
buf.WriteByte('\n')
currentLine++
lineMap = append(lineMap, tl.line)
}
}1 participant
Support interactive debugging when building Docker images.