diff --git a/docs/migration.md b/docs/migration.md index 9417054..eef2e28 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -111,6 +111,13 @@ runs the official CLI is unaffected: build. `up` of a Dockerfile configuration with Features now tags the image `vsc--`, not `vsc---features`; a script that looked the image up by the old name should use the new one. +- `up` of an image configuration with Features tags the image + `vsc---features`, as the official CLI tags it, instead of + `-features` (for example + `mcr.microsoft.com/devcontainers/base:ubuntu-features`). Every workspace on the + same image shared the old tag, so concurrent `up` runs could start a container + from another workspace's image. A script that looked the image up by the old + name should use the new one, or read it from the container. - When the workspace is a Git worktree and the configuration sets both `workspaceMount` and `workspaceFolder`, `up` no longer mounts the main repository's Git common directory, and `read-configuration` no longer reports diff --git a/internal/cli/dockerfile_build_held_test.go b/internal/cli/dockerfile_build_held_test.go index dbba53a..c8cb844 100644 --- a/internal/cli/dockerfile_build_held_test.go +++ b/internal/cli/dockerfile_build_held_test.go @@ -176,6 +176,37 @@ func TestUpDockerfileBuildsOnce(t *testing.T) { } } +func TestUpImageFeaturesTagsFolderImage(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + ws := writeFixture(t, map[string]string{ + ".devcontainer/devcontainer.json": `{"image":"alpine:3","features":{"./hello":{}}}`, + ".devcontainer/hello/devcontainer-feature.json": `{"id":"hello","version":"1.0.0"}`, + ".devcontainer/hello/install.sh": "#!/bin/sh\n", + }) + loadResult, err := config.LoadDevContainerConfig(ws, "", "") + if err != nil { + t.Fatal(err) + } + r := &fakeDockerRunner{} + run := &upRunner{ + log: log.Null, + docker: fakeDockerClient(r), + engine: &docker.EngineClient{API: &fakeDockerAPI{version: "25.0.0", imageConfig: &dockerspec.DockerOCIImageConfig{}}}, + opts: &upOpts{workspaceFolder: ws, noLockfile: true, updateRemoteUserUIDDefault: "never"}, + substitute: func(*[]imagemeta.Entry) error { return errStopAfterBuild }, + } + if _, err := run.fromImage(t.Context(), loadResult.Config, loadResult, ws, nil, true); !errors.Is(err, errStopAfterBuild) { + t.Fatalf("up of an image with a Feature: %v, want it to reach the container", err) + } + builds := imageBuilds(r.calls) + if len(builds) != 1 { + t.Fatalf("docker builds = %v, want one", builds) + } + if got, want := tagsOf(builds[0]), []string{folderImageName(ws) + "-features"}; !slices.Equal(got, want) { + t.Errorf("tags = %v, want %v, as the official CLI names it", got, want) + } +} + type dockerfileBuildRun struct { files map[string]string config config.DevContainer diff --git a/internal/cli/e2e_fixes_test.go b/internal/cli/e2e_fixes_test.go index e1fb61e..aa266ea 100644 --- a/internal/cli/e2e_fixes_test.go +++ b/internal/cli/e2e_fixes_test.go @@ -11,7 +11,7 @@ import ( func writeFixture(t *testing.T, files map[string]string) string { t.Helper() - root := t.TempDir() + root := uniqueWorkspace(t) for rel, content := range files { p := filepath.Join(root, rel) if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { @@ -24,6 +24,17 @@ func writeFixture(t *testing.T, files map[string]string) string { return root } +// Concurrent runs share the Docker daemon, and a Compose project's name and +// images come from the workspace's basename. +func uniqueWorkspace(t *testing.T) string { + t.Helper() + root, err := os.MkdirTemp(t.TempDir(), "ws-") + if err != nil { + t.Fatal(err) + } + return root +} + func dockerExec(t *testing.T, id, cmd string) string { t.Helper() out, err := exec.Command("docker", "exec", id, "sh", "-c", cmd).CombinedOutput() @@ -165,11 +176,11 @@ func TestE2E_SiblingDockerignore(t *testing.T) { ".devcontainer/secret.txt": "SUPERSECRET\n", }) - buildOut := runCLI(t, "build", "--workspace-folder", ws, "--image-name", "e2e-dockerignore:test") + image := uniqueImageName(t, "e2e-dockerignore") + buildOut := runCLI(t, "build", "--workspace-folder", ws, "--image-name", image) assertOutcome(t, buildOut, "success") - defer exec.Command("docker", "rmi", "-f", "e2e-dockerignore:test").Run() - out, err := exec.Command("docker", "run", "--rm", "e2e-dockerignore:test", "cat", "/dockerignore-result").CombinedOutput() + out, err := exec.Command("docker", "run", "--rm", image, "cat", "/dockerignore-result").CombinedOutput() if err != nil { t.Fatalf("docker run failed: %v\n%s", err, out) } diff --git a/internal/cli/e2e_test.go b/internal/cli/e2e_test.go index 789bfcc..2bde3d5 100644 --- a/internal/cli/e2e_test.go +++ b/internal/cli/e2e_test.go @@ -24,6 +24,7 @@ func TestE2E_BuildUpExec_Image(t *testing.T) { if _, err := os.Stat(fixture); os.IsNotExist(err) { t.Skip("fixtures not found") } + fixture = copyFixture(t, fixture) buildOut := runCLI(t, "build", "--workspace-folder", fixture) assertOutcome(t, buildOut, "success") @@ -58,6 +59,7 @@ func TestE2E_BuildUpExec_Dockerfile(t *testing.T) { if _, err := os.Stat(fixture); os.IsNotExist(err) { t.Skip("fixtures not found") } + fixture = copyFixture(t, fixture) buildOut := runCLI(t, "build", "--workspace-folder", fixture) assertOutcome(t, buildOut, "success") @@ -82,6 +84,15 @@ func TestE2E_BuildUpExec_Dockerfile(t *testing.T) { exec.Command("docker", "rm", "-f", containerID).Run() } +func copyFixture(t *testing.T, src string) string { + t.Helper() + dst := uniqueWorkspace(t) + if err := os.CopyFS(dst, os.DirFS(src)); err != nil { + t.Fatal(err) + } + return dst +} + func TestE2E_ReadConfiguration_AllVariants(t *testing.T) { startFakeEngine(t, nil) fixtures := []struct { diff --git a/internal/cli/up.go b/internal/cli/up.go index b230f6a..330becf 100644 --- a/internal/cli/up.go +++ b/internal/cli/up.go @@ -577,7 +577,8 @@ func (r *upRunner) fromDockerfile(ctx context.Context, cfg *config.DevContainer, // mutate-lane: TestParityMatrix - up.image-success, up.image-with-features-success, // up.image-with-git-feature-success and up.update-uid-amd64-platform-option-success -// start through it; also TestE2E_BuildUpExec_Image and TestE2E_FeatureUserHomeAndLocalEnv. +// start through it; also TestUpImageFeaturesTagsFolderImage, TestE2E_BuildUpExec_Image and +// TestE2E_FeatureUserHomeAndLocalEnv. func (r *upRunner) fromImage(ctx context.Context, cfg *config.DevContainer, loadResult *config.LoadResult, workspaceFolder string, idLabels []string, useBuildx bool) (string, error) { logger, dockerClient, engine, opts := r.log, r.docker, r.engine, r.opts if cfg.Image == "" { @@ -600,7 +601,7 @@ func (r *upRunner) fromImage(ctx context.Context, cfg *config.DevContainer, load imageName := cfg.Image if len(cfg.Features) > 0 { r.lockfileRead = !opts.noLockfile - names, err := extendImageWithFeatures(ctx, logger, dockerClient, engine, cfg.Image, cfg.Features, useBuildx, nil, r.featureBuildOptions(cfg, loadResult, platform)) + names, err := extendImageWithFeatures(ctx, logger, dockerClient, engine, cfg.Image, cfg.Features, useBuildx, []string{folderImageName(workspaceFolder) + "-features"}, r.featureBuildOptions(cfg, loadResult, platform)) if err != nil { return "", fmt.Errorf("install features: %w", err) }