On Thu, Jun 12, 2014 at 1:30 AM, Chris Angelico <rosuav@gmail.com> wrote:

On Thu, Jun 12, 2014 at 7:58 AM, Ryan <rymg19@gmail.com> wrote:

...

In all seriousness, to me this is obvious. When you pass a command to the shell, naturally, certain details are shell-specific.

On Windows cmd.exe is used by default: http://hg.python.org/cpython/file/38a325c84564/Lib/subprocess.py#l1108 so it makes sense to make default behavior cross-platform.

...

-10000. Bad idea. Very bad idea. If you want the ^ to be escaped, do it yourself. Or better yet, don't pass shell=True.

Definitely the latter. Why pass shell=True when executing a single command? I don't get it.

This is a complete use case using Rietveld upload script: http://techtonik.rainforce.org/2013/07/code-review-with-rietveld-and-mercuri... I am interested to know how to modify upload script without kludges: https://code.google.com/p/rietveld/source/browse/upload.py#1056 I expect many people are facing with the same problem trying to wrap Git and HG with Python scripts. -- anatoly t.

On Fri, Jun 13, 2014 at 2:55 AM, Ryan Gonzalez <rymg19@gmail.com> wrote:

SHELLS ARE NOT CROSS-PLATFORM!!!! Seriously, there are going to be differences. If you really must:

escape = lambda s: s.replace('^', '^^') if os.name == 'nt' else s

It is not about generic shell problem, it is about specific behavior that on Windows Python already uses cmd.exe shell hardcoded in its sources. So for crossplatform behavior on Windows, it should escape symbols on command passed to cmd.exe that are special to this shell to avoid breaking Python scripts. What you propose is a bad workaround, because it assumes that all Python users who use subprocess to execute hg or git should possess apriori knowledge about default subprocess behaviour with default shell on Windows and implement workaround for that. -- anatoly t.

On 15/06/2014 02:22, Ryan Gonzalez wrote:

Of course cmd.exe is hardcoded;

Of course it's not: (from Lib/subprocess.py) comspec = os.environ.get("COMSPEC", "cmd.exe") I don't often expect, in these post-command.com days, to get anything other than cmd.exe. But alternative command processors are certainly possible. TJG

* anatoly techtonik <techtonik@gmail.com> [2014-06-12 02:00:55 +0300]:

On Thu, Jun 12, 2014 at 1:30 AM, Chris Angelico <rosuav@gmail.com> wrote:

...

Why pass shell=True when executing a single command? I don't get it.

I don't know about Linux, but on Windows programs are not directly available as /usr/bin/python, so you need to find command in PATH directories. Passing shell=True makes this lookup done by shell and not manually.

As it's been said, the whole *point* of shell=True is to be able to use shell features, so ^ being escaped automatically just would be... broken. How would I escape > then, for example ;) You basically have two options: - Do the lookup in PATH yourself, it's not like that's rocket science. I haven't checked if there's a ready function for it in the stdlib, but even when not: Get os.environ['PATH'], split it by os.pathsep, then for every directory check if your binary is in there. There's also some environment variable on Windows which contains the possible extensions for a binary in PATH, add that, and that's all. - Use shell=True and a cross-platform shell escape function. I've wrote one for a project of mine: [1] I've written some tests[2] but I haven't checked all corner-cases, so I can't guarantee it'll always work, as the interpretation of special chars by cmd.exe *is* black magic, at least to me. Needless to say this is probably the worse choice of the two. [1] http://git.the-compiler.org/qutebrowser/tree/qutebrowser/utils/misc.py?id=df... [2] http://git.the-compiler.org/qutebrowser/tree/qutebrowser/test/utils/test_mis... Florian -- http://www.the-compiler.org | me@the-compiler.org (Mail/XMPP) GPG 0xFD55A072 | http://the-compiler.org/pubkey.asc I love long mails! | http://email.is-not-s.ms/

Also notice that using a list with shell=True is using the API incorrectly. It wouldn't even work on Linux, so that torpedoes the cross-platform concern already :) This kind of confusion is why I opened http://bugs.python.org/issue7839. On Wed, 11 Jun 2014 16:58:30 -0500, Ryan <rymg19@gmail.com> wrote:

Of course! And, why not escape everything else, too?

abc -> ^a^b^c

