bpo-47049: Fix incorrect shutil.copytree() behaviour with symlinks #31967
Conversation
|
Something in the CLA signing check process changed (old checking service unavailable), and therefore the check fails although I signed the CLA. How can this be fixed? Do I have to do something, or does a a maintainer have to do something? |
|
I have tested with test_shutil (and the example given) and it looks fine, but it looks like there are tests (in test_shutil) which are negative due to this bug. |
|
I have added a unit test to the fix. That revealed another issue. When dangling symlinks do exist in this situation, an error was thrown, because the ignore_dangling_symlinks argument of the copytree function was not propagated to recursive calls. This is fixed by the last commit, too. |
shutil.copytree incorrectly does not copy symlink contents if called
with symlink=False and ignore_dangling_symlinks=True.
The wrong behaviour can be reproduced like this:
$ tree
.
└── a
├── a.txt
└── b
└── a.txt -> ../a.txt
$ python3 -c "import shutil;shutil.copytree('a/b', 'c', symlinks=False, ignore_dangling_symlinks=True)"
As a result directoy c will be created but it will remain empty.
Expected result is a file c/a.txt with the contents of a/b/a.txt.
Co-authored-by: Oleg Iarygin <dralife@yandex.ru>
The test creates following filesystem structure:
src_dir
├── test.txt
└── subdir
├── doesnotexist.txt -> IDONOTEXIST
└── test.txt -> ../test.txt
Then it uses copytree with symlinks=False and ignore_dangling_symlinks=True
As a result, in the destination file structure the symlink should become
a file and its contents should be the same as that of the original source
file that the symlink was pointing to. The symlink doesnotexist.txt
should be ignored, no error should occur, and no copy should appear in
the destination file tree.
The test revaled another issue with the _copytree implementation, which
was fixed, too.
4051578 to
a60f119
Compare
The unit test in the prevous commit did not test the case that failed prior to the fix, it also succeeded with Python versions containing the bug. Now it tests exaclty the same situation that is described in issue 47049.
a60f119 to
f510c51
Compare
|
@MaxwellDupre can you please have a look on this again? |
There was a problem hiding this comment.
LGTM: module and given tests run ok.
However, there is a lot of comments Lib/test/test_shutil.py esp lines 771-778. Most comment for tests are only a few lines.
You already hae ref'ed the Issue 47049 so people can go look that up if they want or check the PR.
Hence, you description lines 779 onwards could be a little shorter too.
As a result of the code review the test comments were reduced.
|
|
ping @MaxwellDupre |
@MaxwellDupre :) How about now? |
|
Better, but I cant do anymore. I have already approved and now waiting core review. |
|
The issue is still present in Python 3.12.6 and also an open case exists: #91205 (comment) |
shutil.copytree incorrectly does not copy symlink contents if called
with symlink=False and ignore_dangling_symlinks=True.
The wrong behaviour can be reproduced like this:
$ python3 -c "import shutil;shutil.copytree('a/b', 'c', symlinks=False, ignore_dangling_symlinks=True)"As a result directoy c will be created but it will remain empty.
Expected result is a file c/a.txt with the contents of a/b/a.txt.
https://bugs.python.org/issue47049