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

Add subdirectory for Git context #531

Merged
merged 1 commit into from Jan 12, 2022

Conversation

BeyondEvil
Copy link
Contributor

@BeyondEvil BeyondEvil commented Dec 28, 2021

Since v0.9.0 of BuildKit (BuildX v0.7.0) you can provide a subdirectory
to the default Git context.

Closes #460
Closes #528

@BeyondEvil BeyondEvil requested a review from crazy-max as a code owner Dec 28, 2021
src/context.ts Outdated
@@ -97,7 +97,11 @@ export async function getArgs(inputs: Inputs, defaultContext: string, buildxVers
let args: Array<string> = ['buildx'];
args.push.apply(args, await getBuildArgs(inputs, defaultContext, buildxVersion));
args.push.apply(args, await getCommonArgs(inputs, buildxVersion));
if (inputs.context.startsWith('{{defaultContext}}') && buildx.satisfies(buildxVersion, '>=0.7.0')) {
Copy link
Contributor Author

@BeyondEvil BeyondEvil Dec 28, 2021

Choose a reason for hiding this comment

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

Should I add any type of error-handling here?

As it stands, if the version is <0.7.0 it's going to set the context to (literal) {{defaultContext}}:docker.

Copy link
Member

@crazy-max crazy-max Jan 5, 2022

Choose a reason for hiding this comment

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

I don't think we have to check buildx version here.

Copy link
Contributor Author

@BeyondEvil BeyondEvil Jan 5, 2022

Choose a reason for hiding this comment

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

Because the version of build-push-action is tied to the BuildX version?

Copy link
Member

@crazy-max crazy-max Jan 5, 2022

Choose a reason for hiding this comment

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

No because it only affects buildkit not buildx. Also we need to vendor buildkit on moby moby/moby#42968 otherwise it will not work with the docker driver.

@BeyondEvil BeyondEvil force-pushed the subdir-with-default-context branch from 8f6d7e3 to 3dbe9f9 Compare Dec 28, 2021
@BeyondEvil
Copy link
Contributor Author

@BeyondEvil BeyondEvil commented Dec 28, 2021

Locally, test and pre-checkin passes, but not validate.

 => ERROR [build-validate build-validate 1/1] RUN --mount=type=bind,target=.,rw <<EOT (set -e...)                                                                                                                                                                                                                                           0.5s
------
 > [build-validate build-validate 1/1] RUN --mount=type=bind,target=.,rw <<EOT (set -e...):
#25 0.383 ERROR: Build result differs. Please build first with "docker buildx bake build"
#25 0.390 M  dist/index.js
------
error: failed to solve: executor failed running [/bin/sh -c set -e
git add -A
cp -rf /out/* .
if [ -n "$(git status --porcelain -- dist)" ]; then
  echo >&2 'ERROR: Build result differs. Please build first with "docker buildx bake build"'
  git status --porcelain -- dist
  exit 1
fi
]: exit code: 1

Not sure how to fix it. Fixed it. 😊

@BeyondEvil BeyondEvil force-pushed the subdir-with-default-context branch from 3dbe9f9 to bc719c4 Compare Dec 28, 2021
src/context.ts Outdated
@@ -97,7 +97,11 @@ export async function getArgs(inputs: Inputs, defaultContext: string, buildxVers
let args: Array<string> = ['buildx'];
args.push.apply(args, await getBuildArgs(inputs, defaultContext, buildxVersion));
args.push.apply(args, await getCommonArgs(inputs, buildxVersion));
if (inputs.context.startsWith('{{defaultContext}}') && buildx.satisfies(buildxVersion, '>=0.7.0')) {
inputs.context = inputs.context.replace('{{defaultContext}}', defaultContext);
Copy link
Member

@crazy-max crazy-max Jan 5, 2022

Choose a reason for hiding this comment

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

I think it would be better to use handlebars template like it was done in our metadata action.

Copy link
Contributor Author

@BeyondEvil BeyondEvil Jan 5, 2022

Choose a reason for hiding this comment

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

Updated @crazy-max 👍

@BeyondEvil BeyondEvil force-pushed the subdir-with-default-context branch from 64af63c to 6cd5c4d Compare Jan 5, 2022
README.md Outdated
@@ -43,6 +43,19 @@ By default, this action uses the [Git context](#git-context) so you don't need t
done directly by buildkit. The git reference will be based on the [event that triggered your workflow](https://docs.github.com/en/actions/reference/events-that-trigger-workflows)
and will result in the following context: `https://github.com/<owner>/<repo>.git#<ref>`.

Beginning with BuildKit version `v0.9.0` ([Buildx](https://github.com/docker/buildx) `v0.7.0`) you can provide a subdirectory to the [Git context](#git-context) by using the magic variable `{{defaultContext}}`:
Copy link
Contributor Author

@BeyondEvil BeyondEvil Jan 5, 2022

Choose a reason for hiding this comment

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

Should I replace magic variable with handlebar template?

Copy link
Member

@crazy-max crazy-max Jan 5, 2022

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@BeyondEvil BeyondEvil Jan 6, 2022

Choose a reason for hiding this comment

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

Updated @crazy-max 👍

Since v0.9.0 of BuildKit (BuildX v0.7.0) you can provide a subdirectory
to the default Git context.

Closes docker#460
Closes docker#528

Signed-off-by: Jim Brännlund <jimbrannlund@fastmail.com>
@BeyondEvil BeyondEvil force-pushed the subdir-with-default-context branch from 16858fe to fc5a732 Compare Jan 6, 2022
Copy link
Member

@crazy-max crazy-max left a comment

LGTM

@crazy-max crazy-max merged commit 1814d3d into docker:master Jan 12, 2022
26 checks passed
@BeyondEvil BeyondEvil deleted the subdir-with-default-context branch Jan 12, 2022
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.

2 participants