vendor: github.com/moby/buildkit v0.14.0-rc2-dev#47683
vendor: github.com/moby/buildkit v0.14.0-rc2-dev#47683thaJeztah merged 5 commits intomoby:masterfrom
Conversation
vvoland
force-pushed
the
buildkit-update
branch
3 times, most recently
from
6467cc5 to
b51705c
Compare
|
Looks like we forgot a compat check for https://github.com/moby/moby/actions/runs/8830465079/job/24245262993?pr=47683#step:12:2959 |
vvoland
force-pushed
the
buildkit-update
branch
2 times, most recently
from
d935fec to
c3a6c6e
Compare
|
Rebased with the latest master. Looks like moby/buildkit@228e250 did some major refactoring of the @jsternberg PTAL |
vvoland
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
- full diff: moby/buildkit@v0.13.1...v0.14.0-rc2 Signed-off-by: Paweł Gronowski <[email protected]>
eea0b41bf4fb1d69e109ff5ff8045c63f0c0d510 added a new argument to `instructions.Parse` to support issuing linter warnings. Classic builder uses it to parse the Dockerfile instructions and its usage needs adjustment. The classic builder is deprecated and we won't be adding any new features to it, so we just pass a nil linter callback. Signed-off-by: Paweł Gronowski <[email protected]>
1b1c5bc08ad81add007eb647e66ed0929693f3a0 extended the function signature with one additional return value. Signed-off-by: Paweł Gronowski <[email protected]>
Signed-off-by: Jonathan A. Sternberg <[email protected]> Signed-off-by: Paweł Gronowski <[email protected]>
b5c50afa882e2b34aba880fd5028615e2ef94e07 changed the signature of NewGatewayFrontend to include a slice of allowed repositories. Docker does not allow to specify this option, so don't place any restrictions by passing an empty slice. Signed-off-by: Paweł Gronowski <[email protected]>
vvoland
changed the title
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
Should this one be ready for review? Looks like the upstream fix was merged
| defer b.imageSources.Unmount() | ||
|
|
||
| stages, metaArgs, err := instructions.Parse(dockerfile.AST) | ||
| stages, metaArgs, err := instructions.Parse(dockerfile.AST, nil) |
There was a problem hiding this comment.
Would the "correct" way be to pass a linter with SkipAll? linter.New(linter.Config{SkipAll: true}), basically a "dummy" linter?
There was a problem hiding this comment.
I think both are "correct" from BK perspective - I like the nil though as it saves an unnecessary allocation.
There was a problem hiding this comment.
Let me know if you prefer the linter.New though 👍🏻
There was a problem hiding this comment.
It's not a blocker for sure!
I'm fine either way; I'm ok with the nil - and this is still something we can change later. I was mostly pondering "what's correct here"
Also (honestly) I was hoping that linter would be an interface here, but it looks to be a type. If it was an interface, then (possibly?) it would make it easier to swap implementations, and we could even have a linter for the classic builder to assist with deprecations, as well as a better way to indicate "this feature is not supported by the classic builder").
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
most local changes look trivial afaics. would love to have at least 1 person from the build team to give this a review though!
|
Thanks @crazy-max ! I'll bring this one in 👍 |
6 participants
dockerfile/parse: Fix nil dereference of a linter buildkit#4984dockerfile/linter: check for nil linter in linter functions buildkit#4996full diff: moby/buildkit@v0.13.1...v0.14.0-rc1
A draft PR to follow changes on buildkit side.
Moby integration adjustments
builder: Pass nil linter to instructions.Parse
eea0b41bf4fb1d69e109ff5ff8045c63f0c0d510 added a new argument to
instructions.Parseto support issuing linter warnings.Classic builder uses it to parse the Dockerfile instructions and its
usage needs adjustment.
The classic builder is deprecated and we won't be adding any new
features to it, so we just pass a nil linter callback.
builder: Adjust usage of shlex.ProcessWord
1b1c5bc08ad81add007eb647e66ed0929693f3a0 extended the function signature
with one additional return value.
builder: Update detect usage for new detect API from buildkit
builder-next: Adjust NewGatewayFrontend invocation
b5c50afa882e2b34aba880fd5028615e2ef94e07 changed the signature of
NewGatewayFrontend to include a slice of allowed repositories.
Docker does not allow to specify this option, so don't place any
restrictions by passing an empty slice.
Signed-off-by: Paweł Gronowski [email protected]