| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Commit 6b5b013 ("bpo-26978: Implement pathlib.Path.link_to (Using os.link) (pythonGH-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 <toke@redhat.com>
|
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 MissingOur records indicate the following people have not signed the CLA: 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 You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
Sorry, something went wrong.
|
Still working on the CLA... expect to hear back from corporate legal today. |
Sorry, something went wrong.
|
@vstinner could you have the build-bot take a look, please? :) |
Sorry, something went wrong.
|
Custom buildbot builds are special. I prefer to wait until the "CLA signed" label appears on the PR. Ping me when it's the case. At least, ping me when "Contributor Form Received" becomes "Yes" at: https://bugs.python.org/user32968 This part is done manually by a real human :-) It may take a few days. |
Sorry, something went wrong.
|
Could someone please merge this? It's been sitting here for four weeks now... :/ |
Sorry, something went wrong.
|
I reverted the first attempt PR #17170 because the CLA was not signed and the test failed on the Windows 7 buildbot:
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'
https://bugs.python.org/issue38811#msg356858 On this PR, the CLA is signed: good, thanks @tohojo. The tests pass on Linux and Windows (the pre-commit CIs are green). I tested manually the PR on my Windows 10 VM: test_pathlib pass there as well. This new PR also changed the condition to run the test from @unittest.skipIf(support.can_symlink() to @unittest.skipIf(pathlib.supports_symlinks). support.can_symlink() creates a symbolic test and checks for error. It only returns if os.symlink() is available and works. It returns False in case of error. pathlib.supports_symlinks is true on non-Windows platforms. On Windows, it's only true if sys.getwindowsversion()[:2] >= (6, 0) which checks for Windows Vista and newer. It should be true on Windows 7. So I understand that the test is now only run on Windows XP and older, since pathlib.supports_symlinks is only false on very old unsupported Windows versions. Am I right? At least, it looks consistent with the pathlib code: if nt:
if supports_symlinks:
symlink = os.symlink
else:
def symlink(a, b, target_is_directory):
raise NotImplementedError("symlink() not available on this system")
else:
# Under POSIX, os.symlink() takes two args
@staticmethod
def symlink(a, b, target_is_directory):
return os.symlink(a, b)
Note: The original PR #17170 was approved by 3 persons (@cool-RR, @serhiy-storchaka, @brandtbucher). |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM.
I asked @tohojo to revert the os.symlink() change (it was a separated commit). To me, it's really unrelated and I'm not confident that the symlink change was correct, since it looks like dead code. It seems like Python no longer supports platforms without os.symlink().
Sorry, something went wrong.
…17225) Commit 6b5b013 ("bpo-26978: Implement pathlib.Path.link_to (Using os.link) (pythonGH-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 <toke@redhat.com> (cherry picked from commit 092435e) Co-authored-by: Toke Høiland-Jørgensen <toke@redhat.com>
|
GH-17626 is a backport of this pull request to the 3.8 branch. |
Sorry, something went wrong.
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 <toke@redhat.com> (cherry picked from commit 092435e) Co-authored-by: Toke Høiland-Jørgensen <toke@redhat.com>
…17225) Commit 6b5b013 ("bpo-26978: Implement pathlib.Path.link_to (Using os.link) (pythonGH-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 <toke@redhat.com>
| Back | FazBrowse Home | New Git URL |
This is a do-over of #17170, with a symlink test that hopefully doesn't crash on Windows.
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().
Also, add a test for the NotImplemented error raised by the corresponding symlink_to method.
https://bugs.python.org/issue38811