echo %PATH% -> ^e^c^h^o^ ^%^P^A^T^H^%

In all seriousness, to me this is obvious. When you pass a command to the shell, naturally, certain details are shell-specific.

-10000. Bad idea. Very bad idea. If you want the ^ to be escaped, do it yourself. Or better yet, don't pass shell=True.

anatoly techtonik <techtonik@gmail.com> wrote:

...

I am banned from tracker, so I post the bug here:

Normal Windows behavior:

...

hg status --rev ".^1" M mercurial\commands.py ? pysptest.py

...

hg status --rev .^1 abort: unknown revision '.1'!

So, ^ is an escape character. See http://www.tomshardware.co.uk/forum/35565-45-when-special-command-line

But subprocess doesn't escape it, making cross-platform command fail on Windows.

---[cut pysptest.py]-- import subprocess as sp

# this fails with # abort: unknown revision '.1'! cmd = ['hg', 'status', '--rev', '.^1'] # this works #cmd = 'hg status --rev ".^1"' # this works too #cmd = ['hg', 'status', '--rev', '.^^1']

try: print sp.check_output(cmd, stderr=sp.STDOUT, shell=True) except Exception as e: print e.output ------------------------------

-- anatoly t.

------------------------------------------------------------------------

_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/rymg19%40gmail.com

-- Sent from my Android phone with K-9 Mail. Please excuse my brevity. _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/rdmurray%40bitdance.com

On Thu, Jun 12, 2014 at 10:00 AM, anatoly techtonik <techtonik@gmail.com> wrote:

I thought exactly about that. Usually separate arguments are used to avoid problems with escaping of quotes and other stuff.

I'd deprecate subprocess and split it into separate modules. One is about shell execution and another one is for secure process control.

ISTM what you want is not shell=True, but a separate function that follows the system policy for translating a command name into a path-to-binary. That's something that, AFAIK, doesn't currently exist in the Python 2 stdlib, but Python 3 has shutil.which(). If there's a PyPI backport of that for Py2, you should be able to use that to figure out the command name, and then avoid shell=False. ChrisA

On 06/11/2014 07:12 PM, Chris Angelico wrote:

On Thu, Jun 12, 2014 at 12:07 PM, Chris Angelico <rosuav@gmail.com> wrote:

...

ISTM what you want is not shell=True, but a separate function that follows the system policy for translating a command name into a path-to-binary. That's something that, AFAIK, doesn't currently exist in the Python 2 stdlib, but Python 3 has shutil.which(). If there's a PyPI backport of that for Py2, you should be able to use that to figure out the command name, and then avoid shell=False.

Huh. Next time, Chris, search the web before you post. Via a StackOverflow post, learned about distutils.spawn.find_executable().

--> import sys --> sys.executable '/usr/bin/python'

On Thu, Jun 12, 2014 at 5:12 AM, Chris Angelico <rosuav@gmail.com> wrote:

On Thu, Jun 12, 2014 at 12:07 PM, Chris Angelico <rosuav@gmail.com> wrote:

...

ISTM what you want is not shell=True, but a separate function that follows the system policy for translating a command name into a path-to-binary. That's something that, AFAIK, doesn't currently exist in the Python 2 stdlib, but Python 3 has shutil.which(). If there's a PyPI backport of that for Py2, you should be able to use that to figure out the command name, and then avoid shell=False.

Huh. Next time, Chris, search the web before you post. Via a StackOverflow post, learned about distutils.spawn.find_executable().

I remember I even wrote a patch for it, but I forgot about it already. Still feels like a hack that is difficult to find and understand that you need really it. In Rietveld case it won't work, because upload.py script allows user to specify arbitrary diff command to send change for review.

On 15 June 2014 00:15, Greg Ewing <greg.ewing@canterbury.ac.nz> wrote:

However, it says the Windows version uses CreateProcess, which doesn't use PATH.

Huh? CreateProcess uses PATH:

