Better shell quoting

1. Only use split() when we actually come in from CLI (possibly never
   need to do that at all)
2. Add some additional test cases to cover quoting.
3. Better error message in case of missing executable pulled from a run
script.

E.g.:

```
[scripts]
random = "myexecutable a b c"
myexecutable = "echo 5"
```

```
Error: the command myexecutable (from random) could not be found within PATH.
```

vs. previous, which had a slightly disingenous error message.

```
Error: the command myexecutable could not be found within PATH or Pipfile's [scripts].
```
This commit is contained in:
Jeff Tratner
2018-03-29 00:48:37 -07:00
parent 7afa528c0f
commit 5de48b4800
3 changed files with 62 additions and 40 deletions
+1
View File
@@ -2,6 +2,7 @@
- Resolve editable packages on the local filesystem.
- Ensure lock hash does not change based on injected env vars.
- Fix bug in detecting .venv at project root when in subdirectories.
- Parse quoting in [scripts] section correctly + clearer run errors.
11.9.0:
- Vastly improve markers capabilities.
- Support for environment variables in Pipfiles.
+44 -22
View File
@@ -2184,20 +2184,30 @@ def inline_activate_virtualenv():
)
def _construct_run_command(project, command, args):
"""Construct a command to run concatenating project and args.
``command`` and/or the script from the ``Pipfile`` will be inserted before args.
Returns (executable, args for executable)"""
args = list(args or [])
# Script was found…
if command in project.scripts:
prefix_argv = project.scripts[command]
else:
# Separate out things that were passed in as a string.
prefix_argv = list(command.split())
executable, prefix_argv = prefix_argv[0], prefix_argv[1:]
for __c in reversed(prefix_argv):
args.insert(0, __c)
return executable, args
def do_run(command, args, three=None, python=False):
# Ensure that virtualenv is available.
ensure_project(three=three, python=python, validate=False)
load_dot_env()
# Script was found…
if command in project.scripts:
command = ' '.join(project.scripts[command])
# Separate out things that were passed in as a string.
_c = list(command.split())
command = _c.pop(0)
if _c:
args = list(args)
for __c in reversed(_c):
args.insert(0, __c)
executable, args = _construct_run_command(project, command, args)
# Activate virtualenv under the current interpreter's environment
inline_activate_virtualenv()
# Windows!
@@ -2205,23 +2215,35 @@ def do_run(command, args, three=None, python=False):
import subprocess
p = subprocess.Popen(
[command] + list(args), shell=True, universal_newlines=True
[executable] + list(args), shell=True, universal_newlines=True
)
p.communicate()
sys.exit(p.returncode)
else:
command_path = system_which(command)
command_path = system_which(executable)
if not command_path:
click.echo(
'{0}: the command {1} could not be found within {2} or Pipfile\'s {3}.'
''.format(
crayons.red('Error', bold=True),
crayons.red(command),
crayons.normal('PATH', bold=True),
crayons.normal('[scripts]', bold=True),
),
err=True,
)
if command in project.scripts:
click.echo(
'{0}: the command {1} (from {2}) could not be found within {3}.'
''.format(
crayons.red('Error', bold=True),
crayons.red(executable),
crayons.normal(command, bold=True),
crayons.normal('PATH', bold=True),
),
err=True,
)
else:
click.echo(
'{0}: the command {1} could not be found within {2} or Pipfile\'s {3}.'
''.format(
crayons.red('Error', bold=True),
crayons.red(command),
crayons.normal('PATH', bold=True),
crayons.normal('[scripts]', bold=True),
),
err=True,
)
sys.exit(1)
# Execute the command.
os.execl(command_path, command_path, *args)
+17 -18
View File
@@ -4,7 +4,8 @@ import shutil
import json
import pytest
import warnings
from pipenv.core import activate_virtualenv
from pipenv import core
from pipenv.core import activate_virtualenv, _construct_run_command
from pipenv.utils import (
temp_environ, get_windows_path, mkdir_p, normalize_drive, TemporaryDirectory
)
@@ -1170,6 +1171,9 @@ flask = "==0.12.2"
f.write(r"""
[scripts]
printfoo = "python -c \"print('foo')\""
notfoundscript = "randomthingtotally"
appendscript = "cmd arg1"
multicommand = "bash -c \"cd docs && make html\""
""")
c = p.pipenv('install')
assert c.return_code == 0
@@ -1179,20 +1183,15 @@ printfoo = "python -c \"print('foo')\""
assert c.out == 'foo\n'
assert c.err == ''
@pytest.mark.run
@pytest.mark.skip(reason='This fails on Windows (not sure about POSIX).')
def test_scripts_quoted(self):
with PipenvInstance(chdir=True) as p:
with open(p.pipfile_path, 'w') as f:
f.write("""
[scripts]
printfoo = "python -c print('foo')"
""")
c = p.pipenv('install')
assert c.return_code == 0
c = p.pipenv('run printfoo')
assert c.return_code == 0
assert c.out == 'foo\n'
assert c.err == ''
if os.name != 'nt':
c = p.pipenv('run notfoundscript')
assert c.return_code == 1
assert c.out == ''
assert 'Error' in c.err
assert 'randomthingtotally (from notfoundscript)' in c.err
executable, argv = _construct_run_command(Project(), 'multicommand', [])
assert executable == 'bash'
assert argv == ['-c', 'cd docs && make html']
executable, argv = _construct_run_command(Project(), 'appendscript', ['a', 'b'])
assert executable == 'cmd'
assert argv == ['arg1', 'a', 'b']