Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bake should not skip compose model normalization #1803

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented May 16, 2023

closes #906

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@crazy-max
Copy link
Member

Looks like this test fails: https://github.com/docker/buildx/actions/runs/4989923431/jobs/8934490857?pr=1803#step:4:581

#17 67.71 === CONT  TestReadTargetsWithDotCompose
#17 67.71     bake_test.go:366: 
#17 67.71         	Error Trace:	/src/bake/bake_test.go:366
#17 67.71         	Error:      	Not equal: 
#17 67.71         	            	expected: "Dockerfile.webapp"
#17 67.71         	            	actual  : "Dockerfile"
#17 67.71         	            	
#17 67.71         	            	Diff:
#17 67.71         	            	--- Expected
#17 67.71         	            	+++ Actual
#17 67.71         	            	@@ -1 +1 @@
#17 67.71         	            	-Dockerfile.webapp
#17 67.71         	            	+Dockerfile
#17 67.71         	Test:       	TestReadTargetsWithDotCompose

I recall we needed to skip normalization to be able to merge compose files otherwise normalization overrides some fields.

@ndeloof
Copy link
Contributor Author

ndeloof commented May 16, 2023

@crazy-max looking at failing test I guess this is an issue we need to fix on Compose as well

@ndeloof
Copy link
Contributor Author

ndeloof commented May 16, 2023

I'm not sure I understand the fix covered by TestReadTargetsWithDotCompose. sanitizeTargetName to convert web.app with web_app then merge targets with same resulting name, while those were distinct in compose

@crazy-max crazy-max self-assigned this Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

buildx bake complains about undefined default network
2 participants