[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower  added the comment:

Distutils is now deprecated (see PEP 632) and all tagged issues are being 
closed. From now until removal, only release blocking issues will be considered 
for distutils.

If this issue does not relate to distutils, please remove the component and 
reopen it. If you believe it still requires a fix, most likely the issue should 
be re-reported at https://github.com/pypa/setuptools

--
resolution:  -> out of date
stage: test needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Brett Cannon]

Change by Brett Cannon :


--
nosy:  -brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower  added the comment:

There's been progress in terms of setuptools eventually adopting distutils, so 
that looks likely and we're basically planning on it. Adding this as an 
enhancement to setuptools is going to be a better approach than adding it to 
distutils in my opinion.

--
versions: +Python 3.7, Python 3.8 -Python 3.4, Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Mark Lawrence]

Change by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Eric Wieser]

Eric Wieser  added the comment:

> then we should perhaps instead consider the rewrite for 3.6: provide a *new* 
> distutils function that does use Popen and does things "right" (based on 
> everything we've learned since distutils was written), leaving the old 
> function in place, deprecated, for backward compatibility.

Was any progress made towards achieving this? It's frustrating that the correct 
quoting behavior we get with `subprocess.Popen(List[str])` is not used by 
`spawn`, and now every level of distutils code has to think about whether it 
needs to quote its arguments for the shell.

--
nosy: +Eric.Wieser

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Jason R. Coombs]

Jason R. Coombs added the comment:

I'm not upset by the idea of monkey patching in Setuptools for vetting certain 
techniques. I've even considered having Setuptools adopt distutils entirely. 
Today I filed https://bitbucket.org/pypa/setuptools/issues/417/adopt-distutils 
to seed the discussion on that possibility. For the sake of this discussion, I 
would happily accept a pull request to supply the forward-compatible version of 
_nt_quote_args (or similar), allowing a window for other libraries to make the 
switch against versions of Python supported by Setuptools.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower added the comment:

I notice you say adopt rather than vendor - effectively removing distutils 
from the stdlib?

It could work, but to really be able to move distutils forward we need some 
sort of side-by-side versioning, such that a package can declare which version 
of distutils is required, preferably without needing to download old versions 
on the fly when building. I'm sure it's possible but haven't got a complete 
vision yet.

Maybe it's as simple as having flags that setup scripts enable before they 
build?

import distutils
distutils.enable(distutils.flags.smart_quote_args)

That way we can actually change things without breaking old build scripts. 
Should be just as feasible if setuptools adopts distutils, but there'll 
probably be opposition from elsewhere along the lines of including core 
functionality in the stdlib.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Chris Hogan]

Chris Hogan added the comment:

Here's a change that might fix the trailing backslash problem for now without 
breaking anything.  libpath-fix.patch only affects arguments that we know are 
paths.  This happens before anything is quoted.

This avoids the problem when something like 'C:\path with space\trailing 
backslash\' is passed to _nt_quote_args() and becomes 'C:\path with 
space\trailing backslash\'.  The  is escaped which mangles the string.

I'm not sure if it's the responsibility of the user to not pass in such 
arguments, or of cpython to sanitize these things.  Is this change harmless, or 
can you think of situations where it will break something?

--
Added file: http://bugs.python.org/file40221/libpath-fix.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[R. David Murray]

R. David Murray added the comment:

Well, it sounds like there may be no way around the fact that people may 
already be doing the extra needed quoting escaping internal quotes) and 
therefore that this change would break those use cases.  (I note that any such 
code must be conditional on windows vs unix, since escaping the quotes will 
break the same command executed on unix).  

Unless...what if we conditionalize it on whether or not '\' and/or '\\' 
appears in the argument?

And yes, the fundamental problem is that Windows provides no way to call a 
program with a list of not-to-be-modified strings.  The problem with a .bat 
parsing its argument string differently from how everything else parses its 
argument string is not a new problem, and is faced by anyone using subprocess.  
distutils is trying to be (relatively) platform agnostic, to my understanding, 
so again we'd have to actually parse the input, this time looking for '.bat' 
and quoting accordingly.

