build: respect dependency order for classic builder

When using the "classic" (non-BuildKit) builder, ensure that
services are iterated in dependency order for a build so that
it's possible to guarantee the presence of a base image that's
been added as a dependency with `depends_on`. This is a very
common pattern when using base images with Compose.

A fix for BuildKit is blocked currently until we can rely on a
newer version of the engine (see docker#9324)[^1].

[^1]: docker#9232 (comment)

Signed-off-by: Milas Bowman <[email protected]>

Author: milas

pkg/compose/build.go

@@ -205,7 +205,7 @@ func (s *composeService) doBuild(ctx context.Context, project *types.Project, op
 		return nil, nil
 	}
 	if buildkitEnabled, err := s.dockerCli.BuildKitEnabled(); err != nil || !buildkitEnabled {
-		return s.doBuildClassic(ctx, opts)
+		return s.doBuildClassic(ctx, project, opts)
 	}
 	return s.doBuildBuildkit(ctx, project, opts, mode)
 }

pkg/compose/build_classic.go

@@ -27,6 +27,7 @@ import (
 	"runtime"
 	"strings"
 
+	"github.com/compose-spec/compose-go/types"
 	buildx "github.com/docker/buildx/build"
 	"github.com/docker/cli/cli/command/image/build"
 	dockertypes "github.com/docker/docker/api/types"
@@ -41,15 +42,24 @@ import (
 	"github.com/pkg/errors"
 )
 
-func (s *composeService) doBuildClassic(ctx context.Context, opts map[string]buildx.Options) (map[string]string, error) {
+func (s *composeService) doBuildClassic(ctx context.Context, project *types.Project, opts map[string]buildx.Options) (map[string]string, error) {
 	var nameDigests = make(map[string]string)
 	var errs error
-	for name, o := range opts {
+	err := project.WithServices(nil, func(service types.ServiceConfig) error {
+		imageName := getImageName(service, project.Name)
+		o, ok := opts[imageName]
+		if !ok {
+			return nil
+		}
 		digest, err := s.doBuildClassicSimpleImage(ctx, o)
 		if err != nil {
 			errs = multierror.Append(errs, err).ErrorOrNil()
 		}
-		nameDigests[name] = digest
+		nameDigests[imageName] = digest
+		return nil
+	})
+	if err != nil {
+		return nil, err
 	}
 
 	return nameDigests, errs

pkg/e2e/compose_build_test.go renamed to pkg/e2e/build_test.go

@@ -201,3 +201,40 @@ func TestBuildTags(t *testing.T) {
 		res.Assert(t, icmd.Expected{Out: expectedOutput})
 	})
 }
+
+func TestBuildImageDependencies(t *testing.T) {
+	doTest := func(t *testing.T, cli *CLI) {
+		resetState := func() {
+			cli.RunDockerComposeCmd(t, "down", "--rmi=all", "-t=0")
+		}
+		resetState()
+		t.Cleanup(resetState)
+
+		// the image should NOT exist now
+		res := cli.RunDockerOrExitError(t, "image", "inspect", "build-dependencies_service")
+		res.Assert(t, icmd.Expected{
+			ExitCode: 1,
+			Err:      "Error: No such image: build-dependencies_service",
+		})
+
+		res = cli.RunDockerComposeCmd(t, "build")
+		t.Log(res.Combined())
+
+		res = cli.RunDockerCmd(t,
+			"image", "inspect", "--format={{ index .RepoTags 0 }}",
+			"build-dependencies_service")
+		res.Assert(t, icmd.Expected{Out: "build-dependencies_service:latest"})
+	}
+
+	t.Run("ClassicBuilder", func(t *testing.T) {
+		cli := NewParallelCLI(t, WithEnv(
+			"DOCKER_BUILDKIT=0",
+			"COMPOSE_FILE=./fixtures/build-dependencies/compose.yaml",
+		))
+		doTest(t, cli)
+	})
+
+	t.Run("BuildKit", func(t *testing.T) {
+		t.Skip("See https://github.com/docker/compose/issues/9232")
+	})
+}

pkg/e2e/fixtures/build-dependencies/base.dockerfile

@@ -0,0 +1,5 @@
+FROM alpine
+
+COPY hello.txt /hello.txt
+
+CMD [ "/bin/true" ]

pkg/e2e/fixtures/build-dependencies/compose.yaml

@@ -0,0 +1,12 @@
+services:
+  base:
+    image: base
+    build:
+      context: .
+      dockerfile: base.dockerfile
+  service:
+    depends_on:
+      - base
+    build:
+      context: .
+      dockerfile: service.dockerfile

pkg/e2e/fixtures/build-dependencies/hello.txt

@@ -0,0 +1 @@
+this file was copied from base -> service

pkg/e2e/fixtures/build-dependencies/service.dockerfile

@@ -0,0 +1,5 @@
+FROM alpine
+
+COPY --from=base /hello.txt /hello.txt
+
+CMD [ "cat", "/hello.txt" ]