command(bake): Specify local and remote bake files

[c-ameron]

As I mentioned on Slack. This adds the ability to source additional local build definition files when sourcing Bake files via a remote url.
Prefixing a file with cwd:// will source the bake file from the local machine, instead of the remote location.
Local files will be read/have precedence before remote files.

Usage:

docker buildx bake https://github.com/example/upstream.git --file cwd://docker-bake.override.hcl --print

This will source a default file from the example/upstream repository, and also source a build definition from the local machine.

I'm a Go novice so any suggestions are welcome 😄

Thanks!

[tonistiigi]

The "local://" prefix is not ideal but don't have better ideas either.

Ideally, this could have an integration test under tests but let us know if you need help with that.

[jedevc]

Fun tangent: was looking into this PR, only to discover that BAKE_CMD_CONTEXT doesn't work correctly anymore: #1840.

The "local://" prefix is not ideal but don't have better ideas either.

This seems kinda similar to the BAKE_CMD_CONTEXT var actually. Internally, we have this work by doing cwd:// - we could re-use this instead of local:// which should also let us share some code?

Ideally, this could have an integration test under tests but let us know if you need help with that.

👍 yup! Our integration tests are quite new here, so any help you can give to help us build those out is very appreciated ❤️

[tonistiigi]

^ PTAL comment from @jedevc

[c-ameron]

Apologies for the late reply, I was away on holiday so will get back to this PR at the end of the week.

Our integration tests are quite new here, so any help you can give to help us build those out is very appreciated

Could you please point me in the right direction where to do this? I should do it like the TestReadLocalFilesDefault function right?

This seems kinda similar to the BAKE_CMD_CONTEXT var actually. Internally, we have this work by doing cwd:// - we could re-use this instead of local:// which should also let us share some code?

Ok thanks - so I should use the cwd:// prefix instead? I had a look through toBuildOpt but I don't see any reusable methods?

[jedevc]

Could you please point me in the right direction where to do this? I should do it like the TestReadLocalFilesDefault function right?

We have our integration tests in https://github.com/docker/buildx/blob/master/tests/bake.go, and our bake unit tests in https://github.com/docker/buildx/blob/master/bake/bake_test.go. Ideally - we'd be able to have both, though if the logic is only defined in the commands package, we can't have a unit test under our current structure really (which is absolutely fine).

Ok thanks - so I should use the cwd:// prefix instead? I had a look through toBuildOpt but I don't see any reusable methods?

Yeah, cwd:// would be preferable I think, just so we don't end up having multiple different prefixes for the same thing, referencing local directories.

It's fine if the code doesn't share nicely, this is for different purposes 🎉 Maybe it would be good to try and define the cwd:// constant somewhere though, so at least when manipulating the strings we're using common code (having different code for manipulating URLs has gotten us before 😢).

[jedevc]

Also, can you squash commits, and add a sign-off (see https://github.com/docker/buildx/blob/master/.github/CONTRIBUTING.md#sign-your-work)

[c-ameron]

Hi, apologies for the delay in getting this done. I've added an integration test as well. Let me know if you require anything else. Thanks for your time 😃

Comments were addressed

[jedevc]

@c-ameron I realized that 5638919#diff-3f9c21dce82a82fae6d5ca096e77e5ddcf0eccd7051f815b452ec26fbfeb314bR284 was incorrect, which was causing the e2e tests to fail.

I pushed a commit that should fix that up, and simplifies the splitting logic, so we don't need the isRemote argument - hopefully looks good to you.

[c-ameron]

Cool! Thanks for your help @jedevc ! 😃

[c-ameron]

Is this ok to be approved/merged @tonistiigi ? Thanks in advance! 😃

[c-ameron]

I've rebased the commit onto master as there was a conflict in commands/bake.go :)

Although I'm not sure why this test is now failing https://github.com/docker/buildx/actions/runs/6408757454/job/17398732309?pr=1838

[crazy-max]

Although I'm not sure why this test is now failing https://github.com/docker/buildx/actions/runs/6408757454/job/17398732309?pr=1838

@c-ameron Sorry for the delay. I have rebased to fix this ci issue.

I pushed an extra commit to fix some error handling when reading local/remote files and consolidate the logic in a single func as well. Feel free to squash if that looks good to you.

[c-ameron]

No worries, thanks @crazy-max . I've squashed and added you as a co-author too.