Skip to content

Commit

Permalink
RF: run: Always use a string for the command
Browse files Browse the repository at this point in the history
From the command-line, the run command always comes in as a list.  If
it contains a single item, we unwrap it and pass the string directly
to the runner.  From the python api, the run command can come in as a
string or a list.  Commands in the string form are executed with
shell=True (so underneath with "/bin/sh -c"), which makes it possible
to use things like redirection and pipes in commands.

When we add support for commands with {inputs} and {outputs}
placeholders, the two representations will require separate
handling (e.g., if string, call format directly; if list, map format
call).  Instead of doing that, convert the list representation to the
string representation.
  • Loading branch information
kyleam committed May 31, 2018
1 parent 6ac6433 commit 9c3a65e
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
11 changes: 9 additions & 2 deletions datalad/interface/run.py
Expand Up @@ -22,6 +22,8 @@
from os.path import relpath
from os.path import isabs

from six.moves import shlex_quote

from datalad.interface.base import Interface
from datalad.interface.utils import eval_results
from datalad.interface.base import build_doc
Expand Down Expand Up @@ -329,8 +331,13 @@ def run_command(cmd, dataset=None, inputs=None, outputs=None, expand=None,
for res in _unlock_or_remove(ds, rerun_outputs):
yield res

# anticipate quoted compound shell commands
cmd = cmd[0] if isinstance(cmd, list) and len(cmd) == 1 else cmd
if isinstance(cmd, list):
if len(cmd) == 1:
# This is either a quoted compound shell command or a simple
# one-item command. Pass it as is.
cmd = cmd[0]
else:
cmd = " ".join(shlex_quote(c) for c in cmd)

# TODO do our best to guess which files to unlock based on the command string
# in many cases this will be impossible (but see rerun). however,
Expand Down
4 changes: 4 additions & 0 deletions datalad/interface/tests/test_run.py
Expand Up @@ -102,6 +102,10 @@ def test_basics(path, nodspath):
res = ds.run('touch empty', message='NOOP_TEST')
assert_status('notneeded', res)
eq_(last_state, ds.repo.get_hexsha())
# We can also run the command via a single-item list because this is
# what the CLI interface passes in for quoted commands.
res = ds.run(['touch empty'], message='NOOP_TEST')
assert_status('notneeded', res)

# run outside the dataset, should still work but with limitations
with chpwd(nodspath), \
Expand Down

0 comments on commit 9c3a65e

Please sign in to comment.