Skip to content
This repository

Quoting path names with spaces for local() fails on Windows #258

Closed
bitprophet opened this Issue · 18 comments

1 participant

Jeff Forcier
Jeff Forcier
Owner

Description

Environment: Windows 7, Python 2.7, Fabric 0.9.2 final

When using path names with spaces on Windows, you usually wrap the whole path into double quotes. I just can't make this work with Fabric and local(), and the results are quite strange.

Here is my example script:

from fabric.api import *

def demo():
    cmd = r'dir "sub directory with spaces"'
    print('Command: {}'.format(cmd))
    local(cmd, capture=False)

And the output is:

D:\temp>fab demo
Command: dir "sub directory with spaces"
[localhost] run: dir "sub directory with spaces"
The system cannot find the file specified.

Fatal error: local() encountered an error (return code 1) while executing 'dir "sub directory with spaces"'

Aborting.

Of course, copy/pasting the command into the console works just fine. (Output omitted for boringness.)

This seems to be somehow related to some strange behaviour of the subprocess.Popen class and the subprocess.list2cmdline function it uses.

I extracted the local() implementation from the 0.9 branch (source:/entry/fabric/0.9/fabric/operations.py) and removed the in-place list creation on the Popen constructor call and then it works fine (example code attached). That is, I replaced

p = subprocess.Popen([real_command], shell=True, stdout=out_stream, stderr=err_stream)

with

p = subprocess.Popen(real_command, shell=True, stdout=out_stream, stderr=err_stream)

No idea if this is valid for all other platforms as well.


Originally submitted by Henrik Heimbuerger (hheimbuerger) on 2010-11-16 at 05:47am EST

Attachments

Relations

  • Duplicated by #319: operations.local fails under Python 2.7.1 in Windows
  • Duplicated by #327: lcd fails on Windows

Closed as Done on 2011-05-02 at 01:31pm EDT

Jeff Forcier
Owner

Henrik Heimbuerger (hheimbuerger) posted:


Hmm, can't edit the description or add related tickets. :(

So here's the link to the documentation on Popen: http://docs.python.org/library/subprocess.html#subprocess.Popen

Source of list2cmdline: http://svn.python.org/view/python/tags/r27/Lib/subprocess.py?view=markup#list2cmdline

Possible related bugs: #118, #155


on 2010-11-16 at 07:03am EST

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Hi Henrik,

I don't think #118 or #155 are actually related, despite mentioning quotes, since if your diagnosis is accurate this has to do with local() and subprocess -- those other ones are related to run().

From reading those docs re: subprocess and list2cmdline, it sounds like we may be able to get away with a win32 check that toggles the change you referred to, i.e. going from a string-in-a-list to just a string.

If you could try changing the [real_command] expression on that line to real_command if win32 else [real_command] and see if that does it, I'd appreciate it. You'll also need to from fabric.state import win32 inside the top of the function (doing it at module level usually results in circular imports).


on 2010-11-16 at 10:05am EST

Jeff Forcier
Owner

Henrik Heimbuerger (hheimbuerger) posted:


Yes, I can confirm that it works for me after making the modifications you suggested (my new demonstration fabfile is attached).

Are you sure this platform switch is necessary, though? Reading the corresponding line for Unix, "If args is a string, it specifies the command string to execute through the shell", that sounds to me as if it should work as well.

What is the reasoning for passing an (always 1-element) list in there? It's not like local() gave you the option to pass the arguments as a list instead of an already concatenated string.


on 2010-11-16 at 12:13pm EST

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


I seem to remember us having to switch to the list version to avoid other, oddball problems in some edge cases. You're quite right that as the docs go, it should be cool to have it as just a string. But given that this only seems to affect Windows (nobody's ever mentioned it being a problem on POSIX platforms), I'd prefer the platform specific tweak for now to limit any potential damage.


on 2010-11-16 at 04:46pm EST

Jeff Forcier
Owner

Raymond Cote (rgacote) posted:


Just got caught with this error myself (Fabric 0.9.3 installed with pypm, Python 2.7, Windows 7, 32-bit).

Problem not isolated to directories with strings.
In Fabric 0.9.1 on Windows XP, Python 2.5, these commands run fine:
local(r'mkdir dist')
local(r'rmdir /S /Q .\dist')

