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

Support bytes for os.exec*() #48285

Closed
vstinner opened this issue Oct 3, 2008 · 11 comments
Closed

Support bytes for os.exec*() #48285

vstinner opened this issue Oct 3, 2008 · 11 comments
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@vstinner
Copy link
Member

vstinner commented Oct 3, 2008

BPO 4035
Nosy @loewis, @amauryfa, @pitrou, @vstinner
Files
  • os_exec_bytes-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-03-03.22:09:01.588>
    created_at = <Date 2008-10-03.23:38:23.423>
    labels = ['type-feature', 'library']
    title = 'Support bytes for os.exec*()'
    updated_at = <Date 2010-03-03.22:09:01.587>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2010-03-03.22:09:01.587>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2010-03-03.22:09:01.588>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2008-10-03.23:38:23.423>
    creator = 'vstinner'
    dependencies = []
    files = ['11741']
    hgrepos = []
    issue_num = 4035
    keywords = ['patch']
    message_count = 11.0
    messages = ['74283', '74304', '74313', '74449', '74451', '74503', '78681', '78718', '78739', '78829', '100355']
    nosy_count = 4.0
    nosy_names = ['loewis', 'amaury.forgeotdarc', 'pitrou', 'vstinner']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue4035'
    versions = ['Python 3.1']

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 3, 2008

    os.exec*() functions doesn't support bytes if the program name doesn't
    use absolute path. The problem is that PATH is used to complete the
    full path but Python3 disallows bytes+str (which is a good thing!).
    Example:

    python -c "import os; os.execvp('pwd', 'pwd')"
    Traceback (most recent call last):
      ...
      File "/home/haypo/prog/py3k/Lib/os.py", line 328, in execvp
        _execvpe(file, args)
      File "/home/haypo/prog/py3k/Lib/os.py", line 364, in _execvpe
        func(fullname, *argrest)
    TypeError: execv() arg 2 must be a tuple or list

    Attached patch allows bytes in os.exec*(). It converts each directory
    of PATH using sys.getfilesystemencoding().

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 4, 2008

    This is incorrect. On Windows, if the path contains characters with no
    representation in CP_ACP, encoding the path as bytes makes it invalid.

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 4, 2008

    The fix can be changed to be specific to POSIX system:
    + if name == 'posix' \
    + and isinstance(file, bytes):
    + encoding = sys.getfilesystemencoding()
    + PATH = (bytes(dir, encoding) for dir in PATH)

    My example was incorrect. The good example is:
    python -c "import os; os.execvp('pwd', ['pwd'])

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Oct 7, 2008

    Can you come up with a test case?

    Also, it would probably be good to document what parameters can have
    what datatypes in the exec family of functions.

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 7, 2008

    The patch is incomplete. It allows bytes for the arguments but not for
    the environment variables: posix_execve() in Modules/posixmodule.c
    uses:
    PyArg_Parse(key "s", &k)
    PyArg_Parse(val "s", &v)

    @vstinner
    Copy link
    Member Author

    vstinner commented Oct 8, 2008

    Improved patch:

    • support bytes in the env dictionary using the file system default
      encoding
    • support bytes for program arguments but only on a POSIX system

    Document is not updated yet.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 1, 2009

    Hmm, I think the supported types should be the same for all platforms,
    otherwise it creates unnecessary headaches.

    Perhaps, in addition to the proposed behaviour on Posix (convert
    everything to bytes if the program name is a bytes object), the reverse
    could be done under Windows (convert the program name to str if it is a
    bytes object).

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 1, 2009

    Hmm, I think the supported types should be the same for all
    platforms, otherwise it creates unnecessary headaches.

    I'm don't know Windows very well, but I read many times that Windows
    uses unicode everywhere. I'm unable to decide if it's a good thing or
    not to support also bytes on Windows, and anyway I'm unable to write
    such patch.

    See also issue bpo-4036 (bytes for subprocess.Popen), especially Martin's
    message msg74305 (Martin is our Windows expert ;-)).

    @amauryfa
    Copy link
    Member

    amauryfa commented Jan 1, 2009

    Every function of the Windows API comes in pair: an Ansi function (which
    accepts char* names) and a Wide function (which accepts wchar_t* names;
    Py_UNICODE* can be passed as-is)

    Don't perform conversion on Windows, just call the proper function.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jan 2, 2009

    Any discussion in the tracker should be deferred until a PEP has been
    written, discussed, and accepted. Then the question whether to accept a
    specific patch shouldn't be a matter of taste, but should follow from
    the general rules that the PEP has set up.

    @devdanzin devdanzin mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels May 17, 2009
    @vstinner
    Copy link
    Member Author

    vstinner commented Mar 3, 2010

    Fixed by r72313 (PEP-383).

    @vstinner vstinner closed this as completed Mar 3, 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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants