Add symlink support to shutil functions

[akheron]

BPO	12715
Nosy	@pitrou, @vstinner, @tarekziade, @merwok, @socketpair, @akheron, @hynek
Files	e126ceae5ba9.diff c92c4d936255.diff 4832bf7aa028.diff d9212bb67f64.diff 23e6204efe20.diff 8330f2045f4d.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2011-12-29.17:55:47.223>
created_at = <Date 2011-08-09.12:06:22.071>
labels = ['type-feature', 'library']
title = 'Add symlink support to shutil functions'
updated_at = <Date 2011-12-29.17:55:47.222>
user = 'https://github.com/akheron'

bugs.python.org fields:

activity = <Date 2011-12-29.17:55:47.222>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2011-12-29.17:55:47.223>
closer = 'pitrou'
components = ['Library (Lib)']
creation = <Date 2011-08-09.12:06:22.071>
creator = 'petri.lehtinen'
dependencies = []
files = ['23028', '24067', '24069', '24070', '24099', '24102']
hgrepos = ['63']
issue_num = 12715
keywords = ['patch']
message_count = 18.0
messages = ['141813', '141817', '141818', '141819', '141821', '141876', '141966', '142608', '142890', '149986', '149987', '150014', '150027', '150311', '150326', '150333', '150338', '150339']
nosy_count = 9.0
nosy_names = ['pitrou', 'vstinner', 'tarek', 'eric.araujo', 'neologix', 'socketpair', 'python-dev', 'petri.lehtinen', 'hynek']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue12715'
versions = ['Python 3.3']

[akheron]

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.

[akheron]

We could also have symlinks='require' to have copymode() and copystat() raise an error unless both src and dst are symlinks.

[hynek]

Additionally copyfile() might be fixed to understand symlinks=True too.

Currently working on a patch for all 5 of them.

[merwok]

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.

[akheron]

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.

[hynek]

JFTR, my implementation is ready, but I can't/don't want to start writing tests, as long as bpo-12721 is open.

[merwok]

Other reports related to shutil and symlinks: bpo-9993, bpo-4489, bpo-12461.

[hynek]

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).

[hynek]

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.

[hynek]

I have updated the patch to latest default/tip and would appreciate a review.

[pitrou]

I've done a review at http://bugs.python.org/review/12715

[hynek]

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.

[hynek]

The patch contains also the necessary changes for the docs now.

Please let me know, if I can do anything else.

[hynek]

Added another patch fixing the issues pointed out by Antoine and Charles-François.

[hynek]

Added new patch with protection for the remaining UF_NODUMPs in the test case. All raised issues should be fixed now. :)

[pitrou]

I've checked the latest patch to work fine under Windows 7 (and Linux, of course).

[python-dev]

New changeset cf57ef65bcd0 by Antoine Pitrou in branch 'default':
Issue bpo-12715: Add an optional symlinks argument to shutil functions (copyfile, copymode, copystat, copy, copy2).
http://hg.python.org/cpython/rev/cf57ef65bcd0

[pitrou]

Patch is now applied to 3.3, thank you for your patience :)