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

os.readlink does not accept pathlib.Path on Windows #78565

Closed
girtsf mannequin opened this issue Aug 12, 2018 · 9 comments
Closed

os.readlink does not accept pathlib.Path on Windows #78565

girtsf mannequin opened this issue Aug 12, 2018 · 9 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes OS-windows stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@girtsf
Copy link
Mannequin

girtsf mannequin commented Aug 12, 2018

BPO 34384
Nosy @gpshead, @pfmoore, @tjguk, @berkerpeksag, @zware, @serhiy-storchaka, @eryksun, @zooba, @girtsf
PRs
  • bpo-34384: Fix os.readlink() on Windows #8740
  • 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-10-23.20:57:52.838>
    created_at = <Date 2018-08-12.03:41:11.660>
    labels = ['3.7', '3.8', 'type-bug', 'library', 'OS-windows']
    title = 'os.readlink does not accept pathlib.Path on Windows'
    updated_at = <Date 2019-10-23.20:57:52.836>
    user = 'https://github.com/girtsf'

    bugs.python.org fields:

    activity = <Date 2019-10-23.20:57:52.836>
    actor = 'steve.dower'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-10-23.20:57:52.838>
    closer = 'steve.dower'
    components = ['Library (Lib)', 'Windows']
    creation = <Date 2018-08-12.03:41:11.660>
    creator = 'girtsf'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34384
    keywords = ['patch']
    message_count = 9.0
    messages = ['323427', '323460', '323463', '323465', '323486', '323488', '323560', '355263', '355264']
    nosy_count = 9.0
    nosy_names = ['gregory.p.smith', 'paul.moore', 'tim.golden', 'berker.peksag', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'girtsf']
    pr_nums = ['8740']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue34384'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @girtsf
    Copy link
    Mannequin Author

    girtsf mannequin commented Aug 12, 2018

    Documentation for os.readlink says "Changed in version 3.6: Accepts a path-like object.". This works fine on macOS, but blows up on Windows:

    Python 3.7.0 (v3.7.0:1bf9cc5093, Jun 27 2018, 04:06:47) [MSC v.1914 32 bit (Intel)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import os
    >>> import pathlib
    >>> os.readlink(pathlib.Path('foo'))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: readlink() argument 1 must be str, not WindowsPath

    @girtsf girtsf mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Aug 12, 2018
    @serhiy-storchaka serhiy-storchaka added 3.8 only security fixes OS-windows labels Aug 12, 2018
    @berkerpeksag
    Copy link
    Member

    Steve and/or Eryk, I was adding some tests for os.readlink() in PR 8740 and I noticed that os.readlink() always returns str on Windows. However, the documentation for os.readlink() says:

    If the path is a bytes object (direct or indirectly), the result will
    be a bytes object.
    

    The question is, should I add support for bytes as part of PR 8740? And what is the best way to implement it? (e.g. use CreateFileA() if PyUnicode_Check(path.object) returns false?)

    @berkerpeksag
    Copy link
    Member

    In msg235615 (bpo-9949), Zachary said using bytes paths on Windows is deprecated, but I can't see the actual conversation because Rietveld seems to be down: https://bugs.python.org/review/9949/#ps5077 I think the os.readlink() documentation needs to be updated if we decide to keep the status quo.

    @serhiy-storchaka
    Copy link
    Member

    Currently the behavior doesn't match the documentation. Initially I thought that this can be solved by adding the support of path-like objects and backporting this up to 3.6. But os.readlink() on Windows doesn't not support also bytes paths, and never supported. This looks to me now more like a new feature. In 3.6 and 3.7 we can resolve this issue by just updating the documentation.

    Bytes paths on Windows were deprecated before 3.6. Since implementing PEP-529, their should be supported on Windows if they are supported on other platforms. We shouldn't use an 8-bit API like CreateFileA, but instead decode bytes paths with UTF-8 and use a Unicode API (this should be implemented in path_converter()).

    @zooba
    Copy link
    Member

    zooba commented Aug 13, 2018

    Serhiy is exactly right, but to emphasise, the best way to do paths now is to use argument clinic with its path_t type. On Windows, .narrow is a flag that indicates whether you should PyUnicode_FSEncode() any results before returning them, and .wide is always initialized correctly.

    Do not use any *A APIs - only *W from now on :)

    @berkerpeksag
    Copy link
    Member

    Thanks for the suggestions! I've updated PR 8740. I will submit separate PRs to fix os.readlink() documentation in 3.6 and 3.7 branches.

    @berkerpeksag
    Copy link
    Member

    New changeset e0b5b20 by Berker Peksag in branch 'master':
    bpo-34384: Fix os.readlink() on Windows (GH-8740)
    e0b5b20

    @gpshead
    Copy link
    Member

    gpshead commented Oct 23, 2019

    can this be closed?

    @zooba
    Copy link
    Member

    zooba commented Oct 23, 2019

    Yep

    @zooba zooba closed this as completed Oct 23, 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
    3.7 (EOL) end of life 3.8 only security fixes 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

    4 participants