Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_popen fails on Windows if installed to "Program Files" #43977

Closed
loewis mannequin opened this issue Sep 15, 2006 · 14 comments
Closed

test_popen fails on Windows if installed to "Program Files" #43977

loewis mannequin opened this issue Sep 15, 2006 · 14 comments
Assignees
Labels
OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@loewis
Copy link
Mannequin

loewis mannequin commented Sep 15, 2006

BPO 1559298
Nosy @loewis, @birkenfeld, @pjenvey, @tjguk, @ezio-melotti, @florentx, @eryksun, @zooba
Files
  • popen.diff
  • issue1559298_trunk.diff: patch and docs against r77505
  • issue1559298_py3k.diff: py3k version of the patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/tjguk'
    closed_at = <Date 2019-09-11.13:12:30.795>
    created_at = <Date 2006-09-15.12:59:09.000>
    labels = ['type-bug', 'library', 'OS-windows']
    title = 'test_popen fails on Windows if installed to "Program Files"'
    updated_at = <Date 2019-09-11.13:12:30.795>
    user = 'https://github.com/loewis'

    bugs.python.org fields:

    activity = <Date 2019-09-11.13:12:30.795>
    actor = 'steve.dower'
    assignee = 'tim.golden'
    closed = True
    closed_date = <Date 2019-09-11.13:12:30.795>
    closer = 'steve.dower'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2006-09-15.12:59:09.000>
    creator = 'loewis'
    dependencies = []
    files = ['7540', '15885', '15886']
    hgrepos = []
    issue_num = 1559298
    keywords = ['patch', 'needs review', 'buildbot']
    message_count = 14.0
    messages = ['51121', '51122', '51123', '57615', '88328', '90104', '97790', '97791', '97802', '98512', '219999', '220020', '236175', '236209']
    nosy_count = 10.0
    nosy_names = ['loewis', 'georg.brandl', 'pjenvey', 'tim.golden', 'JosephArmbruster', 'ezio.melotti', 'squeegee', 'flox', 'eryksun', 'steve.dower']
    pr_nums = []
    priority = 'normal'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1559298'
    versions = ['Python 2.7']

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Sep 15, 2006

    test_popen fails in 2.5c2. The reason is that popen invokes

    cmd.exe /c "c:\program files\python25\python.exe" -c
    "import sys;print sys.argv"

    cmd.exe does not support that syntax, and gives an
    error (which silently disappears); the pipe read then
    returns an empty string.

    This problem exists atleast since Python 2.3.

    To fix this, cmd.exe needs to be invoked as

    cmd.exe /c "c:\program files\python25\python.exe" -c
    "import sys;print sys.argv"

    The attached patch fixes this by always wrapping the
    command line with an addition pair of quotes.

    It's not clear to me whether this can go into 2.5.1: an
    application may already work around this problem by
    passing extra quotes to popen, which would then break
    if popen adds even more quotes.

    @loewis loewis mannequin added OS-windows labels Sep 15, 2006
    @birkenfeld
    Copy link
    Member

    I don't see a difference between your two command lines here... did you mean to add additional quotes in the second example?

    @loewis
    Copy link
    Mannequin Author

    loewis mannequin commented Mar 14, 2007

    Indeed, this should read

    cmd.exe /c ""c:\program files\python25\python.exe" -c "import sys;print sys.argv""

    @JosephArmbruster
    Copy link
    Mannequin

    JosephArmbruster mannequin commented Nov 18, 2007

    I applied the change to:

    Python 2.6a0 (trunk:58651M, Nov 18 2007, 08:46:54) [MSC v.1400 32 bit
    (Intel)] on win32

    and test_popen passes appeared to pass.

    @devdanzin devdanzin mannequin added tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error labels Mar 30, 2009
    @pjenvey
    Copy link
    Member

    pjenvey commented May 26, 2009

    subprocess also needs this fix applied

    Does the w9xopen command line below not need this?

    @squeegee
    Copy link
    Mannequin

    squeegee mannequin commented Jul 4, 2009

    What is needed is separate quoting for the command and the argument.
    Right now, test_popen only surrounds the entire command line with quotes:

    "c:\Program Files\Python2.6\Python.exe -u c:\Documents and Settings\Russ
    Gibson\cgi-bin\cgi.py"

    It needs to the above as thus:

    "c:\Program Files\Python2.6\Python.exe" -u "c:\Documents and
    Settings\Russ Gibson\cgi-bin\cgi.py"

    From the command line, the second case works, but the first doesn't.
    Neither work from os.popen in 2.6.2.

    To make sure that popen works in all cases under windows, test_popen*
    need to test with separate quotes around the command and argument. That
    will show the real problem with the current os.popen* implementation(s).

    (yeah, I know, shut up and provide a patch...)

    @briancurtin
    Copy link
    Member

    This has come up recently and Martin's approach seems to work. I updated his patch for trunk, and test_popen passes when I run it with and without a space in the path.

    Russ Gibson's suggestion was already applied to test_popen, so the only code change is to posixmodule.c.

    @briancurtin briancurtin added stdlib Python modules in the Lib dir and removed tests Tests in the Lib/test dir labels Jan 14, 2010
    @briancurtin
    Copy link
    Member

    Here is a py3k version of the previous patch. os.popen is implemented using subprocess.Popen, so the quoting change was made there.

    @florentx
    Copy link
    Mannequin

    florentx mannequin commented Jan 15, 2010

    It is on Python 2.7a2

    >>> os.popen('"C:\\Python27\\python.exe" -c "import sys; print sys.argv" 42').read()
    ''
    
    >>> os.popen('""C:\\Python27\\python.exe" -c "import sys; print sys.argv" 42"').read()
    "['-c', '42']\n"
    
    >>> os.popen3('"C:\\Python27\\python.exe" -c "import sys; print sys.argv" 42')[2].read()
    '\'C:\\Python27\\python.exe" -c "import\' est pas reconnu en tant que commande interne ou externe, un programme executable ou un fichier de commandes.\n'

    It may be related to the test_popen failure.
    Actually, it seems that the outer double quotes are removed before executing the cmdstring:
    C:\Python27\python.exe" -c "import
    ^ ^

    @briancurtin
    Copy link
    Member

    Florent is correct. The patch seems to fix regular popen, but popen3 sees problems. I'll see if I can fit this in and have a look.

    Also of note is that the other flavors of popen are not tested...at least not in Lib/test/test_popen.py or Lib/test/test_os.py

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jun 7, 2014

    Is this worth pursuing given the wording here https://docs.python.org/3/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3 ?

    @eryksun
    Copy link
    Contributor

    eryksun commented Jun 8, 2014

    This is fixed for subprocess.Popen in 2.7, 3.1, and 3.2; see bpo-2304. In 2.7, nt.popen still has this problem. As mentioned above, it can be worked around by using subprocess.Popen as described here:

    https://docs.python.org/2/library/subprocess.html#replacing-os-popen-os-popen2-os-popen3

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Feb 18, 2015

    Do we leave this open or close it as there is a known work around for 2.7?

    @zooba
    Copy link
    Member

    zooba commented Feb 19, 2015

    The docs are pretty clear about using Popen instead of os.popen, and people using popen have likely worked around it already, so we're more likely to break them by changing it now.

    I vote to close.

    @zooba zooba closed this as completed Sep 11, 2019
    @zooba zooba closed this as completed Sep 11, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants