[BUG] Rogue string expansion

[ben-bay]

🐛 Bug Report

Describe the bug
The string "$_" expands to a path in the venv.

To Reproduce
Steps to reproduce the behavior:

1. Add "$_" anywhere in a spec
2. merlin run <spec.yaml>
3. Look in the provenance file.

Expected behavior
The string "$_" should not change.

EDIT: According to this, "$_" is a special shell variable.

[lucpeterson]

I think we need to remove the expanduser and expandvars references by Merlin outside of the workers

merlin/merlin/spec/expansion.py

Line 83 in 54166e0

line = expandvars(expanduser(line))

merlin/merlin/spec/expansion.py

Line 141 in 54166e0

new_val = expandvars(new_val)

I propose we leave user and environment variables alone and let the workers take care of them as they would at run time

[ben-bay]

@lucpeterson okay, so even symbols like ~ won't work then

[lucpeterson]

we should check whether having "~" in a spec file would work or not

[lucpeterson]

i bring this up to address Timo's desire to use environment variables in spec descriptions for extra flexibility

as it stands now, environment variables are expanded when "merlin run" is executed and filled out in the spec, and therefore hard-coded into the step. this prevents using things like ${SYS_TYPE} within steps

[ben-bay]

I see, good point. It's an easy fix, I'll run some tests to make sure what we think is happening is happening.

[ben-bay]

Ohhhh I see the problem. Has nothing to do with ~. We should leave env vars alone, to instead be interpreted by each step's shell and not meddled with by merlin.

[ben-bay]

#247

[koning]

To be clear, you should be able to use env variables in all sections except the steps. These are definitely used in the batch section. I mean they should all be replaced at run , except for the steps.

[lucpeterson]

Do we have to explicitly replace them or can they be replaced by the native shell? Can we center around an example?

[koning]

Well the point is to replace them immediately, like paths to flux in the batch section. There are not part of a step so they won't be replaced by the env when they are run. Also in variables in the env section, those should be replaced at run, if a user wants the variable to remain unchanged , it should be in a step.

[koning]

I think this is semantics, by replaced I mean made available at run with the run env variables. So yes, as long as they are in the shell when run it is fine.

[lucpeterson]

Ah ok, I think leaving them alone (and exposed) at run time will actually fix another (related) bug/feature. I was having issues using ${SYS_TYPE} in the batch section for flux w/ run-workers and was having a hard time tracking it down. Not sure yet if it's related, but there was some odd behavior

[ben-bay]

Fixed with #247