thought-machine / pleasings

Addons & new build rules for Please
Apache License 2.0
98 stars 39 forks source link

OCI: shell conditional #70

Closed andrew-womeldorf closed 3 years ago

andrew-womeldorf commented 3 years ago

I'm very open to suggestion for this change! I know this is a common problem with shell scripting, and I'm not sure my solution is "correct".

The problem I'm having is that this conditional always evaluates to true on my machine. The outputted script looks like this:

#!/bin/sh

    if [ !  ]; then
        skopeo copy -q oci:plz-out/gen/src/api/api "docker-daemon:my-image"
        docker run -it my-image $@
    else
        podman run -it oci:plz-out/gen/my-image $@
    fi

I don't know if it's "correct" for the evaluation of $(command -v {CONFIG.PODMAN_TOOL}...) to occur in the build def, or if that should've been output as $(command -v podman...) to the shell script. I'm only doing local builds, so it's inconsequential for me.

At this point, I can say that I've had success with the conditional correctly evaluating when I remove the brackets. My machine does have podman installed, so I expect to evaluate false and run the podman command. If I replace CONFIG.PODMAN_TOOL with something my machine doesn't have, like docker, the conditional correctly evaluates to true.

Tatskaari commented 3 years ago

I think it should be as simple as:

if command -v {CONFIG.PODMAN_TOOL} > /dev/null; then 
    {CONFIG.PODMAN_TOOL} run -it {img_loc} \\\$@
else 
    ...
fi

I don't think we need the subshell. We just want to check the exit code of command -v right?

andrew-womeldorf commented 3 years ago

Thank you both for the review!

tiagovtristao commented 3 years ago

Thank you both for the review!

Can you just remove the command being executed in a subshell as suggested by @Tatskaari? Happy to get this merged afterwards. Thanks!

andrew-womeldorf commented 3 years ago

Can you just remove the command being executed in a subshell as suggested by @Tatskaari? Happy to get this merged afterwards. Thanks!

Yep, done! Thanks!