Fix normalization during install

[techalchemy]

Escape command line requirements as necessary

• Fixes "pipenv install 'foo<=4.5'" fails because of quoting issue #2998

[orf]

Just tested and this correctly installs the pdfminer.six dependency within my lockfile 🎉

[immerrr]

Holy moly! So much headache about quoting and unquoting, and all because delegator uses shell=True unconditionally and is buggy when you pass in a list of arguments

12e80ed#diff-ec74dda0078733c5b6755671cacf1de5R1424

amitt001/delegator.py#58

This is the culprit IMHO, after all it's the library that should handle command execution. Instead arbitrary argument quoting snippets are found here and there all over pipenv.

[techalchemy]

Not entirely accurate. We do proper splitting in various other libraries and are still forced to contend with this.

[immerrr]

@techalchemy why do you need to do extra splitting? I mean you need to do some splitting to handle requirements.txt lines, but other than that passing commands around as lists of verbatim (without any quoting) arguments should be enough, no?

[techalchemy]

What if a path has a space in it? What if that happens on windows? What if both of those are true and the user is using nave path separators (that is, \)? If we figure out a target path for example it still needs to be quoted even if it is split. And if we are parsing user input as we do for --python we also need to be able to perform the splitting

[immerrr]

The short answer is that normally command line quoting should be handled by standard libraries.

What if a path has a space in it?

subprocess.check_output([mycmd, 'hello world']) will pass hello world as a single string to mycmd command, that is if it were a python script it would have sys.argv == ['echo', 'hello world']. It achieves that by using one of the exec-family of functions.

What if that happens on windows?

On windows, CreateProcess takes a string rather than a list, and subprocess uses subprocess.list2cmdline to internally properly quote a string that is passed to a subprocess.

What if both of those are true and the user is using nave path separators (that is, )?

It depends on what do you want to do with it. If you want to pass it forward, then again subprocess does all the quoting as necessary, so something like

subprocess.check_output([cmd, ..., sys.argv[1], ...])

will just work™.

If you want to handle it within pipenv, w.r.t. native/non-native path separators, it's a bit of a mess, but something along the lines of of path_components = os.path.normpath(user_path).split(os.path.sep) and then os.path.join(path_components) down the line should do the trick.

And if we are parsing user input as we do for --python

Same as above.

But like you say, apparently the problems pipenv is experiencing with quoting are related to delegator.py, because it is using shell=True kwarg for all subprocess invocations which is not only slow, but also leads to problems:

• on POSIX with shell=True arguments passed as a list are interpreted differently (see this python bug and this delegator.py bug
• on Windows, it is very tricky to quote shell variables reliably (the Internet is full of advice to do smth like cmd /c prog ^%PATH^%, but it is not 100% bulletproof as you can have an env-variable named PATH^ and it seems to get expanded before ^-escaping takes place)