bpo-38811: Check for presence of os.link method in pathlib

[tohojo]

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.

Fix this by checking for the presence of the 'link' method on pathlib
module import, and if it's not present, turn it into a runtime error like
those emitted when there is no lchmod() or symlink().

Signed-off-by: Toke Høiland-Jørgensen [email protected]

https://bugs.python.org/issue38811

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@tohojo

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

[tohojo]

I assume Red Hat already signed the CLA. How do I go about having that registered in the system?

[cool-RR]

Looks good to me.

[cool-RR]

Is there a way to add testing for this code?

[cool-RR]

Out of curiosity, which system did you try it on that didn't have link?

[tohojo]

Ram Rachum <[email protected]> writes:

Is there a way to add testing for this code?

Dunno? Looking at the other methods that are conditional in pathlib, I'm not so sure: symlink just has a "skip_unless_symlink" decorator, and lchmod() has no test at all.

[cool-RR]

Dunno? Looking at the other methods that are conditional in pathlib, I'm
not so sure: symlink just has a "skip_unless_symlink" decorator, and
lchmod() has no test at all.

I understand, so there's some precedent for leaving it untested, though it wouldn't be a bad idea to add testing both for this feature and the other methods, both in cases where they're supported and not.

[tohojo]

Ram Rachum <[email protected]> writes:

Out of curiosity, which system did you try it on that didn't have `link`?

Whatever Travis CI is using: https://travis-ci.org/tohojo/flent/jobs/611950249

[tohojo]

Ram Rachum <[email protected]> writes:

> Dunno? Looking at the other methods that are conditional in pathlib, I'm > not so sure: symlink just has a "skip_unless_symlink" decorator, and > lchmod() has no test at all. I understand, so there's some precedent for leaving it untested, though it wouldn't be a bad idea to add testing both for this feature and the other methods, both in cases where they're supported and not.

Sure, more tests are always better :) But how? The pathlib definition is done at import, so it's not quite as simple as simply mock()'ing out the link method in os, is it? It would take quite some refactoring of all the pathlib tests, no?

[cool-RR]

Whatever Travis CI is using: https://travis-ci.org/tohojo/flent/jobs/611950249

That is curious indeed, especially since Linux is mentioned there. @bittner do you have an idea which OS Travis is running on?

But how? The pathlib definition is done at import, so it's not quite as
simple as simply mock()'ing out the link method in os, is it? It would
take quite some refactoring of all the pathlib tests, no?

I agree it's difficult and you don't have to take it on. One simple idea would be to run a test only in cases where os.link isn't defined, and then have that test call Path.link_to and expect the NotImplementedError.

[tohojo]

Ram Rachum <[email protected]> writes:

I agree it's difficult and you don't have to take it on. One simple idea would be to run a test only in cases where `os.link` isn't defined, and then have that test call `Path.link_to` and expect the `NotImplementedError`.

That I can certainly do :)

[brandtbucher]

Looks good, just a bit of cleanup here:

[brandtbucher]

Also, the CLA is linked to your BPO account, so you'll need to follow the bot's instructions to sign it.

[brandtbucher]

I wonder if Travis intentionally disables symlinks, as a security measure.

[brandtbucher]

Also, welcome to CPython @tohojo!

[tohojo]

Thanks! I'll see if I can figure out the correct way to handle the CLA :)

[brandtbucher]

Here's the link again.

[tohojo]

Yeah, the trouble with that is that I can't really sign that since my corporate overlord already owns the rights to everything I do; so they will have to sign it. "They" being Red Hat, I'm assuming they already did, I just need to figure out how to get that into the PSF system. I seem to recall there being something about this on our intranet, but that is just giving me "service unavailable" errors right now, so I guess I'll have to try again a bit later :)

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[tohojo]

Oh, BTW, is there an automatic process to get this into 3.8.1, or should I open another pull request against the 3.8 branch?

[corona10]

@tohojo I add backport label for 3.8. Bot will auto-create the PR for 3.8!

[tohojo]

Dong-hee Na <[email protected]> writes:

@tohojo I add backport label for 3.8. Bot will auto-create the PR for 3.8!

Great, thanks!

[miss-islington]

Thanks @tohojo for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

[bedevere-bot]

GH-17204 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

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

Hi! The buildbot AMD64 Windows7 SP1 3.x has failed when building commit 111772f.

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/40/builds/3413) and take a look at the build logs.
4. Check if the failure is related to this commit (111772f) 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/40/builds/3413

Failed tests:

• test_pathlib
• test_asyncio

Failed subtests:

• test_symlink_to_not_implemented - test.test_pathlib.PathTest
• test_huge_content - test.test_asyncio.test_sock_lowlevel.SelectEventLoopTests
• test_symlink_to_not_implemented - test.test_pathlib.WindowsPathTest

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

== Tests result: FAILURE then FAILURE ==

385 tests OK.

10 slowest tests:

• test_multiprocessing_spawn: 2 min 54 sec
• test_tools: 2 min
• test_tokenize: 1 min 53 sec
• test_concurrent_futures: 1 min 35 sec
• test_capi: 1 min 28 sec
• test_mmap: 1 min 20 sec
• test_lib2to3: 1 min 17 sec
• test_asyncio: 1 min 11 sec
• test_unicodedata: 1 min 4 sec
• test_io: 57.6 sec

1 test failed:
test_pathlib

33 tests skipped:
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll
test_fcntl test_fork1 test_gdb test_grp test_ioctl test_kqueue
test_multiprocessing_fork test_multiprocessing_forkserver test_nis
test_openpty test_ossaudiodev test_pipes test_poll test_posix
test_pty test_pwd test_readline test_resource test_spwd
test_syslog test_threadsignals test_tix test_tk test_ttk_guionly
test_wait3 test_wait4 test_xxtestfuzz test_zipfile64

2 re-run tests:
test_asyncio test_pathlib

Total duration: 11 min 29 sec

Click to see traceback logs

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\proactor_events.py", line 768, in _loop_self_reading
    f.result()  # may raise
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 66, in _cancel_overlapped
    self._ov.cancel()
OSError: [WinError 6] The handle is invalid


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\proactor_events.py", line 768, in _loop_self_reading
    f.result()  # may raise
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 808, in _poll
    value = callback(transferred, key, ov)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 457, in finish_recv
    raise ConnectionResetError(*exc.args)
ConnectionResetError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request
k


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 170, in test_huge_content
    self.loop.run_until_complete(
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\base_events.py", line 634, in run_until_complete
    return future.result()
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_asyncio\test_sock_lowlevel.py", line 157, in _basetest_huge_content
    data = await self.loop.sock_recv(sock, DATA_SIZE)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\selector_events.py", line 362, in sock_recv
    return await fut
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\selector_events.py", line 373, in _sock_recv
    data = sock.recv(n)
ConnectionResetError: [WinError 10054] An existing connection was forcibly closed by the remote host


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 453, in finish_recv
    return ov.getresult()
OSError: [WinError 995] The I/O operation has been aborted because of either a thread exit or an application request


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\asyncio\windows_events.py", line 66, in _cancel_overlapped
    self._ov.cancel()
OSError: [WinError 6] The handle is invalid
k


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_pathlib.py", line 2031, in test_symlink_to_not_implemented
    link.symlink_to(target)
  File "C:\buildbot.python.org\3.x.kloth-win64\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: 'C:\\buildbot.python.org\\3.x.kloth-win64\\build\\build\\test_python_1660\\@test_1660_tmp\\fileA' -> 'C:\\buildbot.python.org\\3.x.kloth-win64\\build\\build\\test_python_1660\\@test_1660_tmp\\dirA\\linkAA'


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_pathlib.py", line 2031, in test_symlink_to_not_implemented
    link.symlink_to(target)
  File "C:\buildbot.python.org\3.x.kloth-win64\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: 'C:\\buildbot.python.org\\3.x.kloth-win64\\build\\build\\test_python_1660\\test_python_worker_6236\\@test_6236_tmp\\fileA' -> 'C:\\buildbot.python.org\\3.x.kloth-win64\\build\\build\\test_python_1660\\test_python_worker_6236\\@test_6236_tmp\\dirA\\linkAA'

[brandtbucher]

@serhiy-storchaka This was merged without a signed CLA...

[bedevere-bot]

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

Hi! The buildbot x86 Windows7 3.x has failed when building commit 111772f.

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/58/builds/3257) and take a look at the build logs.
4. Check if the failure is related to this commit (111772f) 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/58/builds/3257

Failed tests:

• test_pathlib

Failed subtests:

• test_symlink_to_not_implemented - test.test_pathlib.PathTest
• test_symlink_to_not_implemented - test.test_pathlib.WindowsPathTest

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

== Tests result: FAILURE then FAILURE ==

388 tests OK.

10 slowest tests:

• test_multiprocessing_spawn: 20 min 27 sec
• test_zipfile: 13 min
• test_concurrent_futures: 8 min 29 sec
• test_regrtest: 6 min 26 sec
• test_tarfile: 5 min 19 sec
• test_asyncio: 5 min 12 sec
• test_pickle: 4 min 11 sec
• test_capi: 4 min 4 sec
• test_venv: 3 min 57 sec
• test_distutils: 3 min 54 sec

1 test failed:
test_pathlib

30 tests skipped:
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll
test_fcntl test_fork1 test_gdb test_grp test_ioctl test_kqueue
test_multiprocessing_fork test_multiprocessing_forkserver test_nis
test_openpty test_ossaudiodev test_pipes test_poll test_posix
test_pty test_pwd test_readline test_resource test_spwd
test_syslog test_threadsignals test_wait3 test_wait4
test_xxtestfuzz test_zipfile64

1 re-run test:
test_pathlib

Total duration: 1 hour 32 min

Click to see traceback logs

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_848_tmp\\fileA' -> 'D:\\cygwin\\home\\db3l\\buildarea\\3.x.bolen-windows7\\build\\build\\test_python_848\\@test_848_tmp\\dirA\\linkAA'


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'