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. |
| @@ -57,6 +57,13 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp exporter.Source, sessionI | |||
| return nil, errors.Errorf("unable to export multiple refs, missing platforms mapping") | |||
| } | |||
|
|
|||
| 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.
| @@ -517,17 +517,22 @@ func Build(ctx context.Context, c client.Client) (*client.Result, error) { | |||
| 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>
cpuguy83
force-pushed
the
metadata_target_platform
branch
from
7f9466b
to
5d3648d
Compare
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.