bump compose-go version to v1.17.0 to fix issue with depends_on

[glours]

@austindrenski identified that the new attribute depends_on.required isn't supported by bake, as this one is mandatory and setup by default in Compose, all the compose configuration using depends_on won't be supported by bake.

Updating compose-go version fixed the problem

A reproduction case is available in the comment linked above

[crazy-max]

Ok so after discussing it seems depends_on is now required. @glours Is it since 0.17.0? If so I think we need a test in https://github.com/docker/buildx/blob/master/bake/compose_test.go to make sure there is no regression.

Looking at docker/compose#10804 (comment), the compose file does not seem complete. Can you post a simple one to repro so we can look at this? Thanks

[crazy-max]

Looks like behavior has changed for smth else 👀 https://github.com/docker/buildx/actions/runs/5678427149/job/15388687707?pr=1971#step:6:771

=== Failed
=== FAIL: bake TestParseCompose (0.01s)
    compose_test.go:65: 
        	Error Trace:	/src/bake/compose_test.go:65
        	Error:      	Not equal: 
        	            	expected: "./db"
        	            	actual  : "/src/bake/db"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-./db
        	            	+/src/bake/db
        	Test:       	TestParseCompose

=== FAIL: bake TestReadEmptyTargets (0.02s)
    bake_test.go:558: 
        	Error Trace:	/src/bake/bake_test.go:558
        	Error:      	Not equal: 
        	            	expected: "."
        	            	actual  : "/src/bake"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-.
        	            	+/src/bake
        	Test:       	TestReadEmptyTargets

=== FAIL: bake TestReadTargetsCompose (0.11s)
    bake_test.go:306: 
        	Error Trace:	/src/bake/bake_test.go:306
        	Error:      	Not equal: 
        	            	expected: "."
        	            	actual  : "/src/bake"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-.
        	            	+/src/bake
        	Test:       	TestReadTargetsCompose

=== FAIL: bake TestReadTargetsWithDotCompose (0.13s)
    bake_test.go:367: 
        	Error Trace:	/src/bake/bake_test.go:367
        	Error:      	Not equal: 
        	            	expected: "."
        	            	actual  : "/src/bake"
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-.
        	            	+/src/bake
        	Test:       	TestReadTargetsWithDotCompose

[glours]

@crazy-max the change was introduce in version v1.17.0 of compose-go, but in version v1.16.0 the include attribute was introduce and bake doesn't support it for now.

The repro case:
Dockerfile

FROM alpine
ECHO "test"

Compose file

services:

  fetch:
    build: .
    depends_on:
    - otel

  otel:
    image: nginx

Command to reproduce
docker compose config | docker buildx bake --file - --push

[glours]

@crazy-max unit tests have been fixed, since the introduction of include, build contexts are now always absolute paths

[crazy-max]

include attribute was introduce and bake doesn't support it for now.

Can we have a test case for this?

[crazy-max]

It fixes the test but don't think it's right. We are not matching the HCL/JSON implementation anymore 😕.

[crazy-max]

Rebased and added extra test for include. Will add an integration test in follow-up for testing with remote bake def.

[crazy-max]

I was working on the integration tests and when building a remote bake definition we are just reading default files or ones provided with -f flag in

buildx/bake/remote.go

Lines 174 to 200 in 50fbdd8

	func filesFromRef(ctx context.Context, ref gwclient.Reference, names []string) ([]File, error) {
	// TODO: auto-remove parent dir in needed
	var files []File
	isDefault := false
	if len(names) == 0 {
	isDefault = true
	names = defaultFilenames()
	}
	for _, name := range names {
	_, err := ref.StatFile(ctx, gwclient.StatRequest{Path: name})
	if err != nil {
	if isDefault {
	continue
	}
	return nil, err
	}
	dt, err := ref.ReadFile(ctx, gwclient.ReadRequest{Filename: name})
	if err != nil {
	return nil, err
	}
	files = append(files, File{Name: name, Data: dt})
	}
	return files, nil
	}

I think after reading the content of the definition, we can check if it's a compose file, parse it, return the list of include, read them back and add to the list of files. We also need to handle infinite loop. loader.LoadIncludeConfig from compose-go would have been handy but it tries to resolve absolute path of the includes that are not yet been requested from the gateway. Guess we need our own logic for this.

[crazy-max]

Actually this is not just include but also env_file or any attribute that requires reading a local file.

[crazy-max]

@glours As discussed get this one in to fix the depends_on issue. For remote definition with include and env_file, I made some tests in #1992 but not ideal and probably needs changes in compose-go first so we can use our own FS handler.

[austindrenski]

@glours Recognize this patch is several supply chain steps away from docker/compose#10804, so understand if you can't commit to an ETA, but any chance you can ballpark it and/or cross-link the next step (e.g. does buildx need a patch release, does docker/setup-buildx-action, etc.)?

[furai]

Could we get this fix into new release earlier than 0.12.0 milestone?

[furai]

Actually this is preventing me from upgrading docker installation for like 2 months now. I could drop the depends_on after generating bake config from compose file with some sed command, but that's really not sustainable.

So if we're not getting update any time soon - can I somehow get a build artefact with latest state of the development or with this incompatibility patched?