-
Notifications
You must be signed in to change notification settings - Fork 1.4k
remove buildinfo #3582
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
remove buildinfo #3582
Conversation
edba81d to
70da3c8
Compare
This comment was marked as outdated.
This comment was marked as outdated.
70da3c8 to
850d894
Compare
850d894 to
ea86d1f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
9f26b07 to
363767c
Compare
| case keyOCITypes: | ||
| err = parseBoolWithDefault(&c.OCITypes, k, v, true) | ||
| case keyBuildInfo: | ||
| err = parseBoolWithDefault(&c.BuildInfo, k, v, true) |
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.
buildinfo=false should be still valid for compatibility reason
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.
By "valid" you mean should not fail when exporting?
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.
Yes
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.
It does not afaik:
$ docker buildx build --output type=docker,buildinfo=false .
...
#34 [binaries-linux 4/4] COPY --link --from=buildkitd /usr/bin/buildkitd /
#34 merging 0.1s done
#34 DONE 0.2s
#35 [buildkit-linux 1/1] COPY --link --from=binaries / /usr/bin/
#35 DONE 0.2s
#36 exporting to docker image format
#36 exporting layers
#36 exporting layers 3.7s done
#36 exporting manifest sha256:26bff5cd69b2f771dad27d23b2c62c56377d6e8b91e4528b4af0f6bc9e4504e8 0.0s done
#36 exporting config sha256:e790b681a392ef3add1e375eda8fe35885ffb6ecd1f351dba3152169f7f22fdb 0.0s done
#36 sending tarball
#36 sending tarball 1.0s done
#36 DONE 4.7s
#37 importing to docker
#37 DONE 0.7s
See
buildkit/exporter/containerimage/opts.go
Lines 72 to 74 in faaa836
| default: | |
| rest[k] = v | |
| } |
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
Signed-off-by: CrazyMax <[email protected]>
363767c to
6d9c24d
Compare
thaJeztah
left a comment
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.
SGTM, but not too familiar with this.
Do current versions print a warning if someone is using this?
|
r.e. warnings - there is no warning as of now. We could add a warning to the v0.11 release branch, but I'm not convinced it's worth it. One of the main reasons for the removal of the feature is that not really anyone uses it AFAIK. |
follow-up #3614
v0.11is released so we can remove build information implementation on master branch.