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: add call methods support and printing #2556

Merged
merged 7 commits into from
Jul 3, 2024

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Jun 26, 2024

Define call method in the bake target

target "validate" {
	call = "check"
}
docker buildx bake validate

Set call/check on all active targets

docker buildx bake --call=outline
docker buildx bake --check
docker buildx bake --call check,format=json

The text output of call=outline should be updated in follow-up to include bake definition and variables. Getting variables seems tricky and I think needs HCL parser updates.

» docker buildx bake binaries test --call=outline 
[+] Building 1.0s (11/11) FINISHED                                                                                                                                           0.0s
binaries

TARGET: binaries

BUILD ARG                  VALUE   DESCRIPTION
GO_VERSION                 1.21
XX_VERSION                 1.4.0
GO_EXTRA_FLAGS
BUILDKIT_SBOM_SCAN_STAGE   true


test

TARGET: test-coverage

BUILD ARG    VALUE   DESCRIPTION
GO_VERSION   1.21
XX_VERSION   1.4.0

JSON output contains both Bake definition and the method result.

List all bake targets

docker buildx bake --list-targets
TARGET				DESCRIPTION
binaries
binaries-cross
default				binaries
image
image-cross
image-local
integration-test
integration-test-base
lint
lint-gopls			Run gopls analyzers
meta-helper

List all bake variables

docker buildx bake --list-variables
VARIABLE			VALUE	DESCRIPTION
DESTDIR				./bin	Destination directory for build artifacts
DOCS_FORMATS			md
GOLANGCI_LINT_MULTIPLATFORM
GO_VERSION			<null>
HTTPS_PROXY
HTTP_PROXY
NO_PROXY
TEST_BUILDKIT_TAG		<null>
TEST_COVERAGE			<null>

@dvdksn
Copy link
Contributor

dvdksn commented Jul 1, 2024

Should call be a block type in the bake file when defining subfields? And should it be possible to set control directives here?

# Short form
target "default" {
  call = "check"
}

# Long form
target "default" {
  call = {
    method = "check"
    format = "json"
    error = true # ignorestatus / BUILDKIT_DOCKERFILE_CHECK=error=true
    skip = [ "NoEmptyContinuation", "StageNameCasing" ] # BUILDKIT_DOCKERFILE_CHECK=skip=StageNameCasing,NoEmptyContinuations
  }
}

@crazy-max crazy-max linked an issue Jul 1, 2024 that may be closed by this pull request
@crazy-max
Copy link
Member

Should call be a block type in the bake file when defining subfields?

Yes in the future we would like objects instead of csv values in HCL. This is tracked in #438

commands/bake.go Outdated
Comment on lines 442 to 447
flags.BoolVar(&options.listTargets, "list-targets", false, "List available targets")
flags.BoolVar(&options.listVars, "list-variables", false, "List defined variables")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we open another PR for these flags? I don't think this is the same use case as call related to #1072

Also I think it should be a single flag --list defaulting to targets as value but can also be variables? I'm looking at https://github.com/casey/just#command-line-options

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could consider using existing --print flag so we could pass --print=targets or --print=variables and --print would default to --print=definition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is possible to use --print as both bool and string flag.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we have a clear use case, I would be keen not to overcomplicate things at this point. We can always make additional updates in the future

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

list-targets and list-variables have been marked as experimental in the meantime.

@tonistiigi
Copy link
Member Author

Should call be a block type in the bake file when defining subfields? And should it be possible to set control directives here?

format=json is for the whole output atm, so it can be set with --call but in bake definition it is just a method name. Should there be multiple targets with different format, there is no way to print it as if user requests JSON we shouldn't print random text in the middle of the output.

ignorestatus and #check=error=true are not quite the same field as well. I'm not sure if ignorestatus is even worth documenting atm.

@crazy-max
Copy link
Member

crazy-max commented Jul 3, 2024

👀 https://github.com/docker/buildx/actions/runs/9781238278/job/27004845569?pr=2556#step:7:468

=== FAIL: tests TestIntegration/TestBakeCallCheckFlag/worker=docker-container (4.20s)
    bake.go:1086: 
        	Error Trace:	/src/tests/bake.go:1086
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:96
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:211
        	Error:      	Received unexpected error:
        	            	invalid character '/' after top-level value
        	Test:       	TestIntegration/TestBakeCallCheckFlag/worker=docker-container
    --- FAIL: TestIntegration/TestBakeCallCheckFlag/worker=docker-container (4.20s)

Looks flaky

@crazy-max
Copy link
Member

Will open a follow-up when #2562 is merged to add cross linking from bake to build

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonistiigi PTAL with my last commit

Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum now happens in https://github.com/docker/buildx/actions/runs/9781672942/job/27006277267?pr=2556#step:7:428

=== Failed
=== FAIL: tests TestIntegration/TestBakeCallCheckFlag/worker=docker-container (2.04s)
    bake.go:1086: 
        	Error Trace:	/src/tests/bake.go:1086
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:96
        	            				/src/vendor/github.com/moby/buildkit/util/testutil/integration/run.go:211
        	Error:      	Received unexpected error:
        	            	invalid character '/' after top-level value
        	Test:       	TestIntegration/TestBakeCallCheckFlag/worker=docker-container

@tonistiigi
Copy link
Member Author

@crazy-max Something is printing to stdout on format=json I think

tests/bake.go Outdated Show resolved Hide resolved
Copy link
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

opened #2575 to make sure we don't miss it before GA

@tonistiigi tonistiigi merged commit c51004e into docker:master Jul 3, 2024
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bake: subrequest support for printing outline/targets/lint
5 participants