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

build: fix localstate for remote context and stdin #2560

Merged
merged 2 commits into from
Jun 28, 2024

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Jun 28, 2024

relates to #1735
needs or closes #2561

When using a remote context or stdin the LocalPath and DockerfilePath are not correct:

$ docker buildx buildx https://github.com/docker/buildx.git
...
#14 exporting to image
#14 exporting layers done
#14 writing image sha256:d9015ff8b549fee9b8b8100cd168efea32015136c1a0a463dd5f696e130e72fa done
#14 DONE 0.0s

View build details: docker-desktop://dashboard/build/default/default/aav2ix4nw5eky66fw045dkylr

$ jq . ~/.docker/buildx/refs/default/default/aav2ix4nw5eky66fw045dkylr
{
  "Target": "default",
  "LocalPath": "/home/crazy/foo/bar/https:/github.com/docker/buildx.git",
  "DockerfilePath": ""
}
$ docker buildx build -f hello.Dockerfile https://github.com/docker/actions-toolkit.git#:__tests__/fixtures
...
#10 exporting to image
#10 exporting layers done
#10 writing image sha256:9deae3ebca280147d5096bd8858e2a008014cd6cc2e7b33e6307f06cbf11d992 done
#10 DONE 0.0s

View build details: docker-desktop://dashboard/build/default/default/7pnnqpgacnqq98oa1a1h5sz6t

$ jq . ~/.docker/buildx/refs/default/default/7pnnqpgacnqq98oa1a1h5sz6t
{
  "Target": "default",
  "LocalPath": "/home/crazy/foo/bar/https:/github.com/docker/actions-toolkit.git#:__tests__/fixtures",
  "DockerfilePath": "/home/crazy/foo/bar/hello.Dockerfile"
}
$ docker buildx build -f- . < hello.Dockerfile
...
#14 exporting to image
#14 exporting layers done
#14 writing image sha256:d9015ff8b549fee9b8b8100cd168efea32015136c1a0a463dd5f696e130e72fa done
#14 DONE 0.0s

View build details: docker-desktop://dashboard/build/default/default/aav2ix4nw5eky66fw045dkylr

$ jq . ~/.docker/buildx/refs/default/default/w38vcd5fo5cfvfyig77qjec0v
{
  "Target": "default",
  "LocalPath": "/home/crazy/hello",
  "DockerfilePath": "/home/crazy/hello/-"
}

With this change we set LocalPath and DockerfilePath accordingly if a remote context is used or stdin.

Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max force-pushed the fix-localstate-remote branch 6 times, most recently from 06bb273 to 42f1086 Compare June 28, 2024 09:50
@crazy-max
Copy link
Member Author

crazy-max commented Jun 28, 2024

When checking Inputs.ContextPath, I found that it is not the same when building through controller:

$ BUILDX_EXPERIMENTAL=1 docker buildx build -f hello.Dockerfile .
...
#10 exporting to image
#10 exporting layers done
#10 writing image sha256:9deae3ebca280147d5096bd8858e2a008014cd6cc2e7b33e6307f06cbf11d992 done
#10 DONE 0.0s

View build details: docker-desktop://dashboard/build/default/default/nb6ivxlwkacfvnxach0m1vg9g

$ jq . ~/.docker/buildx/refs/default/default/nb6ivxlwkacfvnxach0m1vg9g
{
  "ContextPath": "/home/crazy/hello",
  "Target": "default",
  "LocalPath": "/home/crazy/hello",
  "DockerfilePath": "/home/crazy/hello/hello.Dockerfile"
}

Without controller:

$ docker buildx build -f hello.Dockerfile .
...
#11 exporting to image
#11 exporting layers done
#11 writing image sha256:9deae3ebca280147d5096bd8858e2a008014cd6cc2e7b33e6307f06cbf11d992 done
#11 DONE 0.0s

View build details: docker-desktop://dashboard/build/default/default/szmtd9nf4urns5872py50l9pm

$ jq . ~/.docker/buildx/refs/default/default/szmtd9nf4urns5872py50l9pm
{
  "ContextPath": ".",
  "Target": "default",
  "LocalPath": "/home/crazy/hello",
  "DockerfilePath": "/home/crazy/hello/hello.Dockerfile"
}

Seems related to

options.ContextPath, err = filepath.Abs(options.ContextPath)

@crazy-max crazy-max force-pushed the fix-localstate-remote branch 3 times, most recently from fae21af to a106f33 Compare June 28, 2024 12:51
@crazy-max crazy-max changed the title build: fix localstate for remote context build: fix localstate for remote context and stdin Jun 28, 2024
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max
Copy link
Member Author

Added integration tests

@crazy-max crazy-max marked this pull request as ready for review June 28, 2024 13:08
@tonistiigi tonistiigi merged commit 63eb73d into docker:master Jun 28, 2024
103 checks passed
@crazy-max crazy-max deleted the fix-localstate-remote branch June 29, 2024 10:04
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.

2 participants