remove buildah requirement for the libpod image library #1054
Conversation
|
LGTM |
cmd/podman/commit.go
Outdated
| @@ -83,7 +84,7 @@ func commitCmd(c *cli.Context) error { | |||
| return errors.Errorf("messages are only compatible with the docker image format (-f docker)") | |||
| } | |||
| case "docker": | |||
| mimeType = buildah.Dockerv2ImageManifest | |||
| mimeType = manifest.DockerV2Schema1MediaType | |||
There was a problem hiding this comment.
This is changing the value from ...manifest.v2... to ...manifest.v1...:
$ git grep V2S2MediaTypeManifest vendor | grep 'types\.go\|Dockerv2ImageManifest'
vendor/github.com/projectatomic/buildah/docker/types.go:const V2S2MediaTypeManifest = "application/vnd.docker.distribution.manifest.v2+json"
vendor/github.com/projectatomic/buildah/image.go: Dockerv2ImageManifest = docker.V2S2MediaTypeManifest
$ git grep 'DockerV2Schema1MediaType =' vendor
vendor/github.com/containers/image/manifest/manifest.go: DockerV2Schema1MediaType = "application/vnd.docker.distribution.manifest.v1+json"I think you want to use DockerV2Schema2MediaType:
$ git grep 'application/vnd.docker.distribution.manifest.v2' vendor/github.com/containers/image/manifest
vendor/github.com/containers/image/manifest/manifest.go: DockerV2Schema2MediaType = "application/vnd.docker.distribution.manifest.v2+json"(and similarly in the other file).
There was a problem hiding this comment.
good catch ... fixed ... need caffeine
if we snip the requirement to use a buildah const in the libpod image library, we can save something on the order of 85 vendored files in consumers of the the library. Signed-off-by: baude <bbaude@redhat.com>
cmd/podman/commit.go
Outdated
| @@ -83,7 +84,7 @@ func commitCmd(c *cli.Context) error { | |||
| return errors.Errorf("messages are only compatible with the docker image format (-f docker)") | |||
| } | |||
| case "docker": | |||
| mimeType = buildah.Dockerv2ImageManifest | |||
| mimeType = manifest.DockerV2Schema1MediaType | |||
There was a problem hiding this comment.
I think you want manifest.DockerV2Schema2MediaType here.....
'''
image/manifest/manifest.go
// DockerV2Schema1MediaType MIME type represents Docker manifest schema 1
DockerV2Schema1MediaType = "application/vnd.docker.distribution.manifest.v1+json"
// DockerV2Schema2MediaType MIME type represents Docker manifest schema 2
DockerV2Schema2MediaType = "application/vnd.docker.distribution.manifest.v2+json"
Buildah side
image.go: Dockerv2ImageManifest = docker.V2S2MediaTypeManifest
./docker/types.go:const V2S2MediaTypeManifest = "application/vnd.docker.distribution.manifest.v2+json"
There was a problem hiding this comment.
yup, @wking pointed this out
|
📌 Commit 1ce187d has been approved by |
Personally, I don't have a problem with hard-coding the media type string wherever it's needed. Zero deps, and not all that many more characters in the Go source. But that is apparently not a popular approach, based on previous discussion in opencontainers/image-spec#320 and opencontainers/image-spec#414 ;). |
|
☀️ Test successful - status-papr |
|
@TomSweeneyRedHat Open a PR for buildah to use containers/image as well. |
github-actions
bot
added
the
locked - please file new issue/PR
if we snip the requirement to use a buildah const in the libpod image library,
we can save something on the order of 85 vendored files in consumers of the
the library.
Signed-off-by: baude bbaude@redhat.com