SCIF v0.8.2 does not substitute [e]ENV_VAR correctly

[dtrudg]

Over on sylabs/singularity, we noticed that some of our end-to-end tests that run Docker SCIF containers (not Singularity containers) started failing overnight, coincident with the release of v0.8.2.

We do an exec hello-world-echo echo of the SCIF app hello-world-echo, with a command line that includes the [e]SCIF_APPNAME construct that previously substituted the value of $SCIF_APPNAME:

i.e.

exec hello-world-echo echo "This is different text that should still include [e]SCIF_APPNAME"

resulted in the following output on SCIF v0.8.1:

This is different text that should still include hello-world-echo

Now, with SCIF v0.8.2, the [e]SCIF_APPNAME is substituted with a literal $SCIF_APPNAME, not its value...

This is different text that should still include $SCIF_APPNAME

[dtrudg]

To repicate using the tutorial containers...

Expected substitution with older SCIF version in the image referenced in tutorials:

$ docker run vanessa/scif:hw exec hello-world-env echo [e]OMG
[hello-world-env] executing /bin/echo $OMG
TACOS

Update to v0.8.2 does and substitution does not work correctly

$ docker run -it --rm --entrypoint=/bin/bash vanessa/scif:hw
(base) root@8187c28fb5da:/# pip install --upgrade scif
...
Successfully installed scif-0.0.82

(base) root@8187c28fb5da:/# scif exec hello-world-env echo [e]OMG
[hello-world-env] executing /bin/echo $OMG
$OMG

[dtrudg]

It looks to me as if the root cause is the change here:

67ea105#diff-9a3e86c7ce958be3e6d12390d98a2b51380fbd09434ecb030d839edc92574c87R175

From the PR: #68

interactive was previously true, so the command was executed in a shell, which performs the variable substitution.

Now, interactive is false, so the command is not executed in a shell, and the variable substitution does not happen.

It's not clear to me exactly how to get the shell substitution back, while preserving the behaviour in #68

Not sure whether the shell substitution is more important, so #68 should be rolled back, or the exit code behaviour is more important - and we should update our tests for the non-substituted env var?

[vsoch]

lol! That was fast. I didn't realize the python scif was being used here. @dtrudg how do you want to fix it? Should I just revert the PR for now? The issue is just having the return code for run.

[vsoch]

I'll roll back for now and we can figure out another fix for the return code.

[dtrudg]

lol! That was fast. I didn't realize the python scif was being used here. @dtrudg how do you want to fix it? Should I just revert the PR for now? The issue is just having the return code for run.

Having looked around, I notice the [e]ENV_VAR behaviour is mentioned in various places, and has been there for some time... so I'd probably suggest that rolling back is the right approach for now.

Hopefully @dmachi might have an idea for an alternative approach to their code that doesn't impact the substitution?

[dmachi]

I don't think that interactive=False change is the problem here. It was originally interactive=interactive, and in one of my intermediate patches before the PR I changed it to interactive=True, exit=True and then in my final patch I changed it back to interactive=interactive. I must have missed something else. I will test this and get it figured out.

[dtrudg]

As an aside... @vsoch ... is there any chance you'd be able to add tag this repo with the version used at each PyPi release?

That would make it easier to do a git bisect to pin down any future issue observed.

[vsoch]

As an aside... @vsoch ... is there any chance you'd be able to add tag this repo with the version used at each PyPi release?

yep can do.