py -3.4 Python 3.4.0 (v3.4.0:04f714765c13, Mar 16 2014, 19:25:23) [MSC v.1600 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information.

...

...

import subprocess subprocess.check_call(['echo', 'hello']) hello 0

"echo" is an executable "C:\Utils\GnuWin64\echo.exe" which is on PATH but not in the current directory... Paul

Paul Moore wrote:

Huh? CreateProcess uses PATH:

Hmm, in that case Microsoft's documentation is lying, or subprocess is doing something itself before passing the command name to CreateProcess. Anyway, looks like there's no problem. -- Greg

On Fri, Jun 13, 2014 at 12:11 PM, Nikolaus Rath <Nikolaus@rath.org> wrote:

Can someone describe an use case where shell=True actually makes sense at all?

It seems to me that whenever you need a shell, the argument's that you pass to it will be shell specific. So instead of e.g.

Popen('for i in `seq 42`; do echo $i; done', shell=True)

you almost certainly want to do

Popen(['/bin/sh', 'for i in `seq 42`; do echo $i; done'], shell=False)

because if your shell happens to be tcsh or cmd.exe, things are going to break.

Some features, while technically shell-specific, are supported across a lot of shells. You should be able to pipe output from one command into another in most shells, for instance. But yes, I generally don't use it. ChrisA

* Nikolaus Rath <Nikolaus@rath.org> [2014-06-12 19:11:07 -0700]:

"R. David Murray" <rdmurray@bitdance.com> writes:

...

Also notice that using a list with shell=True is using the API incorrectly. It wouldn't even work on Linux, so that torpedoes the cross-platform concern already :)

This kind of confusion is why I opened http://bugs.python.org/issue7839.

Can someone describe an use case where shell=True actually makes sense at all?

It seems to me that whenever you need a shell, the argument's that you pass to it will be shell specific. So instead of e.g.

Popen('for i in `seq 42`; do echo $i; done', shell=True)

you almost certainly want to do

Popen(['/bin/sh', 'for i in `seq 42`; do echo $i; done'], shell=False)

because if your shell happens to be tcsh or cmd.exe, things are going to break.

My usecase is a spawn-command in a GUI application, which the user can use to spawn an executable. I want the user to be able to use the usual shell features from there. However, I also pass an argument to that command, and that should be escaped. Florian -- http://www.the-compiler.org | me@the-compiler.org (Mail/XMPP) GPG 0xFD55A072 | http://the-compiler.org/pubkey.asc I love long mails! | http://email.is-not-s.ms/

Nikolaus Rath wrote:

you almost certainly want to do

Popen(['/bin/sh', 'for i in `seq 42`; do echo $i; done'], shell=False)

because if your shell happens to be tcsh or cmd.exe, things are going to break.

On Unix, the C library's system() and popen() functions always use /bin/sh, NOT the user's current login shell, for this very reason. I would hope that the Python versions of these, and also the new subprocess stuff, do the same. That still leaves differences between Unix and Windows, but explicitly naming the shell won't help with that. -- Greg

On 13/06/2014 03:11, Nikolaus Rath wrote:

"R. David Murray" <rdmurray@bitdance.com> writes:

...

Also notice that using a list with shell=True is using the API incorrectly. It wouldn't even work on Linux, so that torpedoes the cross-platform concern already :)

This kind of confusion is why I opened http://bugs.python.org/issue7839.

Can someone describe an use case where shell=True actually makes sense at all?

On Windows (where I think the OP is), Popen & friends ultimately invoke CreateProcess. In the case where shell=True, subprocess invokes the command interpreter explictly under the covers and tweaks a few other things to avoid a Brief Flash of Unstyled Console. This is the relevant snippet from subprocess.py: if shell: startupinfo.dwFlags |= _winapi.STARTF_USESHOWWINDOW startupinfo.wShowWindow = _winapi.SW_HIDE comspec = os.environ.get("COMSPEC", "cmd.exe") args = '{} /c "{}"'.format (comspec, args) That's all. It's more or less equivalent to your prefixing your commands with "cmd.exe /c". The only reasons you should need to do this are: * If you're using one of the few commands which are actually built-in to cmd.exe. I can't quickly find an online source for these, but typical examples will be: "dir" or "copy". * In some situations -- and I've never been able to nail this -- if you're trying to run a .bat/.cmd file. I've certainly been able to run batch files without shell=True but other people have failed within what appears to be the same configuration unless invoking cmd.exe via shell=True. I use hg.exe (from TortoiseHg) but ISTR that the base Mercurial install supplies a .bat/.cmd. If that's the OP's case then he might find it necessary to pass shell=True. TJG