From b4df803d969aa3e46148bf9cde755983dc7a5e8a Mon Sep 17 00:00:00 2001 From: bilby91 Date: Fri, 22 May 2026 20:31:37 -0300 Subject: [PATCH 1/3] engine: add Engine.Build for image-only builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #78. Engine.Build resolves a workspace's devcontainer.json and produces the final container image (base image plus feature pipeline) without creating or running a container. Unblocks a future CLI \`build\` subcommand and matches the embedding-relevant subset of upstream \`devcontainer build\`. Scope decisions (agreed on the issue): - updateRemoteUserUID is deliberately skipped. The output is meant to be portable (push to a registry, reuse across hosts); UID reconcile bakes the calling host's UID into the image. - Compose sources are refused with a typed error — matching upstream. - Image source with no features short-circuits: pull and return the ref unchanged. ImageName is ignored in that case (the runtime has no TagImage primitive yet — tracked as a follow-up). - Single ImageName tag (not []string). Multi-tag, --push, --cache-to require BuildSpec extensions on both backends; first-pass ships what we already support. Implementation factors through the existing prepareBaseImage + layerFeatures helpers via a new unexported buildOverride threaded on UpOptions. Engine.Up sets it nil; Engine.Build populates FinalTag, BaseIsFinal (so BuildSource-no-features tags the base directly), NoCache, Platform, and ExtraCacheFrom. Integration tests cover: image-source short-circuit, build+features happy path with ImageName, build-no-features ImageName-on-base, compose refusal, and a no-container-left-behind assertion. Co-Authored-By: Claude Opus 4.7 (1M context) --- build.go | 143 +++++++++++++++++++++++++ test/integration/build_test.go | 185 +++++++++++++++++++++++++++++++++ up.go | 59 ++++++++++- 3 files changed, 386 insertions(+), 1 deletion(-) create mode 100644 build.go create mode 100644 test/integration/build_test.go diff --git a/build.go b/build.go new file mode 100644 index 0000000..f692dbe --- /dev/null +++ b/build.go @@ -0,0 +1,143 @@ +package devcontainer + +import ( + "context" + "fmt" + + "github.com/crunchloop/devcontainer/config" + "github.com/crunchloop/devcontainer/events" +) + +// BuildOptions configures Engine.Build. +type BuildOptions struct { + // LocalWorkspaceFolder is the absolute host path to the project. Required. + LocalWorkspaceFolder string + + // ConfigPath is the absolute path to devcontainer.json. If empty, + // discovered under LocalWorkspaceFolder per Resolve's rules. + ConfigPath string + + // LocalEnv overrides os.Environ() for ${localEnv:*} resolution. + // Nil means use the current process environment. + LocalEnv map[string]string + + // ImageName, when non-empty, is the tag applied to the final + // produced image. It replaces the engine's auto-generated tag + // (dc-go-final-:latest or dc-go-base-:latest). Ignored for + // pure image sources with no features layered (there is nothing + // to retag — the engine would need a TagImage primitive it + // doesn't currently expose; tracked as a follow-up). + ImageName string + + // NoCache forces --no-cache on every BuildImage call in the chain + // (Dockerfile build, features build). + NoCache bool + + // PullPolicy controls when the base image is pulled. Default + // IfNotPresent — same as Up. + PullPolicy PullPolicy + + // Platform pins the target platform (e.g. "linux/amd64", + // "linux/arm64") on every BuildImage call. Empty leaves it + // unspecified. + Platform string + + // CacheFrom is appended to whatever the source declares + // (devcontainer.json `build.cacheFrom`). Empty means no extra + // cache sources. + CacheFrom []string + + // Events optionally receives structured engine events for the + // duration of this Build call: ConfigResolved, ConfigWarning, + // BuildStart/Log/Layer/Completed, FeatureResolveStart, FeatureResolved. + // Drop-on-full; the engine never blocks on send. See package events + // (experimental until v1.0.0). + // + // Ownership: the caller owns the channel. The engine only writes — + // it never closes the channel. The caller MUST NOT close it before + // Build returns. + Events chan<- events.Event +} + +// BuildResult is the outcome of Engine.Build. +type BuildResult struct { + // ImageID is the tag of the produced image. When BuildOptions.ImageName + // was set and applicable, it equals that. Otherwise it's the engine's + // auto-generated tag (dc-go-final-:latest or dc-go-base-:latest). + ImageID string +} + +// Build resolves a workspace's devcontainer.json and produces the final +// container image (base image plus the feature pipeline) without +// creating or running a container. +// +// Compose sources are refused with a typed error — use the compose +// workflow for those. +// +// updateRemoteUserUID is deliberately skipped: Build's output is +// designed to be portable (pushed to a registry, reused across hosts), +// while UID reconciliation bakes the calling host's UID into the +// image. Use Engine.Up if you need a UID-reconciled local image. +// +// For pure image sources with no features, Build short-circuits: it +// just ensures the image is present locally and returns its ref. +// BuildOptions.ImageName is ignored in that case (no build step to +// retag); a TagImage primitive is tracked as a follow-up. +func (e *Engine) Build(ctx context.Context, opts BuildOptions) (*BuildResult, error) { + if err := ctxIfDone(ctx); err != nil { + return nil, err + } + if opts.LocalWorkspaceFolder == "" { + return nil, fmt.Errorf("BuildOptions.LocalWorkspaceFolder is required") + } + + bus := newEventBus(e.emitter, opts.Events) + defer bus.Close() + + cfg, err := Resolve(ctx, ResolveOptions{ + LocalWorkspaceFolder: opts.LocalWorkspaceFolder, + ConfigPath: opts.ConfigPath, + LocalEnv: opts.LocalEnv, + }) + if err != nil { + return nil, err + } + bus.Emit(events.ConfigResolvedEvent{Config: cfg}) + for _, w := range cfg.Warnings { + bus.Emit(events.ConfigWarningEvent{Code: string(w.Code), Message: w.Message}) + } + + if _, isCompose := cfg.Source.(*config.ComposeSource); isCompose { + return nil, fmt.Errorf("Engine.Build: compose-source devcontainers are not supported") + } + + _, isBuildSource := cfg.Source.(*config.BuildSource) + baseIsFinal := isBuildSource && len(cfg.Features) == 0 + + upOpts := UpOptions{ + LocalWorkspaceFolder: opts.LocalWorkspaceFolder, + ConfigPath: opts.ConfigPath, + LocalEnv: opts.LocalEnv, + PullPolicy: opts.PullPolicy, + bus: bus, + override: &buildOverride{ + FinalTag: opts.ImageName, + BaseIsFinal: baseIsFinal, + NoCache: opts.NoCache, + Platform: opts.Platform, + ExtraCacheFrom: opts.CacheFrom, + }, + } + + baseImage, err := e.prepareBaseImage(ctx, cfg, upOpts) + if err != nil { + return nil, err + } + + finalImage, _, err := e.layerFeatures(ctx, cfg, baseImage, upOpts) + if err != nil { + return nil, err + } + + return &BuildResult{ImageID: finalImage}, nil +} diff --git a/test/integration/build_test.go b/test/integration/build_test.go new file mode 100644 index 0000000..6c05366 --- /dev/null +++ b/test/integration/build_test.go @@ -0,0 +1,185 @@ +//go:build integration + +package integration + +import ( + "context" + "path/filepath" + "strings" + "testing" + "time" + + devcontainer "github.com/crunchloop/devcontainer" +) + +// TestEngineBuild_ImageSource_NoFeatures exercises the short-circuit: +// pure image source with no features should pull the image and return +// its ref unchanged, with ImageName ignored (there's nothing to retag +// — a TagImage primitive is a tracked follow-up). +func TestEngineBuild_ImageSource_NoFeatures(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{"image": "`+testImage+`"}`) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + res, err := eng.Build(ctx, devcontainer.BuildOptions{ + LocalWorkspaceFolder: ws, + ImageName: "ignored-because-no-build-step:latest", + }) + if err != nil { + t.Fatalf("Build: %v", err) + } + if res.ImageID != testImage { + t.Errorf("ImageID = %q, want %q (ImageName should be ignored when no build step exists)", res.ImageID, testImage) + } +} + +// TestEngineBuild_BuildSource_WithFeature is the canonical happy path: +// a Dockerfile source plus a local feature. Build should run both +// stages (base Dockerfile, then feature-layered final image) and +// return the final tag. With ImageName set, the final image carries +// that tag. +func TestEngineBuild_BuildSource_WithFeature(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeBuildSourceWorkspace(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + const customTag = "dc-go-build-test:v1" + res, err := eng.Build(ctx, devcontainer.BuildOptions{ + LocalWorkspaceFolder: ws, + ImageName: customTag, + }) + if err != nil { + t.Fatalf("Build: %v", err) + } + if res.ImageID != customTag { + t.Errorf("ImageID = %q, want %q", res.ImageID, customTag) + } + + // Confirm the image actually exists locally — Build should not + // have created or run any container, only produced the image. + details, err := rt.InspectImage(ctx, res.ImageID) + if err != nil { + t.Fatalf("InspectImage(%s): %v", res.ImageID, err) + } + if details == nil { + t.Fatalf("InspectImage(%s) returned nil", res.ImageID) + } + + // Clean up the tagged image so subsequent runs don't accumulate. + t.Cleanup(func() { + _ = rt.RemoveImage(context.Background(), res.ImageID) + }) +} + +// TestEngineBuild_BuildSource_NoFeatures: when there's a Dockerfile +// but no features, the base build IS the final build, so ImageName +// applies to it directly. +func TestEngineBuild_BuildSource_NoFeatures(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "Dockerfile"), "FROM "+testImage+"\nRUN echo build-no-features > /etc/marker\n") + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), + `{"build": {"dockerfile": "Dockerfile", "context": ".."}}`) + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + + const tag = "dc-go-build-test-base-only:v1" + res, err := eng.Build(ctx, devcontainer.BuildOptions{ + LocalWorkspaceFolder: dir, + ImageName: tag, + }) + if err != nil { + t.Fatalf("Build: %v", err) + } + if res.ImageID != tag { + t.Errorf("ImageID = %q, want %q", res.ImageID, tag) + } + t.Cleanup(func() { _ = rt.RemoveImage(context.Background(), tag) }) +} + +// TestEngineBuild_ComposeRefused: compose sources are refused with a +// clear error rather than silently doing nothing. +func TestEngineBuild_ComposeRefused(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "compose.yaml"), + "services:\n app:\n image: "+testImage+"\n command: [\"sleep\", \"infinity\"]\n") + mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), + `{"dockerComposeFile": "../compose.yaml", "service": "app", "workspaceFolder": "/workspace"}`) + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + _, err := eng.Build(ctx, devcontainer.BuildOptions{LocalWorkspaceFolder: dir}) + if err == nil { + t.Fatal("Build accepted a compose source; expected refusal") + } + if !strings.Contains(err.Error(), "compose") { + t.Errorf("error %q should mention compose", err.Error()) + } +} + +// TestEngineBuild_NoContainerCreated: Build must not leave a container +// behind for the workspace it builds — that's Up's job. +func TestEngineBuild_NoContainerCreated(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeBuildSourceWorkspace(t) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + res, err := eng.Build(ctx, devcontainer.BuildOptions{LocalWorkspaceFolder: ws}) + if err != nil { + t.Fatalf("Build: %v", err) + } + t.Cleanup(func() { _ = rt.RemoveImage(context.Background(), res.ImageID) }) + + // Resolve the workspace ID through a second pass; any container + // labelled with it would indicate Build ran a container. + cfg, err := devcontainer.Resolve(ctx, devcontainer.ResolveOptions{LocalWorkspaceFolder: ws}) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + c, err := rt.FindContainerByLabel(ctx, devcontainer.LabelDevcontainerID, cfg.DevcontainerID) + if err != nil { + t.Fatalf("FindContainerByLabel: %v", err) + } + if c != nil { + t.Errorf("Build left a container behind (id=%s) — should be image-only", c.ID) + } +} diff --git a/up.go b/up.go index 54ccb06..ab3e517 100644 --- a/up.go +++ b/up.go @@ -99,6 +99,32 @@ type UpOptions struct { // through internal helpers via the UpOptions value copy. Never read or // written by callers. bus *eventBus + + // override carries build-time knobs supplied by Engine.Build callers + // (final image tag, --no-cache, --platform, additional --cache-from). + // Engine.Up itself leaves this nil; prepareBaseImage and layerFeatures + // merge override values into their runtime.BuildSpec when non-nil. + override *buildOverride +} + +// buildOverride is the bag of build-time knobs Engine.Build threads +// through Up's internal helpers. Internal-only — Engine.Build sets it, +// Engine.Up leaves it nil. +type buildOverride struct { + // FinalTag, when non-empty, replaces the auto-generated tag of the + // *last* BuildImage call in the chain. Which call that is depends + // on BaseIsFinal: when true the base Dockerfile build is the last + // step (BuildSource with no features); otherwise it's the features + // build (any source with at least one feature). + FinalTag string + BaseIsFinal bool + // NoCache forces --no-cache on every BuildImage call in the chain. + NoCache bool + // Platform pins the target platform on every BuildImage call. + Platform string + // ExtraCacheFrom is appended to whatever CacheFrom the source + // declares (devcontainer.json `build.cacheFrom`). + ExtraCacheFrom []string } // PullPolicy controls when images are pulled from a registry. @@ -860,6 +886,19 @@ func (e *Engine) prepareBaseImage(ctx context.Context, cfg *config.ResolvedConfi case *config.BuildSource: tag := "dc-go-base-" + cfg.DevcontainerID + ":latest" + cacheFrom := s.CacheFrom + var ( + noCache bool + platform string + ) + if opts.override != nil { + if opts.override.BaseIsFinal && opts.override.FinalTag != "" { + tag = opts.override.FinalTag + } + noCache = opts.override.NoCache + platform = opts.override.Platform + cacheFrom = append(append([]string(nil), cacheFrom...), opts.override.ExtraCacheFrom...) + } opts.bus.Emit(events.BuildStartEvent{Source: events.BuildSourceDockerfile, Ref: tag}) _, err := e.runtime.BuildImage(ctx, runtime.BuildSpec{ ContextPath: s.Context, @@ -867,7 +906,9 @@ func (e *Engine) prepareBaseImage(ctx context.Context, cfg *config.ResolvedConfi Tag: tag, Args: s.Args, Target: s.Target, - CacheFrom: s.CacheFrom, + CacheFrom: cacheFrom, + NoCache: noCache, + Platform: platform, }, opts.bus.BuildChan(events.BuildSourceDockerfile)) if err != nil { return "", fmt.Errorf("build base image from %s: %w", s.Context, err) @@ -992,6 +1033,19 @@ func (e *Engine) layerFeatures(ctx context.Context, cfg *config.ResolvedConfig, } finalTag := "dc-go-final-" + cfg.DevcontainerID + ":latest" + var ( + noCache bool + platform string + cacheFrom []string + ) + if opts.override != nil { + if !opts.override.BaseIsFinal && opts.override.FinalTag != "" { + finalTag = opts.override.FinalTag + } + noCache = opts.override.NoCache + platform = opts.override.Platform + cacheFrom = append([]string(nil), opts.override.ExtraCacheFrom...) + } opts.bus.Emit(events.BuildStartEvent{Source: events.BuildSourceFeatures, Ref: finalTag}) _, err = e.runtime.BuildImage(ctx, runtime.BuildSpec{ ContextPath: tmp, @@ -1000,6 +1054,9 @@ func (e *Engine) layerFeatures(ctx context.Context, cfg *config.ResolvedConfig, Args: map[string]string{ "_DEV_CONTAINERS_BASE_IMAGE": baseImage, }, + CacheFrom: cacheFrom, + NoCache: noCache, + Platform: platform, }, opts.bus.BuildChan(events.BuildSourceFeatures)) if err != nil { return "", nil, fmt.Errorf("build feature-extended image: %w", err) From 43ff0746247074d193c0dd259c3d0ed3e02b8106 Mon Sep 17 00:00:00 2001 From: bilby91 Date: Fri, 22 May 2026 20:45:22 -0300 Subject: [PATCH 2/3] =?UTF-8?q?engine:=20address=20PR=20#82=20review=20?= =?UTF-8?q?=E2=80=94=20typed=20compose=20error=20+=20ImageName=20retag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught two real issues: 1. Compose-source refusal used a plain string error. Replace with a package-level sentinel ErrComposeSourceUnsupported wrapped via %w, so callers can match with errors.Is. Update the integration test to assert via errors.Is. 2. ImageName was lost when the feature pipeline short-circuited — features declared but every one already pre-baked into the base image. layerFeatures returns baseImage unchanged in that case, but BaseIsFinal was false (because len(features) > 0), so the base build kept its auto tag. Same gap surfaced as the documented "ignored for pure image source" caveat. Fix: after layerFeatures, if finalImage == baseImage and the caller asked for a specific ImageName, run a thin `FROM ` Dockerfile build via a new retagImage helper. Covers both the pure-image-source case and the features-all-pre-baked case. The "ImageName ignored" caveat goes away. This is a workaround for the absent runtime.TagImage primitive (still tracked as a follow-up); a TagImage would let us avoid the extra build. Tests updated: - TestEngineBuild_ImageSource_NoFeatures_NoImageName — unchanged path - TestEngineBuild_ImageSource_RetagToImageName — new, exercises the retag step - TestEngineBuild_ComposeRefused — now asserts errors.Is rather than substring matching Co-Authored-By: Claude Opus 4.7 (1M context) --- build.go | 87 ++++++++++++++++++++++++++++++---- test/integration/build_test.go | 51 +++++++++++++++----- 2 files changed, 118 insertions(+), 20 deletions(-) diff --git a/build.go b/build.go index f692dbe..7e7900b 100644 --- a/build.go +++ b/build.go @@ -2,12 +2,22 @@ package devcontainer import ( "context" + "errors" "fmt" + "os" + "path/filepath" "github.com/crunchloop/devcontainer/config" "github.com/crunchloop/devcontainer/events" + "github.com/crunchloop/devcontainer/runtime" ) +// ErrComposeSourceUnsupported is returned (wrapped) from Engine.Build +// when the resolved devcontainer.json is a compose source. Callers can +// match with errors.Is to surface a friendlier message or fall back to +// the compose workflow. +var ErrComposeSourceUnsupported = errors.New("compose-source devcontainers are not supported by Engine.Build") + // BuildOptions configures Engine.Build. type BuildOptions struct { // LocalWorkspaceFolder is the absolute host path to the project. Required. @@ -23,10 +33,14 @@ type BuildOptions struct { // ImageName, when non-empty, is the tag applied to the final // produced image. It replaces the engine's auto-generated tag - // (dc-go-final-:latest or dc-go-base-:latest). Ignored for - // pure image sources with no features layered (there is nothing - // to retag — the engine would need a TagImage primitive it - // doesn't currently expose; tracked as a follow-up). + // (dc-go-final-:latest or dc-go-base-:latest). + // + // Honored uniformly across source kinds. For paths where no build + // step would otherwise carry the tag — pure image source, or a + // build source whose features are all already pre-baked into the + // base image — Build runs a thin `FROM ` Dockerfile + // build to apply the tag. (This workaround goes away once the + // runtime exposes a TagImage primitive; tracked as a follow-up.) ImageName string // NoCache forces --no-cache on every BuildImage call in the chain @@ -79,10 +93,11 @@ type BuildResult struct { // while UID reconciliation bakes the calling host's UID into the // image. Use Engine.Up if you need a UID-reconciled local image. // -// For pure image sources with no features, Build short-circuits: it -// just ensures the image is present locally and returns its ref. -// BuildOptions.ImageName is ignored in that case (no build step to -// retag); a TagImage primitive is tracked as a follow-up. +// When the feature pipeline produces no work (no features declared, +// or every requested feature is already present in the base image's +// metadata label), Build still honors BuildOptions.ImageName via a +// thin no-op `FROM ` Dockerfile build that applies the +// tag. func (e *Engine) Build(ctx context.Context, opts BuildOptions) (*BuildResult, error) { if err := ctxIfDone(ctx); err != nil { return nil, err @@ -108,7 +123,7 @@ func (e *Engine) Build(ctx context.Context, opts BuildOptions) (*BuildResult, er } if _, isCompose := cfg.Source.(*config.ComposeSource); isCompose { - return nil, fmt.Errorf("Engine.Build: compose-source devcontainers are not supported") + return nil, fmt.Errorf("Engine.Build: %w", ErrComposeSourceUnsupported) } _, isBuildSource := cfg.Source.(*config.BuildSource) @@ -139,5 +154,59 @@ func (e *Engine) Build(ctx context.Context, opts BuildOptions) (*BuildResult, er return nil, err } + // If the feature pipeline returned baseImage unchanged (no features + // declared, or every feature was already pre-baked into the base + // image's metadata) and the caller asked for a specific tag, apply + // it via a thin `FROM ` build. Until the runtime grows a + // TagImage primitive this is the only way to honor ImageName in + // these short-circuit paths. + if opts.ImageName != "" && finalImage == baseImage && finalImage != opts.ImageName { + finalImage, err = e.retagImage(ctx, baseImage, opts.ImageName, upOpts.override, bus) + if err != nil { + return nil, err + } + } + return &BuildResult{ImageID: finalImage}, nil } + +// retagImage applies `target` as an additional tag on `source` by +// running a one-line `FROM ` Dockerfile build. Used by Build +// when the feature pipeline short-circuits and the caller asked for +// a specific ImageName — there's no other tag-affecting step in the +// pipeline to carry the tag, and the runtime doesn't expose a +// dedicated tag primitive yet. +// +// Cost: roughly one BuildKit FROM-fetch + a metadata write. No +// userland layers are added. +func (e *Engine) retagImage(ctx context.Context, source, target string, override *buildOverride, bus *eventBus) (string, error) { + tmp, err := os.MkdirTemp("", "dc-go-retag-*") + if err != nil { + return "", fmt.Errorf("retag: create tmpdir: %w", err) + } + defer os.RemoveAll(tmp) + + if err := os.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte("FROM "+source+"\n"), 0o644); err != nil { + return "", fmt.Errorf("retag: write Dockerfile: %w", err) + } + + var ( + platform string + cacheFrom []string + ) + if override != nil { + platform = override.Platform + cacheFrom = override.ExtraCacheFrom + } + bus.Emit(events.BuildStartEvent{Source: events.BuildSourceDockerfile, Ref: target}) + if _, err := e.runtime.BuildImage(ctx, runtime.BuildSpec{ + ContextPath: tmp, + Dockerfile: "Dockerfile", + Tag: target, + Platform: platform, + CacheFrom: cacheFrom, + }, bus.BuildChan(events.BuildSourceDockerfile)); err != nil { + return "", fmt.Errorf("retag %s as %s: %w", source, target, err) + } + return target, nil +} diff --git a/test/integration/build_test.go b/test/integration/build_test.go index 6c05366..d809f05 100644 --- a/test/integration/build_test.go +++ b/test/integration/build_test.go @@ -4,19 +4,18 @@ package integration import ( "context" + "errors" "path/filepath" - "strings" "testing" "time" devcontainer "github.com/crunchloop/devcontainer" ) -// TestEngineBuild_ImageSource_NoFeatures exercises the short-circuit: -// pure image source with no features should pull the image and return -// its ref unchanged, with ImageName ignored (there's nothing to retag -// — a TagImage primitive is a tracked follow-up). -func TestEngineBuild_ImageSource_NoFeatures(t *testing.T) { +// TestEngineBuild_ImageSource_NoFeatures_NoImageName exercises the +// pure short-circuit path: pure image source, no features, no +// ImageName. Build should pull the image and return its ref unchanged. +func TestEngineBuild_ImageSource_NoFeatures_NoImageName(t *testing.T) { if testing.Short() { t.Skip() } @@ -29,16 +28,46 @@ func TestEngineBuild_ImageSource_NoFeatures(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() + res, err := eng.Build(ctx, devcontainer.BuildOptions{LocalWorkspaceFolder: ws}) + if err != nil { + t.Fatalf("Build: %v", err) + } + if res.ImageID != testImage { + t.Errorf("ImageID = %q, want %q", res.ImageID, testImage) + } +} + +// TestEngineBuild_ImageSource_RetagToImageName: pure image source + +// ImageName should run a thin retag build so the returned ImageID +// equals the requested tag. +func TestEngineBuild_ImageSource_RetagToImageName(t *testing.T) { + if testing.Short() { + t.Skip() + } + + eng, rt := newEngine(t) + defer rt.Close() + + ws := writeWorkspace(t, `{"image": "`+testImage+`"}`) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + + const tag = "dc-go-build-test-retag-image-source:v1" res, err := eng.Build(ctx, devcontainer.BuildOptions{ LocalWorkspaceFolder: ws, - ImageName: "ignored-because-no-build-step:latest", + ImageName: tag, }) if err != nil { t.Fatalf("Build: %v", err) } - if res.ImageID != testImage { - t.Errorf("ImageID = %q, want %q (ImageName should be ignored when no build step exists)", res.ImageID, testImage) + if res.ImageID != tag { + t.Errorf("ImageID = %q, want %q", res.ImageID, tag) + } + if details, err := rt.InspectImage(ctx, tag); err != nil || details == nil { + t.Errorf("InspectImage(%s): details=%v err=%v", tag, details, err) } + t.Cleanup(func() { _ = rt.RemoveImage(context.Background(), tag) }) } // TestEngineBuild_BuildSource_WithFeature is the canonical happy path: @@ -143,8 +172,8 @@ func TestEngineBuild_ComposeRefused(t *testing.T) { if err == nil { t.Fatal("Build accepted a compose source; expected refusal") } - if !strings.Contains(err.Error(), "compose") { - t.Errorf("error %q should mention compose", err.Error()) + if !errors.Is(err, devcontainer.ErrComposeSourceUnsupported) { + t.Errorf("error %v does not match ErrComposeSourceUnsupported", err) } } From c7fb2cd9ce60ceb7cb8b225d73c4827493dba519 Mon Sep 17 00:00:00 2001 From: bilby91 Date: Fri, 22 May 2026 21:02:01 -0300 Subject: [PATCH 3/3] engine: propagate NoCache through retagImage CodeRabbit catch on PR #82: retagImage read Platform and ExtraCacheFrom from buildOverride but dropped NoCache, so Engine.Build(..., NoCache: true) could still reuse a cached FROM-only layer. Co-Authored-By: Claude Opus 4.7 (1M context) --- build.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build.go b/build.go index 7e7900b..947f735 100644 --- a/build.go +++ b/build.go @@ -191,10 +191,12 @@ func (e *Engine) retagImage(ctx context.Context, source, target string, override } var ( + noCache bool platform string cacheFrom []string ) if override != nil { + noCache = override.NoCache platform = override.Platform cacheFrom = override.ExtraCacheFrom } @@ -205,6 +207,7 @@ func (e *Engine) retagImage(ctx context.Context, source, target string, override Tag: target, Platform: platform, CacheFrom: cacheFrom, + NoCache: noCache, }, bus.BuildChan(events.BuildSourceDockerfile)); err != nil { return "", fmt.Errorf("retag %s as %s: %w", source, target, err) }