Add platform to manifest descriptor in metadata. - #2993
Conversation
|
Doesn't this revert #1985 ? |
|
@tonistiigi As far as I can tell, there is no effect except that the descriptor returned includes the platform. |
|
|
||
| var p exptypes.Platforms | ||
| if ok { | ||
| if err := json.Unmarshal(platformsBytes, &p); err != nil { |
There was a problem hiding this comment.
This doesn't seem to be backward compatible. The case is when new daemon(exporter) is used with an old frontend that doesn't set this key(because it is not exporting a manifest list).
There was a problem hiding this comment.
Would a length check here satisfy this concern? It's already not doing this unmarshal if the metadata is missing entirely.
There was a problem hiding this comment.
Updated to include that, also tested against some already released docker/dockerfile versions as the frontend, seems to be ok.
| return errors.Wrapf(err, "failed to marshal build info") | ||
| } | ||
|
|
||
| p := platforms.DefaultSpec() |
There was a problem hiding this comment.
Not new code but these need Normalize() calls as well.
This just make it simpler to determine the output image's platform. Today we inject this into the metadata after the build (when we can) which causes some headache in our build pipelines. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
7f9466b to
5d3648d
Compare
vendor: update buildkit to v0.20.0-rc2
2 participants
This just make it simpler to determine the output image's platform.
Today we inject this into the metadata after the build (when we can)
which causes some headache in our build pipelines.