If we solve the .bat problem (as opposed to ignoring it like we currently do in 
subprocess) we should fix it in subprocess too.

This is certainly getting non-trivial :(

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[R. David Murray]

R. David Murray added the comment:

*I'm* not suggesting using Popen.  That would be a more significant change to 
the code, and I don't see a motivation for it.  I'm just suggesting using 
list2cmdline for the quoting.  (If we were rewriting distutils it would be a 
different story, but we're not.)

And no, Popen with shell=True does *not* handle quoting.  If you use 
shell=True, *you* are responsible for doing correct quoting (and is what makes 
using shell=True so dangerous).  That applies to both unix and windows, and the 
confusion about that fact was what led to the need for the fix backout in 
issue 8972.

If a fix can't go in as a bug fix (whether or not it gets into 3.5.0), then we 
should perhaps instead consider the rewrite for 3.6: provide a *new* distutils 
function that does use Popen and does things right (based on everything we've 
learned since distutils was written), leaving the old function in place, 
deprecated, for backward compatibility.

That doesn't solve the problem for people running in to this bug trying to 
build extensions for 2.7, 3.4, and 3.5, though.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower added the comment:

Maybe a better starting point is to monkey-patch via setuptools? I know it's 
nasty, but it'll give us a chance to actually iterate on this before committing 
to a distutils change. (IMO the biggest problem with distutils is that it gets 
basically no testing before a final release, other than people who have already 
worked around its issues.) CCompiler.spawn is fairly easy to patch too.

Adding Jason in case he's still so upset from last time I did this (finding VC 
compiler for 2.7) he wants to shut the suggestion down immediately :)

As an aside, escaping quotes by doubling them (as in the OP) works fine for 
MSVC, but is not necessarily standard across all applications. Escaping with a 
backslash is probably more common, but (e.g.) batch files don't need them 
escaped if they aren't followed by a space and can't escape them at all if they 
are followed by a space (then again, we all agree that cmd.exe is fundamentally 
broken here, so using 
https://msdn.microsoft.com/en-us/library/windows/desktop/bb776391.aspx 
(CommandLineToArgvW API) as the benchmark is reasonable).

--
nosy: +jason.coombs

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Chris Hogan]

Chris Hogan added the comment:

 Since issue 8972 has been resolved by fixing the broken behavior, I think we 
 should just use list2cmdline.  
 We could leave _nt_quote_args alone and replace the call to it in _spawn_nt 
 with:

  cmd = [list2cmdline([arg]) for arg in cmd]

I verified that this passes the high-level test I wrote.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower added the comment:

The problem is I can easily find plenty of cases where it won't work.

My biggest concern is that list2cmdline will require already-quoted arguments, 
which is going to break anyone who's already worked around distutils failing to 
do so (which makes it really difficult to justify fixing 2.7 and 3.4, and soon 
3.5 unless we get it into 3.5.0). The escaping it applies for '\' also does 
not take into account someone who has correctly escaped their own argument.

Particularly for distutils, we look up tools by simple name and use PATHEXT, so 
we may find someone's link.bat, which then needs very different quoting from 
the actual link.exe (this one is probably unsolvable, unfortunately).

The solution isn't as simple as calling an existing function. It's probably 
actually as complicated as requiring callers to specify whether strings are 
quoted or not, and then switching between quoting modes based on what type of 
program (ie. EXE vs BAT) is being launched. list2cmdline is false hope here, 
IMO, as it just makes it harder for people to work around.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower added the comment:

FWIW, the problem in the original post is that '-DMODULE_VERSION=1.0.5' is 
not quoted by distutils and the quotes are eaten.

This can be fixed by specifying '-DMODULE_VERSION=\1.0.5\'. There are no 
spaces in the argument, so the problem is that cl.exe eats unescaped quotes, 
not that we aren't automatically changing users arguments.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[R. David Murray]

R. David Murray added the comment:

