-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix bug that cann't recognize ONBUILD in schemaV1 img #3053
Conversation
e356c83
to
2c7ed2c
Compare
e924d18
to
9b74216
Compare
@@ -30,6 +30,7 @@ import ( | |||
"github.com/moby/buildkit/util/apicaps" | |||
binfotypes "github.com/moby/buildkit/util/buildinfo/types" | |||
"github.com/moby/buildkit/util/gitutil" | |||
"github.com/moby/buildkit/util/imageutil" |
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.
imageutil
imports many containerd libraries and isn't ideal to be imported here. Move the image types to a separate package that is imported both by dockerfile and imageutil.
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.
@tonistiigi thanks for your review, I have move the Image definition to a new package under util/schema1
, PTALA.
9b74216
to
ee7fa5b
Compare
@@ -30,6 +30,7 @@ import ( | |||
"github.com/moby/buildkit/util/apicaps" | |||
binfotypes "github.com/moby/buildkit/util/buildinfo/types" | |||
"github.com/moby/buildkit/util/gitutil" | |||
"github.com/moby/buildkit/util/schema1" |
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.
This isn't technically schema1
. Schema1 is that V1Compatibility
mess. Put it under exporter/containerimage/image
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.
According to your suggestion, move Image
definition to package exporter/containerimage/image
, PTALA.
fix bug moby#3052 Original `convertSchema1ConfigMeta` fuction using the wrong `ocispec.Image` type to unmarshal schemaV1 image config, Which miss ONBUILD field, the correct `Image` type was defined in `frontend/dockerfile/dockerfile2llb/image.go`, I move its definition to `exporter/containerimage/image`, then both package `util/imageutil` and `frontend/dockerfile` could use the correct `Image` type. Signed-off-by: frank yang <yyb196@gmail.com>
ee7fa5b
to
9dd3dfd
Compare
fix bug #3052
Original
convertSchema1ConfigMeta
fuction usingthe wrong
ocispec.Image
type to unmarshal schemaV1image config, Which miss OnBbuild field, the correct
Image
type was defined infrontend/dockerfile/dockerfile2llb/image.go
,I move its definition to
exporter/containerimage/image
, then bothpackage
util/imageutil
andfrontend/dockerfile
coulduse the correct
Image
type.Signed-off-by: frank yang yyb196@gmail.com