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

shutil.which() should support bytes #62483

Closed
Arfrever mannequin opened this issue Jun 22, 2013 · 11 comments
Closed

shutil.which() should support bytes #62483

Arfrever mannequin opened this issue Jun 22, 2013 · 11 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@Arfrever
Copy link
Mannequin

Arfrever mannequin commented Jun 22, 2013

BPO 18283
Nosy @loewis, @vstinner, @tarekziade, @hynek, @serhiy-storchaka, @csabella
PRs
  • bpo-18283: Add support for bytes to shutil.which #11818
  • Files
  • which_bytes.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 2019-02-13.11:25:48.982>
    created_at = <Date 2013-06-22.15:29:11.010>
    labels = ['3.8', 'type-feature', 'library']
    title = 'shutil.which() should support bytes'
    updated_at = <Date 2019-02-13.11:25:48.982>
    user = 'https://bugs.python.org/Arfrever'

    bugs.python.org fields:

    activity = <Date 2019-02-13.11:25:48.982>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-02-13.11:25:48.982>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2013-06-22.15:29:11.010>
    creator = 'Arfrever'
    dependencies = []
    files = ['30674']
    hgrepos = []
    issue_num = 18283
    keywords = ['patch']
    message_count = 11.0
    messages = ['191644', '191684', '191686', '191691', '206370', '206373', '206374', '206386', '335141', '335209', '335415']
    nosy_count = 8.0
    nosy_names = ['loewis', 'vstinner', 'tarek', 'Arfrever', 'python-dev', 'hynek', 'serhiy.storchaka', 'cheryl.sabella']
    pr_nums = ['11818']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18283'
    versions = ['Python 3.8']

    @Arfrever
    Copy link
    Mannequin Author

    Arfrever mannequin commented Jun 22, 2013

    shutil.which() should support bytes. Some other functions in shutil module support bytes.

    >>> shutil.which("echo")
    '/bin/echo'                                                                                                                                                  
    >>> shutil.which(b"echo")                                                                                                                                    
    Traceback (most recent call last):                                                                                                                           
      File "<stdin>", line 1, in <module>                                                                                                                        
      File "/usr/lib64/python3.3/shutil.py", line 1126, in which
        name = os.path.join(dir, thefile)
      File "/usr/lib64/python3.3/posixpath.py", line 92, in join
        "components.") from None
    TypeError: Can't mix strings and bytes in path components.
    >>> shutil.which("echo", path="/bin")
    '/bin/echo'
    >>> shutil.which("echo", path=b"/bin")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.3/shutil.py", line 1098, in which
        path = path.split(os.pathsep)
    TypeError: Type str doesn't support the buffer API
    >>> shutil.which(b"echo", path=b"/bin")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib64/python3.3/shutil.py", line 1098, in which
        path = path.split(os.pathsep)
    TypeError: Type str doesn't support the buffer API

    @Arfrever Arfrever mannequin added the stdlib Python modules in the Lib dir label Jun 22, 2013
    @serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Jun 22, 2013
    @vstinner
    Copy link
    Member

    Here is an implementation.

    @serhiy-storchaka
    Copy link
    Member

    What about bytearrays and other byte-like objects?

    However I'm not sure this enhancement is worth to be accepted. Many other high-level functions in os and shutil modules do not support bytes paths. For shutil.which() there is no backward compatibility with Python 2 which we should support.

    @vstinner
    Copy link
    Member

    "What about bytearrays and other byte-like objects?

    However I'm not sure this enhancement is worth to be accepted. Many other high-level functions in os and shutil modules do not support bytes paths. For shutil.which() there is no backward compatibility with Python 2 which we should support."

    Bytes is the native type for path on all platforms... except Windows. So I fixed many functions of the os module to support bytes parameters. For example, functions of os.path support str and bytes types, but not bytearray. I also prefer to reject byte-like objects, to keep it simple.

    The bytes type should be accepted for any kind of os function parameter: path, environment variable, command line argument, etc. It's not a question of being compatible with Python 2, an application can make the choice of only used bytes because bytes is the native type for OS data on UNIX.

    Python 3 supports also "bytes stored in Unicode" thanks to the PEP-393, but it's not exactly the same: a path stored as str depends on the filesystem locale, whereas a bytes string is well defined (it's just an array of 8-bit unsigned numbers) and doesn't depend on any configuration variable.

    @serhiy-storchaka
    Copy link
    Member

    All low-level functions in the os.path module supports string and bytes paths. But not all high-level functions in the shutil module supports bytes paths. Adding support for bytes paths will complicate implementation and tests.

    @vstinner
    Copy link
    Member

    Almost all functions of the shutil module support bytes filenames. I tested shutil.copyfile(), shutil.ignore_patterns(), shutil.copytree(), shutil.rmtree() and shutil.move().

    I only found one function which only support Unicode filenames: shutil.make_archive().

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 16, 2013

    New changeset a1a05e2724dd by Victor Stinner in branch 'default':
    Issue bpo-18283: shutil.which() now supports bytes argument, not only text argument.
    http://hg.python.org/cpython/rev/a1a05e2724dd

    @vstinner
    Copy link
    Member

    which_bytes.patch does not work on Windows.

    According to Serhiy, it's a new feature and so should wait for Python 3.5.

    @vstinner vstinner reopened this Dec 16, 2013
    @csabella
    Copy link
    Contributor

    csabella commented Feb 9, 2019

    Would it be worthwhile to make a PR of this for 3.8?

    @csabella csabella added the 3.8 only security fixes label Feb 9, 2019
    @vstinner
    Copy link
    Member

    Would it be worthwhile to make a PR of this for 3.8?

    Yes. Would you be interested to convert my old patch to a fresh PR? I can help you to work on that.

    @vstinner
    Copy link
    Member

    New changeset 5680f65 by Victor Stinner (Cheryl Sabella) in branch 'master':
    bpo-18283: Add support for bytes to shutil.which (GH-11818)
    5680f65

    @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
    3.8 only security fixes 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