So I think the only objection to committing this as a bug fix would be the 
unfortunately real possibility that doing so will break someone's workaround.  
My *guess* is that such a workaround would most likely take the form of 
replacing _nt_quote_args as the one example we have in had did.  So I'm in 
favor of fixing this.

I've added some people to nosy who might have an opinion and even perhaps some 
way to look for other examples of people coping with this (in open source code 
bases that also target windows).

I haven't reviewed the patch yet but I will.

--
nosy: +barry, dstufft, ncoghlan, steve.dower

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Steve Dower]

Steve Dower added the comment:

Switching distutils.spawn to simply use subprocess.Popen (probably keeping the 
explicit path search) looks good to me.

Quoting rules are different when you call with shell=True (aka cmd.exe /c), 
since then you also need to escape ^, | and  (with a ^), but if it's being 
passed directly to the process (via CreateProcess) you don't need to do this. 
Popen doesn't appear to handle this correctly either.

Because of the backwards compatibility concerns, realistically I think we can 
only fix this in 3.6 at this stage. The issue has existed for too long to 
convince Larry to take it for 3.5.0, and changing the behaviour of either 
distutils or subprocess between .0 and .1 feels risky to me.

(Adding Brett in case he's motivated to take the opportunity to definitively 
learn and implement Windows's quoting rules and becoming the first person in 
the world to achieve it. Sounds like his kind of thing :) )

--
nosy: +brett.cannon

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[R. David Murray]

R. David Murray added the comment:

There are two features of this I have questions about.  If I'm understanding 
correctly, if passed a quoted string you are not re-quoting it, but you are 
always stripping a trailing slash even if it is inside quotes.

For the first, that seems wrong.  Either we should be quoting the quotes, or we 
should assume that if passed a string that starts and ends with quotes, that it 
is already fully quoted, and not change it.

For the second, doesn't that potentially change the semantics of the string 
passed in?  You can't actually know that the backslash is part of a path, nor 
can you be sure that a trailing backslash is not significant even if it is a 
path.  And in fact if there is a backslash before a double quote, arguably that 
is supposed to be a double quote that doesn't end the string (see point one 
above).

I tried list2cmdline, and what it does is to backslash escape appropriately, 
including preserving blackslashes that are in front of double quotes, and does 
not add surrounding quotes if there are no internal blanks.  I think 
list2cmdline is correct here (and it has certainly seen a lot of use), and I 
think we should either replicate its logic or just use it.

Now, subprocess calls CreateProcess, while distutils calls os.spawnv, but my 
understanding is that the quoting rules are the same (that is, that the CRT's 
_spawnv ultimately calls CreateProcess with the arguments joined by blanks).  

Since issue 8972 has been resolved by fixing the broken behavior, I think we 
should just use list2cmdline.  We could leave _nt_quote_args alone and replace 
the call to it in _spawn_nt with:

  cmd = [list2cmdline([arg]) for arg in cmd]

or we could put the fix in _nt_quote_args.  The backward compatibility calculus 
is...not obvious :(

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Mark Lawrence]

Mark Lawrence added the comment:

As far as I'm concerned distutils on Windows is a farce so do what you like 
with it.  It usually can't pick up the version of VS that you know is correct 
and is installed, the error messages are less than useless, so the only damage 
that I see is something that is even more broken than it all ready is.  
Alternatively it gets slightly further before bombing with yet another useless 
message.

--
nosy: +BreamoreBoy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Chris Hogan]

Changes by Chris Hogan christopher.ho...@intel.com:


Added file: http://bugs.python.org/file40205/quote-args-ext.tar.gz

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Chris Hogan]

Chris Hogan added the comment:

At Intel, we've run into problems with external modules giving paths to 
_nt_quote_args that contain trailing backslashes, which escapes the final quote 
and breaks the command.  This fix takes care of special characters, trailing 
backslashes, and embedded quotes as in Matt's example.  I've also added unit 
tests for these cases, as well as a high level test that builds an extension 
and defines some macros that contain quotes and special characters. 

setup.py - Includes a library directory with a trailing backslash. I compiled a 
simple library, test_module.lib, and put it in that directory to test the 
linking.  This fails without my fix.  The macro definitions also fail without 
this fix.

