classification
Title: Add symlink support to shutil functions
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, haypo, hynek, mmarkk, neologix, petri.lehtinen, pitrou, python-dev, tarek
Priority: normal Keywords: patch

Created on 2011-08-09 12:06 by petri.lehtinen, last changed 2011-12-29 17:55 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
e126ceae5ba9.diff hynek, 2011-08-24 17:52 review
c92c4d936255.diff hynek, 2011-12-21 13:31 review
4832bf7aa028.diff hynek, 2011-12-21 16:55 review
d9212bb67f64.diff hynek, 2011-12-21 18:07 review
23e6204efe20.diff hynek, 2011-12-28 18:29 review
8330f2045f4d.diff hynek, 2011-12-29 10:45 review
Repositories containing patches
http://bitbucket.org/hynek/cpython-shutil-symlinks#shutil-symlinks
Messages (18)
msg141813 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-08-09 12:06
The shutil.copytree() function aknowledges symlinks when given
the symlink=True argument. Other shutil functions should do this
too:

copymode(src, dst, symlinks=True)
   If both src and dst are symlinks, copy the source symlink mode
   to the destination symlink. If only one of them is a symlink,
   dereference it normally.

copystat(src, dst, symlinks=True)
   If both src and dst are symlinks, copy the statinfo of the
   source symlink to the destination symlink. If only one of them
   is a symlink, dereference it normally.

copy(src, dst, symlinks=True)
   If the src argument is a symlink, copy the source symlink
   instead of dereferencing it.

   Problem: What if dst is a symlink to a directory? Should it be
   replaced or should the symlink copied inside that directory?

copy2(src, dst, symlinks=True)
   Like copy(src, dst, symlinks=True), but copy the metadata too.
   The same problem applies.
msg141817 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-08-09 12:55
We could also have symlinks='require' to have copymode() and copystat() raise an error unless both src and dst are symlinks.
msg141818 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-08-09 14:21
Additionally copyfile() might be fixed to understand symlinks=True too.

Currently working on a patch for all 5 of them.
msg141819 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-09 14:24
> Problem: What if dst is a symlink to a directory? Should it be
> replaced or should the symlink copied inside that directory?

Doing what UNIX’ cp program does would be best here.
msg141821 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2011-08-09 14:50
> Doing what UNIX’ cp program does would be best here.

$ cp --version
cp (GNU coreutils) 8.5

$ touch file
$ ln -s file link_to_file 
$ mkdir directory
$ ln -s directory link_to_directory
$ cp -a link_to_file link_to_directory
$ ls -l directory
lrwxrwxrwx 1 user user 4 2011-08-09 17:46 link_to_file -> file

So at least cp from GNU coreutils seems to copy the link into the directory, dereferencing the dst link.
msg141876 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-08-10 17:25
JFTR, my implementation is ready, but I can't/don't want to start writing tests, as long as #12721 is open.
msg141966 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-12 16:09
Other reports related to shutil and symlinks: #9993, #4489, #12461.
msg142608 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-08-21 10:56
We have an edge case when the caller calls copymode with symlinks=True on an OS that doesn't have lchmod (ie any other than FreeBSD).

Should it be a nop (as in copystat), use chmod or raise an Exception?

I Personally (and some people on #python-dev) believe it should raise an Exception as the caller made the effort to say that (s)he wants the link _not_ to be followed and it's just one operation (contrary to copystat that does several things).
msg142890 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-08-24 17:51
What started as a 2 lines fix, grew to a patch of ~400 lines. :)

It's mostly tests though:

Lib/shutil.py           |  102 +++++++++++++++++++++++++++++----------
Lib/test/test_shutil.py |  220 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Misc/ACKS               |    1 +
3 files changed, 297 insertions(+), 26 deletions(-)

I've AFAIS implemented the desired behavior and hope for feedback/bug reports.

It's tested on Linux & FreeBSD 8.2 and applies cleanly against default/tip.

