Quoting issues when using '--job' with GC3Pie

[geimer]

EasyBuild environment variables seem to be converted to command-line options when using --job with GC3Pie. However, quoting seems to be broken in some places. Below is a list of the issues I ran into (but there may be more, of course):

• Using

  EASYBUILD_TEST_REPORT_ENV_FILTER="^SSH|GPG|USER|HOSTNAME|UID|KDE|XDG|.*(COOKIE|SESSION).*"
  

  leads to

  --test-report-env-filter=^SSH|GPG|USER|HOSTNAME|UID|KDE|XDG|.*(COOKIE|SESSION).*
  

  Here, the parens cause havoc:

  /bin/sh: 1: Syntax error: word unexpected (expecting ")")
  

• Using

  EASYBUILD_SUFFIX_MODULES_PATH=
  

  to get rid of the all default setting leads to

  --suffix-modules-path=\\\"\\\"
  

  which in the end creates a directory called "" in the modules path. This lets the sanity check fail (loading fake module failed).

[geimer]

Initial tests indicate that both issues disappear if shell_quote (see https://github.com/hpcugent/vsc-base/blob/master/lib/vsc/utils/missing.py#L282) gets fixed. Below is a poor man's approach of addressing it, but I guess there are much better ways to express this in Python ;-) I'm also not sure whether I caught all shell special characters that need escaping (or whether I escaped too much).

def shell_quote(x):
    """Add quotes so it can be passed to shell"""
    result = str(x)
    result = result.replace("\\", "\\\\")
    result = result.replace("\"", "\\\"")
    result = result.replace("\'", "\\\'")
    result = result.replace(" ", "\\ ")
    result = result.replace("(", "\\(")
    result = result.replace(")", "\\)")
    result = result.replace("[", "\\[")
    result = result.replace("]", "\\]")
    result = result.replace("|", "\\|")
    result = result.replace("<", "\\<")
    result = result.replace(">", "\\>")
    result = result.replace("&", "\\&")
    result = result.replace("*", "\\*")
    result = result.replace("?", "\\?")
    return result

[geimer]

This is apparently a known issue, see hpcugent/vsc-base#152.

[boegel]

@geimer: an attempt to fix this was made in hpcugent/vsc-base#151, but it didn't make it in...

The gist of it was to define shell_quote as follows, using a negative lookbehind assertion (see also https://docs.python.org/2/library/re.html):

def shell_quote(token):
    return "'%s'" % re.sub(r"(?<!\\)'", r"\'", str(token))

Can you check whether that fixes the problem you're having too? If so, I can make a new attempt at fixing it.

[geimer]

@boegel: Yep, this unreadable piece of code fixes my issues as well ;-)
I guess the "unset vs. empty string" thing has to be handled in generate_cmd_line (https://github.com/hpcugent/vsc-base/blob/master/lib/vsc/utils/generaloption.py#L1550)? Or maybe it is already and I just missed it...

[stdweird]

@geimer so both errors occur when using --job? it has not so much to do with the shell_quote (i'm not sure how idempotent shell_quote should be, need to think about it), but there is a bug in the generation of the commandline in generaloption i think, but there is something a lot better that can be done to handle this imho.

[geimer]

@stdweird Yes, both happen with --job and GC3Pie. I don't have access to a PBS system, so I can't verify that it's job backend affected as well. There may be other ways to address this, sure, but shell_quote doesn't do what the name implies, i.e., that characters are properly quoted to be used in a shell command.

[boegel]

+1 on the latter: shell_quote doesn't do what it's supposed to do (and worse, it uses an undocumented function from subprocess...)

[wpoely86]

Using --try-toolchain is also very broken (probably same reason). I get this (using --job):

--try-toolchain="['\"[\'foss\'', '']" PLUMED-2.2.0-foss-2015b.eb '2015b']

[geimer]

@wpoely86: The issues may be related, but there is apparently a difference. I'm locally using the proposed fix from hpcugent/vsc-base#151 to get GC3Pie to work with my setup, but today also ran into problems with --try-toolchain and strange quoting. Need to investigate...

[geimer]

Any chance that hpcugent/vsc-base#151 gets revisited? This damn issue is hitting me every time I upgrade to a new EasyBuild version, which is really annoying...

[geimer]

@wpoely86: The --try-toolchain thing is a different problem. But once it is resolved, the quoting may become an issue again ;-)

But first things first: It seems that the generate_cmd_line method in generaloption.py of vsc-base does not handle lists correctly (whatever that means). Or it is triggered with improper arguments when composing the command line for the job script. The branch taken for --try-toolchain is actually https://github.com/hpcugent/vsc-base/blob/master/lib/vsc/utils/generaloption.py#L1695, where opt_value is some sort of Python list object. This seems to get expanded to a string like ["'[\\'GCC\\'", " \\'4.9.3-2.25\\']'"], which is then subject to quoting. This can only end up in a mess...

[geimer]

@wpoely86: Please reopen #1475. Using --try-toolchain together with --job is a different issue. It can be fixed by filtering out the --try-toolchain option like it is done for --robot, see https://github.com/hpcugent/easybuild-framework/blob/master/easybuild/tools/parallelbuild.py#L130. This is valid as the temporary easyconfig is already generated by the eb "driver" instance, i.e., the eb instance running within the batch job doesn't have to do the same thing again. One can probably also filter out some more --try-* options...

[geimer]

I'd like to give this issue another ping...

What about monkey patching shell_quote in EB until there is a proper fix in vsc-base? Of course we have to make sure that this doesn't break anything else.

[boegel]

workaround for the quoting issue implemented in #1915

the problem with --try-toolchain is largely irrelevant thanks to #1908, but it still highlights a problem

I figured out that this was caused by the incorrect type specification for the --toolchain (and thus also --try-toolchain) option, which should have been 'strlist', 'extend' rather than None, 'extend'...

[boegel]

@geimer now #1908 and #1915 are merged, this can be closed?

[geimer]

@boegel Yes, I would consider it as fixed. Unless the "real" fix in vsc-base breaks it again ;-)

[JensTimmerman]

real fix came in hpcugent/vsc-base#257