-
Notifications
You must be signed in to change notification settings - Fork 35
add context input to replace workdir and source inputs #365
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
base: master
Are you sure you want to change the base?
Conversation
5e0257f to
07280a9
Compare
de437d7 to
f4f1492
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feedback is a tad verbose, sorry about that 😓 (I repeat some things a bit, short on time to revise)
Rough overview
I think this change is alright (with some improvements to documentation), but I'm curious if workdir input would still be relevant?
Especially if docker/buildx#1028 is resolved, I think we should take into account that changing all relative paths now (via working directory change as a workaround) will not provide the expected consistency intended from context (you fix one issue and regress by introducing another, which workdir is at least clear why).
That said, I know you've also considered to address the separate consistency concern with Docker Compose when the Bake definition is a Compose file. If users were to want similar (while others have valid reasons for avoiding that), consider what an opt-in/opt-out option might be in the docker bake CLI, so that it wouldn't conflict with the decision here.
For the most part, it seems like a context input would only influence the context target fields with a prefix (if the value is a relative path), and maybe the search path of default docker-bake.hcl (docker build doesn't use context for explicit --file paths, I assume it's the same with the docker/build-push-action).
| | `source` | String | Context to build from. Can be either local (`.`) or a [remote bake definition](https://docs.docker.com/build/bake/remote-definition/) | | ||
| | `allow` | List/CSV | Allow build to access specified resources (e.g., `network.host`) | | ||
| | `call` | String | Set method for evaluating build (e.g., check) | | ||
| | `context` | String | Context to build from. Can be either local to specify the working directory or a [remote bake definition](https://docs.docker.com/build/bake/remote-definition/) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference: build-push-action context input:
Build's context is the set of files located in the specified
PATHorURL(default Git context)
This should be better clarified for bake-action, since it's not a single build context for a Dockerfile to use? (affecting instructions like COPY/ADD)
When I use docker bake I don't need to provide context at the CLI like with docker build, that is instead handled per target (defaulting to .), similar to how both docker bake and docker build default to Dockerfile too.
Except with the working directory changed, the resolution of all relative paths would also be adjusted, there is no equivalent of docker build --file Dockerfile ./docker-context (despite the docs presently saying dockerfile target field is equivalent to --file, unless using an absolute path), only the implicit default with relative paths prepending the context target field (which would allow traversal up directories via ../).
There is no equivalent feature in the CLI to what this bake-action option is doing (but has been proposed since March 2022):
Docker Bake CLI help text
$ docker bake --help
Usage: docker buildx bake [OPTIONS] [TARGET...]
Build from a file
Aliases:
docker buildx bake, docker buildx f
Options:
--allow stringArray Allow build to access specified resources
--builder string Override the configured builder instance (default "default")
--call string Set method for evaluating build ("check", "outline", "targets") (default "build")
--check Shorthand for "--call=check"
-D, --debug Enable debug logging
-f, --file stringArray Build definition file
--list string List targets or variables
--load Shorthand for "--set=*.output=type=docker". Conditional.
--metadata-file string Write build result metadata to a file
--no-cache Do not use cache when building the image
--print Print the options without building
--progress string Set type of progress output ("auto", "none", "plain", "quiet", "rawjson", "tty"). Use plain to show container output (default "auto")
--provenance string Shorthand for "--set=*.attest=type=provenance"
--pull Always attempt to pull all referenced images
--push Shorthand for "--set=*.output=type=registry". Conditional.
--sbom string Shorthand for "--set=*.attest=type=sbom"
--set stringArray Override target value (e.g., "targetpattern.key=value")The closest workaround via the CLI is to use --set *.context=some/path/here, which replaces that context entirely for all selected targets, rather than adjusting the working directory to affect relative path resolution (aka equivalent --workdir feature).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the above and the behaviour differences of docker bake (target options delegated to bake config / --set) vs docker build CLI:
- The
workdirinput is definitely a bit of a workaround when you're only interested in adjusting thecontexttarget field, but may still have some value:- If you expected a local
outputs/cache-to/cache-fromto be relative to some other location (perhaps invoking the action multiple times, or some variable to select a different bake config). - The Bake docs also discuss the working directory in regards to remote bake definitions when using
cwd://forcontext/contexts, so it's a valid concern there too. - I haven't considered if that applies to
cwd://for the files input (which Bake docs also mention for use withmetadata-action), but this PR doesn't adjust behaviour in how the action resolves such anyway? - Does it actually make sense to drop
workdir?
- If you expected a local
- Changing
sourcetocontext, with the included functionality ofworkdirinput for local contexts (until this is better supported bydocker bakeupstream) still seems relevant (providing a UX benefit of consistency for expectations betweendocker/bake-actionanddocker/build-push-action).
Perhaps something like this? (I haven't aligned the final |):
| | `context` | String | Context to build from. Can be either local to specify the working directory or a [remote bake definition](https://docs.docker.com/build/bake/remote-definition/) | | |
| | `context` | String | Sets the Bake context which all relative paths use as the working directory. Can be assigned a local path ([Path context](#path-context)) or [remote bake definition](https://docs.docker.com/build/bake/remote-definition/) ([Git context](#git-context)) | |
I'm not sure how accurate that is with regards to cwd:// for git context, but looking at the PR it seems the context set here results in changing the working directory for git contexts too?
I think that it should also clarify that expectation of target fields context influencing the relative path of the dockerfile, but as that gets verbose for the inputs descriptions maybe introduce a "Bake Context" section header, with "Git Context" + "Path Context" sections under that?
Then this input description can just link directly to that, and describe:
- The working directory from the
contextinput affecting relative paths. - The
context+dockerfiletarget fields behaviour. - The implicit default
{{defaultContext}}(further expanded on in the "Git Context" section), and explicit Path / Git contexts. - Git context section including the remote bake definition link, which it presently doesn't.
That'd provide this terser description which is what the user cares about (_link for more details on what bake context is + what default to expect.
| | `context` | String | Context to build from. Can be either local to specify the working directory or a [remote bake definition](https://docs.docker.com/build/bake/remote-definition/) | | |
| | `context` | String | Sets the [Bake context](#bake-context). Uses [`{{ defaultContext }}`](#git-context) by default. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than that, you may want to hold off if you can resolve the relevant CLI issue in a timely manner first, considering that workdir input still seems relevant and that context should perhaps only be focused on affecting the resolution of the context target fields base path (which likewise influences the dockerfile field resolution)?
Reasoning:
docker buildCLI differs fromdocker compose build/docker bakeCLI, wheredocker/build-push-actioncan set the build context of aDockerfile(and it's default location), but override that with a relative path via--fileunaffected by the context. Docker Compose and Docker Bake don't support such AFAIK and would require absolute paths.- It seems that there is a valid expectation for the working directory to be distinct from the build context.
docker buildargs like--outputand--cache-[from|to]can use paths releative to the CWD.- This is also what Docker Compose does, but this PR's
contextinput fordocker/bake-actionwould not be consistent in the same manner as the equivalent input fordocker/build-push-action.
context input vs workdir input functionality:
- The
contexttarget field using a path relative to the Bake definition would ideally be resolved as part of docker/buildx#1028 (which will probably break existing workarounds, but become consistent withdocker compose build) - Other than addressing the
contexttarget field, this input only seems relevant for thefilesinput? (If considered similar todocker buildcontext for the defaultDockerfilefile location)
My understanding of docker/buildx#1028 is the "Bake context" expectation is to favor relative path resolution from where the Bake definition (eg: docker-bake.hcl, compose.yaml) is located (the first file if overrides are provided), but all discussion there has been focused on context / dockerfile target fields with regards to Docker Compose specifically.
- I've confirmed that the Docker Compose setting
build.cache_to(instead ofbuild.x-bake.cache-to) resolves it's relative path to the CWD just like Docker Bake. - Similarly Docker Compose treats it's build
contextfield the same way withdockerfileas Docker Bake does, so no difference there. - If
compose.yamlonly sets a path as the value tobuild, that becomes thecontextand implicitDockerfileis used at that context location. Same as with Docker Bake, just a relative path resolution difference (from file vs from CWD).
Thus it seems the only relevant difference with how Docker Compose is handling paths applicable to Docker Bake is for how the context base is adjusted.
f4f1492 to
e6c3496
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
e6c3496 to
206aa85
Compare
|
@polarathene Thank you for the detailed review, appreciate the time you put into outlining the nuances around Bake context resolution, relative paths, and the current upstream behavior. I will clarify a few core points to better explain the intent of this PR and how it fits within existing Bake capabilities and constraints. The primary motivation is to remove the long-standing ambiguity between:
Both inputs attempted to answer the same question: "Where should Bake run from?", but used different naming, different semantics, and frequently confused users. Aligning this with docker/build-push-action To avoid conflating this with the Bake "context" target field, the PR explicitly describes the input as:
This isn't redefining how upstream Bake interprets target contexts; it's unifying the entry point that the action uses before invoking Bake. You are correct that This gap is exactly why The action must provide a way to run Bake from a subdirectory: -
name: Build and push
uses: docker/bake-action@v6
with:
context: ./subdirThis mirrors the original purpose of
It would only work if every target's context is meant to be replaced, and the user also manually points -
name: Build and push
uses: docker/bake-action@v6
with:
files: ./subdir/docker-bake.hcl
set: |
*.context=./subdirThis modifies the bake definition itself, not the working directory. It doesn't provide a general mechanism for running Bake from a subdirectory. By contrast: -
name: Build and push
uses: docker/bake-action@v6
with:
context: ./subdirsimply makes the action evaluate the bake definition exactly as if the user had executed: This is the behaviour users expect today from Given the discussion above:
So the removal is intentional and simplifies the API without reducing functionality. Although I agree with your point that the documentation should explicitly distinguish:
I will update the docs accordingly, and your suggested shorter table description fits well once that section is added. |
TL;DRIt seems better to delay this PR until resolved upstream, otherwise keep The Git context is the expected behaviour for what the Pragmatically I agree with you and so long as documentation covers what to expect when using Full responseI've provided this workflow to demonstrate where Path context fails (unrelated to this PR) with the present workaround to change the working directory. I later express why a new Comparison between Bake Action (v6) and Build Push Action
name: Example
on:
workflow_dispatch:
jobs:
bake-remote:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Build with remote context (Git)
uses: docker/bake-action@v6
with:
source: "{{ defaultContext }}:bake-action"
files: ./docker-bake-x.hcl
# NOTE:
# - For Docker Compose projects it's expected that their `build.context` relative path is from the location of the config.
# - For Docker Bake configs the relative path for target `context`/`build-contexts` fields is the current working directory.
# - Thus the below settings would fail in this case for local Path Context, but for remote Git Context (as above) with a subdir
# context is managed as expected without adjusting the working directory.
#source: "{{ defaultContext }}"
#files: ./bake-action/docker-bake-x.hcl
# Likewise `workdir: ./bake-action` would only work if you do not expect the parent directory to be available for
# - Referencing files via `cwd://`
# - Other relative paths unrelated to the build context, such as `type=local` and cache import/export
#source: "{{ defaultContext }}"
#workdir: ./bake-action
#files: ./docker-bake-x.hcl
- run: ls artifacts
bake-local:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Build with local context (Path)
uses: docker/bake-action@v6
with:
# The proposed `context` input for Path context intends to unify these two inputs from v6:
source: .
workdir: ./bake-action
# Setting the `workdir` does resolve custom config filenames similar to Git context,
# by not requiring a subpath prefix, however the the `cwd` build-context will now fail.
files: ./docker-bake-x.hcl
# This will fail due to working directory change, actual location is now `./bake-action/artifacts`:
- run: ls artifacts
bpa-remote:
runs-on: ubuntu-latest
steps:
# Only required for added local `cwd` context:
- name: Checkout repository
uses: actions/checkout@v4
- name: Build with remote context (Git)
uses: docker/build-push-action@v6
with:
context: "{{ defaultContext }}:build-push-action"
# Resolved relative to the context (which must be set for it's relative COPY to work)
file: ./Dockerfile-x
# Local working directory context:
# (This action (or equivalent upstream support rather) does not understand `cwd://`; just use `.`)
build-contexts: cwd=.
outputs: ./artifacts/
- run: ls artifacts
bpa-local:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v4
- name: Build with local context (Path)
uses: docker/build-push-action@v6
with:
context: build-push-action
# Unlike with remote Git context, the Path context requires an explicit
# relative path when the filename doesn't match the default `Dockerfile`:
file: ./build-push-action/Dockerfile-x
build-contexts: cwd=.
outputs: ./artifacts/
- run: ls artifacts
target "default" {
contexts = {
cwd = "cwd://"
}
dockerfile-inline = <<-HEREDOC
FROM scratch
COPY --link ./hello.txt /
COPY --link --from=cwd README.md /
HEREDOC
output = ["./artifacts/"]
}
FROM scratch
COPY --link ./world.txt /
COPY --link --from=cwd README.md /Hopefully that illustrates how even with
EDIT: Yet pragmatically.. this kind of problem is unlikely I guess. I understand the motive to unify on a Git ContextUsing Git context demonstrates Docker Bake is capable of solving this concern without requiring a change to the working directory. It correctly applies the relative context path only to each targets build context +
|

source#352This PR introduces a new
contextinput that replaces bothworkdirandsourceinputs. It resolves long-standing confusion around how bake-action determines what directory or Git reference to use when running bake.The
workdirinput was originally added in #76 to support running Bake from a subdirectory inside the GitHub workspace.Later, Git context support was added in #181 introducing a new
sourceinput. This created overlap and ambiguity between:workdir(local path inside workspace)source(Git context but can be skipped with.)This duality has consistently caused confusion for users, as highlighted in multiple issues and discussions. Users also pointed out that naming was inconsistent with
build-push-action, where the input is calledcontext.This unified
contextinput aligns withbuild-push-action. If a local dir is detected it will use it as working directory to run Bake otherwise it will use the Git context like it does forbuild-push-action.cc @ModerNews @polarathene @Clashsoft