In Fabric 0.9.3 on Windows 7, Python 2.7, they always fail. When I dig into the error it says the command "mkdir dist" is not found.
After reading through the above, I changed line 632 of operations.py to be:
p = subprocess.Popen(real_command if win32 else [real_command], shell=True, stdout=out_stream,

and added from fabric.state import win32

to the start of the routine. Now all runs as expected on my Windows box.


on 2010-12-19 at 11:50am EST

Jeff Forcier
Owner

**** (khorn) posted:


I have also encountered this issue, and the workarounds given in this ticket work for me as well.

Fabric 0.9.3
Python 2.7
WinXP


on 2011-02-01 at 03:06pm EST

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Thanks for doing that tweak/test, Raymond -- that definitely sounds like the way to go. I'll try to incorporate it soon (not on my own computer right now, stranded at the in-laws thanks to the northeast US blizzard. hurrah.)


on 2010-12-29 at 03:07pm EST

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Applied in changeset commit:eb9c7a86dac25042c8b66bfafa89de4ca10ae761.


on 2011-03-16 at 10:04pm EDT

Jeff Forcier
Owner

David Chandek-Stark (dchandekstark) posted:


I've noticed a number of issues with Fabric 0.9.3 on Python 2.7 (32)/Windows 7 (64), some of which are probably not related to Fabric proper. But the workaround in this ticket solved my problem executing

local('svn ls http://svn.example.com/')

on 2011-02-15 at 10:21am EST

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Thanks, you found a dumb mistake which I've just fixed+pushed :) (An import was missing which is present in 1.0 and master but not 0.9.)


on 2011-03-17 at 12:25pm EDT

Jeff Forcier
Owner

**** (joelryan2k) posted:


I have tested your fix and verified that this has fixed .9. 1.0 is still working, too

Thanks!


on 2011-03-17 at 01:10pm EDT

Jeff Forcier
Owner

**** (joelryan2k) posted:


In testing the 0.9 branch, I get the error:

File "c:\pythonenv\myloaninfo\src\fabric\fabric\operations.py", line 631, in local
cmd_arg = [real_command] if win32 else real_command
NameError: global name 'win32' is not defined

The 1.0 branch works fine


on 2011-03-17 at 12:08pm EDT

Jeff Forcier
Owner

Siddhaarth Verma (sid) posted:


Oops, the above should read:

This is easily fixed by changing line 1005 in fabric/operations from:

cmd_arg = [wrapped_command] if win32 else wrapped_command

to:

cmd_arg = wrapped_command if win32 else [wrapped_command]

on 2011-04-27 at 03:58pm EDT

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Yes, looks like the original "fix" was logically inverted. Just pushed the flipped version to the 1.0 branch so it will be in 1.0.2 -- I will probably release 1.0.2 shortly before/after 1.1 which should be out this week.


on 2011-04-27 at 04:35pm EDT

Jeff Forcier
Owner

Siddhaarth Verma (sid) posted:


This is still broken for me using Fabric 1.0.1 on Windows XP, Python 2.7.1.

It works in the very simple case of a command with no spaces (e.g. local("dir")), but fails for something like local("dir w*").

This is easily fixed by changing line 1005 in fabric/operations from:

cmd_arg = wrapped_command if win32 else [wrapped_command]

to:

cmd_arg = wrapped_command if win32 else [wrapped_command]

on 2011-04-27 at 03:56pm EDT

Jeff Forcier
Owner

Siddhaarth Verma (sid) posted:


Jeff - confirmed, this is working correctly for me in the 1.0 branch. Thanks for the quick fix!


on 2011-05-02 at 01:29pm EDT

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Sid -- if you're able to check out the 1.0 branch from the Git repo and verify that it actually does work now, that would be great. No pressure though.


on 2011-04-27 at 04:36pm EDT

Jeff Forcier
Owner

Jeff Forcier (bitprophet) posted:


Forgot to change the ticket itself...meh.


on 2011-05-02 at 01:31pm EDT

Jeff Forcier bitprophet closed this
Ramon van Alteren ramonvanalteren referenced this issue from a commit in ramonvanalteren/fabric
Jeff Forcier Fixes #258 - Windows local logic inversion fix 2e90af6
Rich Schumacher richid referenced this issue from a commit in richid/fabric
Jeff Forcier Tweak subprocess invocation on Windows.
Fixes #258

Conflicts:

	fabric/operations.py
eb9c7a8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.