Skip to content
This repository has been archived by the owner on Aug 14, 2020. It is now read-only.

lib: allow relative paths in ImageManifest's App.Exec #114

Merged
merged 1 commit into from
Jan 25, 2016

Conversation

iaguis
Copy link
Member

@iaguis iaguis commented Jan 25, 2016

Since appc/spec#527 the spec allows relative
paths in App.Exec, so we don't need to prepend "/bin/sh -c".

@alban
Copy link
Member

alban commented Jan 25, 2016

LGTM if it works.

@@ -440,18 +440,10 @@ func writeACI(layer io.ReadSeeker, manifest schema.ImageManifest, curPwl []strin
}

func getExecCommand(entrypoint []string, cmd []string) appctypes.Exec {
var command []string
if entrypoint == nil && cmd == nil {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I pushed an old version, this is not needed. If both entrypoint and cmd are nil, append(entrypoint, cmd...) returns nil already.

Since appc/spec#527 the spec allows relative
paths in App.Exec, so we don't need to prepend "/bin/sh -c".
@alban
Copy link
Member

alban commented Jan 25, 2016

ok, LGTM as well.

@iaguis
Copy link
Member Author

iaguis commented Jan 25, 2016

Tested with busybox (it uses a relative path):

$ docker2aci docker://busybox
Downloading sha256:eeee0535bf3: [==============================] 676 KB/676 KB 
Downloading sha256:a3ed95caeb0: [==============================] 32 B/32 B

Generated ACI(s):
library-busybox-latest.aci
$ actool cat-manifest --pretty-print library-busybox-latest.aci
{
    "acKind": "ImageManifest",
    "acVersion": "0.7.0",
    "name": "registry-1.docker.io/library/busybox",
    "labels": [
        {
            "name": "version",
            "value": "latest"
        },
        {
            "name": "os",
            "value": "linux"
        },
        {
            "name": "arch",
            "value": "amd64"
        }
    ],
    "app": {
        "exec": [
            "sh"
        ],
        "user": "0",
        "group": "0"
    },
    "annotations": [
        {
            "name": "created",
            "value": "2016-01-15T18:06:41Z"
        },
        {
            "name": "appc.io/docker/registryurl",
            "value": "registry-1.docker.io"
        },
        {
            "name": "appc.io/docker/repository",
            "value": "library/busybox"
        },
        {
            "name": "appc.io/docker/imageid",
            "value": "964092b7f3e54185d3f425880be0b022bfc9a706701390e0ceab527c84dea3e3"
        },
        {
            "name": "appc.io/docker/parentimageid",
            "value": "9e77fef7a1c9f989988c06620dabc4020c607885b959a2cbd7c2283c91da3e33"
        }
    ]
}
$ sudo rkt --insecure-options=image run --interactive library-busybox-latest.aci
rkt: using image from file /home/iaguis/work/coreos/go/src/github.com/coreos/rkt/build-rkt/bin/stage1-coreos.aci
rkt: using image from file library-busybox-latest.aci
/ # exit

iaguis added a commit that referenced this pull request Jan 25, 2016
lib: allow relative paths in ImageManifest's App.Exec
@iaguis iaguis merged commit 603cdf9 into appc:master Jan 25, 2016
@@ -440,18 +440,7 @@ func writeACI(layer io.ReadSeeker, manifest schema.ImageManifest, curPwl []strin
}

func getExecCommand(entrypoint []string, cmd []string) appctypes.Exec {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just inline this now..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants