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

subprocess: support undecodable current working directory on POSIX OS #52640

Closed
vstinner opened this issue Apr 14, 2010 · 11 comments
Closed

subprocess: support undecodable current working directory on POSIX OS #52640

vstinner opened this issue Apr 14, 2010 · 11 comments
Labels
OS-windows stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 8393
Nosy @loewis, @amauryfa, @vstinner
Files
  • posixsubprocess_cwd-2.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 = None
    closed_at = <Date 2010-04-20.20:19:48.599>
    created_at = <Date 2010-04-14.00:55:22.764>
    labels = ['library', 'OS-windows']
    title = 'subprocess: support undecodable current working directory on POSIX OS'
    updated_at = <Date 2010-04-20.20:19:48.588>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2010-04-20.20:19:48.588>
    actor = 'loewis'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-04-20.20:19:48.599>
    closer = 'loewis'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2010-04-14.00:55:22.764>
    creator = 'vstinner'
    dependencies = []
    files = ['16939']
    hgrepos = []
    issue_num = 8393
    keywords = ['patch']
    message_count = 11.0
    messages = ['103105', '103106', '103274', '103380', '103559', '103563', '103565', '103568', '103572', '103611', '103745']
    nosy_count = 3.0
    nosy_names = ['loewis', 'amaury.forgeotdarc', 'vstinner']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue8393'
    versions = ['Python 3.1', 'Python 3.2']

    @vstinner
    Copy link
    Member Author

    In py3k, subprocess uses _posixsubprocess.fork_exec() function. This function uses surrogateescape error handler for most arguments, but not for the current working directory (cwd).

    Attached patch uses PyUnicode_FSConverter() as done for other arguments. I don't know if PyUnicode_FSConverter() result is always a PyBytes, so I added an assertion. It should be fixed.

    @vstinner vstinner added stdlib Python modules in the Lib dir OS-windows labels Apr 14, 2010
    @vstinner
    Copy link
    Member Author

    See also bpo-8391.

    @vstinner
    Copy link
    Member Author

    New patch supporting bytearray type (remove the buggy assertion).

    @vstinner
    Copy link
    Member Author

    Commited in py3k (r80135), blocked in 3.1 (r80136).

    Python 3.1 has no _posixsubprocess module, it uses os.chdir() which accepts bytes, bytearray and str with surrogates.

    @amauryfa
    Copy link
    Member

    It does not work on Windows:

    >>> subprocess.Popen("c:/windows/notepad.exe", cwd=b'c:/temp')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "D:\afa\python\py3k-1\lib\subprocess.py", line 681, in __init__
        restore_signals, start_new_session)
      File "D:\afa\python\py3k-1\lib\subprocess.py", line 892, in _execute_child
        startupinfo)
    TypeError: must be string or None, not bytes

    The function sp_CreateProcess() in PC/_subprocess.c should probably be fixed.
    And please add unit tests...

    @amauryfa amauryfa reopened this Apr 19, 2010
    @vstinner
    Copy link
    Member Author

    And please add unit tests...

    I'm thinking on this. I plan to write tests for all my last changes about surrogates.

    @vstinner
    Copy link
    Member Author

    It does not work on Windows

    I always consider Windows as a special case because Windows uses unicode internally. Byte string are converted quickly to unicode using the current locale.

    My patch was for UNIX/BSD which uses byte string internally.

    sp_CreateProcess() in PC/_subprocess.c uses CreateProcessW. To support byte string, we should use the byte string version of CreateProcess ("CreateProcessA" ?) or convert current directory to unicode (using the current locale). The second solution minimize the changes.

    @amauryfa
    Copy link
    Member

    PEP-277 explicitly states that unicode strings should be passed to wide-character functions, whereas byte strings use "standard" functions.
    This is done in posixmodule.c, for example.

    The "current locale" is a moving thing.

    @vstinner
    Copy link
    Member Author

    PEP-277 explicitly states that unicode strings should be passed to
    wide-character functions, whereas byte strings use "standard"
    functions. This is done in posixmodule.c, for example.

    CreateProcessW takes a lot of arguments. Should we CreateProcessA or CreateProcessW if some arguments are byte string and other are unicode string?

    The "current locale" is a moving thing.

    Don't CreateProcessA do the same thing? Convert byte string to unicode using the current locale. To use the right words, by "current locale" I mean the "mbcs" Windows codec.

    @amauryfa
    Copy link
    Member

    "mbcs" is not a fixed encoding and may change between Windows sessions, see the Rationale in PEP-277 http://www.python.org/dev/peps/pep-0277/

    The mixed case is interesting. We could use CreateProcessW when at least one string is Unicode, and CreateProcessA when all strings are bytes.
    OTOH this is unlikely, because for example the environment will be passed as unicode; so I'm OK to use CreateProcessW in all cases, and use mbcs to convert bytes.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Apr 20, 2010

    Amaury, I fail to see how the error you get on Windows is related to this issue. AFAICT, neither the issue nor the patch affects Windows at all.

    Closing the issue as fixed.

    If you think there is also an issue on Windows, please submit a new bug report. However, I don't think the error you get is a bug: what you see is correct behavior.

    @loewis loewis mannequin closed this as completed Apr 20, 2010
    @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
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants