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

GH-109187: Improve symlink loop handling in pathlib.Path.resolve() #109192

Merged

Conversation

barneygale
Copy link
Contributor

@barneygale barneygale commented Sep 9, 2023

Treat symlink loops like other errors: in strict mode, raise OSError, and in non-strict mode, do not raise an exception.


📚 Documentation preview 📚: https://cpython-previews--109192.org.readthedocs.build/

…ve()`

Treat symlink loops like other errors: in strict mode, raise `OSError`, and
in non-strict mode, do not raise any exception.
@barneygale
Copy link
Contributor Author

barneygale commented Sep 23, 2023

Hey @zooba, please could you review this if you have the time/inclination? It's a sort-of follow-up to #25264, which you reviewed a couple years ago. No worries if not! Thank you.

Previously we decided to raise RuntimeError from Path.resolve(), regardless of the value of strict, when a symlink loop was encountered. This behaviour differs from os.path.realpath(), but maintains compatibility with older pathlib.

Now we're looking at adding a new PathBase class with its own resolve() method, which provides an additional incentive to straighten out how symlink loops are handled, and makes the minor backwards compatibility break worthwhile IMHO.

@barneygale barneygale merged commit ecd813f into python:main Sep 26, 2023
23 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot s390x Debian 3.x has failed when building commit ecd813f.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/49/builds/6673) and take a look at the build logs.
  4. Check if the failure is related to this commit (ecd813f) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/49/builds/6673

Failed tests:

  • test_tools

Failed subtests:

  • test_freeze_simple_script - test.test_tools.test_freeze.TestFreeze.test_freeze_simple_script

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/test/test_tools/test_freeze.py", line 32, in test_freeze_simple_script
    outdir, scriptfile, python = helper.prepare(script, outdir)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Tools/freeze/test/freeze.py", line 146, in prepare
    copy_source_tree(srcdir, SRCDIR)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Tools/freeze/test/freeze.py", line 95, in copy_source_tree
    shutil.copytree(oldroot, newroot, ignore=ignore_non_src)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/shutil.py", line 588, in copytree
    return _copytree(entries=entries, src=src, dst=dst, symlinks=symlinks,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/Lib/shutil.py", line 542, in _copytree
    raise Error(errors)
shutil.Error: [('/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/build/test_python_3316406æ/test_python_qj_0d1oo.sock', '/tmp/test_python_qsnyutqh/tmpcgick913/cpython/build/test_python_3316406æ/test_python_qj_0d1oo.sock', "[Errno 6] No such device or address: '/home/dje/cpython-buildarea/3.x.edelsohn-debian-z/build/build/test_python_3316406æ/test_python_qj_0d1oo.sock'")]

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x has failed when building commit ecd813f.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/345/builds/5925) and take a look at the build logs.
  4. Check if the failure is related to this commit (ecd813f) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/345/builds/5925

Failed tests:

  • test_signal

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_signal.py", line 1287, in test_stress_delivery_dependent
    self.assertEqual(len(sigs), N, "Some signals were lost")
AssertionError: 4525 != 10000 : Some signals were lost

csm10495 pushed a commit to csm10495/cpython that referenced this pull request Sep 28, 2023
…ve()` (pythonGH-109192)

Treat symlink loops like other errors: in strict mode, raise `OSError`, and
in non-strict mode, do not raise any exception.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants