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
read bake definition files from remote context support #82
Conversation
src/context.ts
Outdated
@@ -55,6 +57,9 @@ export async function getArgs(inputs: Inputs, buildxVersion: string): Promise<Ar | |||
|
|||
async function getBakeArgs(inputs: Inputs, buildxVersion: string): Promise<Array<string>> { | |||
const args: Array<string> = ['bake']; | |||
if (inputs.remote) { | |||
args.push(inputs.remote); |
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.
Do we need special logic for this if no other opts are specified?
i.e. what happens if we have {inputs.remote} {inputs.targets...}
?
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.
It's fine as remote bake def arg is stripped out if detected: https://github.com/docker/buildx/blob/3cf549a7f7ee015cabe99d4a08e4b9aabfc83ccc/commands/bake.go#L45
README.md
Outdated
| `load` | Bool | Load is a shorthand for `--set=*.output=type=docker` (default `false`) | | ||
| `push` | Bool | Push is a shorthand for `--set=*.output=type=registry` (default `false`) | | ||
| `set` | List | List of [targets values to override](https://github.com/docker/buildx/blob/master/docs/reference/buildx_bake.md#set) (eg: `targetpattern.key=value`) | | ||
| `remote` | String | Read bake definition files from [remote context](https://github.com/docker/buildx/blob/master/docs/guides/bake/file-definition.md#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.
It seems like multiple remotes are allowed? If so, should this be a List
instead?
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.
Only the first arg is checked as remote atm: https://github.com/docker/buildx/blob/3cf549a7f7ee015cabe99d4a08e4b9aabfc83ccc/commands/bake.go#L43
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.
Oh actually you're right we can define remote bake files and remote context with a 2nd arg that will be used if BAKE_CMD_CONTEXT
is defined: https://github.com/docker/buildx/blob/3cf549a7f7ee015cabe99d4a08e4b9aabfc83ccc/commands/bake.go#L46-L50
We call this "remote" internally but not sure if this is the best name for the users. Even something like "source", although overused, might be better. |
Yes was not sure myself about its name. Maybe I can just use |
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
renamed |
Signed-off-by: CrazyMax crazy-max@users.noreply.github.com