testmodule.c - A simple C extension.  It just makes sure everything worked and 
returns the string Success!

As for current workarounds in setup scripts, I know numpy uses their own 
distutils.  They wrote a quote_args() function that they use instead of 
_nt_quote_args(). It checks for spaces, but only if the argument isn't already 
quoted.  It doesn't account for other special characters or trailing 
backslashes.

--
nosy: +christopher.hogan
Added file: http://bugs.python.org/file40204/quote-args.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Zachary Ware]

Changes by Zachary Ware zachary.w...@gmail.com:


--
nosy: +zach.ware

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Mark Lawrence]

Changes by Mark Lawrence breamore...@yahoo.co.uk:


--
versions: +Python 2.7, Python 3.4, Python 3.5 -Python 2.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Éric Araujo]

Changes by Éric Araujo mer...@netwok.org:


--
Removed message: http://bugs.python.org/msg107751

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Éric Araujo]

Éric Araujo added the comment:

I’m more inclined to use the proposed patch rather than list2cmdline, given the 
issue linked.

I would not like a fix for this to break people’s setup.py scripts; it would be 
good to know if people work around this bug by monkey-patching the 
_nt_quote_args function, escaping quotes themselves, or something else.

If the proposed patch is the right solution, unit tests for _nt_quote_args 
would be needed, plus a high-level test that builds an extension module with a 
flag like '-DMODULE_VERSION=1.0.5', so that the windows buildbots can confirm 
this works.

--
assignee: tarek - 
components: +Windows
keywords: +needs review
stage:  - test needed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[R. David Murray]

R. David Murray rdmur...@bitdance.com added the comment:

Note that list2cmdline does correct quoting (which includes the s) if you are 
passing the string directly to a program.  In that case cmd.exe's 
metacharacters aren't special.  (As I noted in issue 8972, I believe that 
list2cmdline's current quoting of '|' characters is in error).

Perhaps a canonical list2cmdline actually belongs in shutil?

--
nosy: +r.david.murray

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Matt Giuca]

New submission from Matt Giuca matt.gi...@gmail.com:

I discovered this investigating a bug report that python-cjson doesn't compile 
properly on Windows (http://pypi.python.org/pypi/python-cjson). Cjson's 
setup.py asks distutils to compile with the flag '-DMODULE_VERSION=1.0.5', 
but distutils.spawn._nt_quote_args is not escaping the quotes correctly.

Specifically, the current behaviour is:
 distutils.spawn._nt_quote_args(['-DMODULE_VERSION=1.0.5'])
['-DMODULE_VERSION=1.0.5']

I expect the following:
 distutils.spawn._nt_quote_args(['-DMODULE_VERSION=1.0.5'])
['-DMODULE_VERSION=1.0.5']

Not surprising, since that function contains a big comment:
# XXX this doesn't seem very robust to me -- but if the Windows guys
# say it'll work, I guess I'll have to accept it.  (What if an arg
# contains quotes?  What other magic characters, other than spaces,
# have to be escaped?  Is there an escaping mechanism other than
# quoting?)

It only escapes spaces, and that's it. I've proposed a patch which escapes the 
following characters properly: ()^| (as far as I can tell, these are the 
reserved characters on Windows).

Note: I did not escape * or ?, the wildcard characters. As far as I can tell, 
these only have special meaning on the command-line itself, and not when 
supplied to a program.

Alternatively, it could call subprocess.list2cmdline (but there seem to be 
issues with that: http://bugs.python.org/issue8972).

--
assignee: tarek
components: Distutils
files: spawn.patch
keywords: patch
messages: 107722
nosy: mgiuca, tarek
priority: normal
severity: normal
status: open
title: Distutils doesn't quote Windows command lines properly
versions: Python 2.6
Added file: http://bugs.python.org/file17653/spawn.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue8987] Distutils doesn't quote Windows command lines properly

[Matt Giuca]

Changes by Matt Giuca matt.gi...@gmail.com:


--
type:  - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue8987
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com