Please let me know if I've done something stupid.
msg149986 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-12-21 13:36
I have updated the patch to latest default/tip and would appreciate a review.
msg149987 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-21 13:45
I've done a review at http://bugs.python.org/review/12715
msg150014 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-12-21 16:57
Thanks to Antoine & Charles-François for their reviews! I think I've incorporated all desired changes.

Please let me know if I have missed something, meanwhile I'll tackle the docs.
msg150027 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-12-21 18:09
The patch contains also the necessary changes for the docs now.

Please let me know, if I can do anything else.
msg150311 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-12-28 18:50
Added another patch fixing the issues pointed out by Antoine and Charles-François.
msg150326 - (view) Author: Hynek Schlawack (hynek) * (Python committer) Date: 2011-12-29 10:47
Added new patch with protection for the remaining UF_NODUMPs in the test case. All raised issues should be fixed now. :)
msg150333 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-29 16:01
I've checked the latest patch to work fine under Windows 7 (and Linux, of course).
msg150338 - (view) Author: Roundup Robot (python-dev) Date: 2011-12-29 17:55
New changeset cf57ef65bcd0 by Antoine Pitrou in branch 'default':
Issue #12715: Add an optional symlinks argument to shutil functions (copyfile, copymode, copystat, copy, copy2).
http://hg.python.org/cpython/rev/cf57ef65bcd0
msg150339 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-12-29 17:55
Patch is now applied to 3.3, thank you for your patience :)
History
Date User Action Args
2012-02-22 20:08:56petri.lehtinenlinkissue12702 superseder
2011-12-29 17:55:47pitrousetstatus: open -> closed
resolution: fixed
messages: + msg150339

stage: commit review -> resolved
2011-12-29 17:55:13python-devsetnosy: + python-dev
messages: + msg150338
2011-12-29 16:01:16pitrousetmessages: + msg150333
stage: commit review
2011-12-29 10:47:32hyneksetmessages: + msg150326
2011-12-29 10:45:59hyneksetfiles: + 8330f2045f4d.diff
2011-12-28 18:50:11hyneksetmessages: + msg150311
2011-12-28 18:29:31hyneksetfiles: + 23e6204efe20.diff
2011-12-21 18:09:34hyneksetmessages: + msg150027
2011-12-21 18:07:58hyneksetfiles: + d9212bb67f64.diff
2011-12-21 16:57:08hyneksetmessages: + msg150014
2011-12-21 16:55:08hyneksetfiles: + 4832bf7aa028.diff
2011-12-21 13:45:30pitrousetnosy: + pitrou
messages: + msg149987
2011-12-21 13:36:07hyneksetmessages: + msg149986
2011-12-21 13:31:50hyneksetfiles: + c92c4d936255.diff
2011-08-24 18:14:22pitrousetnosy: + haypo, neologix
2011-08-24 17:52:03hyneksetfiles: + e126ceae5ba9.diff
keywords: + patch
2011-08-24 17:51:23hyneksethgrepos: + hgrepo63
messages: + msg142890
2011-08-22 02:53:25mmarkksetnosy: + mmarkk
2011-08-21 10:56:26hyneksetmessages: + msg142608
2011-08-21 10:21:11eric.araujolinkissue12461 superseder
2011-08-12 16:09:55eric.araujosetmessages: + msg141966
2011-08-10 17:25:07hyneksetmessages: + msg141876
2011-08-09 14:50:39petri.lehtinensetmessages: + msg141821
2011-08-09 14:24:03eric.araujosetnosy: + eric.araujo
messages: + msg141819
2011-08-09 14:21:31hyneksetmessages: + msg141818
2011-08-09 12:55:51petri.lehtinensetmessages: + msg141817
2011-08-09 12:18:26petri.lehtinenlinkissue12702 dependencies
2011-08-09 12:06:22petri.lehtinencreate