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

Pathlib crashes when os module is missing 'link' method #82992

Closed
tohojo mannequin opened this issue Nov 15, 2019 · 10 comments
Closed

Pathlib crashes when os module is missing 'link' method #82992

tohojo mannequin opened this issue Nov 15, 2019 · 10 comments
Labels
3.8 only security fixes 3.9 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@tohojo
Copy link
Mannequin

tohojo mannequin commented Nov 15, 2019

BPO 38811
Nosy @brettcannon, @vstinner, @serhiy-storchaka, @miss-islington, @tohojo
PRs
  • bpo-38811: Check for presence of os.link method in pathlib #17170
  • [3.8] bpo-38811: Check for presence of os.link method in pathlib. (GH-17170) #17204
  • Revert "bpo-38811: Check for presence of os.link method in pathlib" #17219
  • bpo-38811: Check for presence of os.link method in pathlib (v2) #17225
  • [3.8] bpo-38811: Check for presence of os.link method in pathlib (GH-17225) #17626
  • 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-12-16.12:28:38.360>
    created_at = <Date 2019-11-15.12:04:34.777>
    labels = ['3.8', 'type-bug', 'library', '3.9']
    title = "Pathlib crashes when os module is missing 'link' method"
    updated_at = <Date 2019-12-16.12:42:56.354>
    user = 'https://github.com/tohojo'

    bugs.python.org fields:

    activity = <Date 2019-12-16.12:42:56.354>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-12-16.12:28:38.360>
    closer = 'vstinner'
    components = ['Library (Lib)']
    creation = <Date 2019-11-15.12:04:34.777>
    creator = 'tohojo'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 38811
    keywords = ['patch']
    message_count = 10.0
    messages = ['356670', '356696', '356703', '356818', '356858', '356860', '358477', '358480', '358482', '358483']
    nosy_count = 5.0
    nosy_names = ['brett.cannon', 'vstinner', 'serhiy.storchaka', 'miss-islington', 'tohojo']
    pr_nums = ['17170', '17204', '17219', '17225', '17626']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue38811'
    versions = ['Python 3.8', 'Python 3.9']

    @tohojo
    Copy link
    Mannequin Author

    tohojo mannequin commented Nov 15, 2019

    Commit 6b5b013 ("bpo-26978: Implement pathlib.Path.link_to (Using os.link) (GH-12990)") introduced a new link_to method in pathlib. However, this makes pathlib crash when the 'os' module is missing a 'link' method, as can be seen in the report in this CI test:

    https://travis-ci.org/tohojo/flent/jobs/611950252

    @tohojo tohojo mannequin added type-crash A hard crash of the interpreter, possibly with a core dump 3.8 only security fixes stdlib Python modules in the Lib dir labels Nov 15, 2019
    @brettcannon
    Copy link
    Member

    Just an FYI a raised exception isn't considered a crash. For us a crash is a C-level crash like a segfault.

    @brettcannon brettcannon added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Nov 15, 2019
    @tohojo
    Copy link
    Mannequin Author

    tohojo mannequin commented Nov 15, 2019

    Right, sure, that makes sense; thanks for clarifying :)

    @serhiy-storchaka
    Copy link
    Member

    New changeset 111772f by Serhiy Storchaka (Toke Høiland-Jørgensen) in branch 'master':
    bpo-38811: Check for presence of os.link method in pathlib. (GH-17170)
    111772f

    @vstinner
    Copy link
    Member

    The change failed on x86 Windows7 3.x buildbot:
    https://buildbot.python.org/all/#/builders/58/builds/3257

    Moreover, PR 17170 was merged whereas the CLA was not signed. So I created PR 17219 to revert the change to fix the Windows 7 failure and to wait until the CLA is signed.

    ======================================================================
    ERROR: test_symlink_to_not_implemented (test.test_pathlib.PathTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_pathlib.py", line 2031, in test_symlink_to_not_implemented
        link.symlink_to(target)
      File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\pathlib.py", line 1395, in symlink_to
        self._accessor.symlink(target, self, target_is_directory)
    OSError: [WinError 1314] A required privilege is not held by the client: 'D:\\cygwin\\home\\db3l\\buildarea\\3.x.bolen-windows7\\build\\build\\test_python_848\\test_python_worker_2032\\@test_2032_tmp\\fileA' -> 'D:\\cygwin\\home\\db3l\\buildarea\\3.x.bolen-windows7\\build\\build\\test_python_848\\test_python_worker_2032\\@test_2032_tmp\\dirA\\linkAA'

    ======================================================================
    ERROR: test_symlink_to_not_implemented (test.test_pathlib.WindowsPathTest)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\test\test_pathlib.py", line 2031, in test_symlink_to_not_implemented
        link.symlink_to(target)
      File "D:\cygwin\home\db3l\buildarea\3.x.bolen-windows7\build\lib\pathlib.py", line 1395, in symlink_to
        self._accessor.symlink(target, self, target_is_directory)
    OSError: [WinError 1314] A required privilege is not held by the client: 'D:\\cygwin\\home\\db3l\\buildarea\\3.x.bolen-windows7\\build\\build\\test_python_848\\test_python_worker_2032\\@test_2032_tmp\\fileA' -> 'D:\\cygwin\\home\\db3l\\buildarea\\3.x.bolen-windows7\\build\\build\\test_python_848\\test_python_worker_2032\\@test_2032_tmp\\dirA\\linkAA'

    @vstinner
    Copy link
    Member

    New changeset 59c8088 by Victor Stinner in branch 'master':
    Revert "bpo-38811: Check for presence of os.link method in pathlib. (GH-17170)" (bpo-17219)
    59c8088

    @vstinner
    Copy link
    Member

    New changeset 092435e by Victor Stinner (Toke Høiland-Jørgensen) in branch 'master':
    bpo-38811: Check for presence of os.link method in pathlib (GH-17225)
    092435e

    @vstinner
    Copy link
    Member

    Thanks Toke Høiland-Jørgensen! I merged your fix.

    I don't really get why the Debian/Ubuntu image on Travis CI didn't get os.link() function, but I'm not really intersted to dig into this question. It's fine to simply skip the feature if it's not available, we do that in many places in Python.

    I asked Toke Høiland-Jørgensen to revert his changes about os.symlink(): the pathlib code to handle missing os.symlink() seems to be broken, but it also seems like Python no longer support platforms without os.symlink(). Only old Windows version didn't support os.symlink(), but Python don't support these versions anymore.

    I merged the os.link() change into master and made a 3.8 backport which will be merged automatically soon.

    I close the issue.

    @vstinner vstinner added the 3.9 only security fixes label Dec 16, 2019
    @miss-islington
    Copy link
    Contributor

    New changeset 8d0f369 by Miss Islington (bot) in branch '3.8':
    bpo-38811: Check for presence of os.link method in pathlib (GH-17225)
    8d0f369

    @vstinner
    Copy link
    Member

    If someone cares about the missing os.symlink code path: please open a separated issue.

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