classification
Title: shutil.copytree doesn't give control over directory permissions
Type: enhancement Stage: resolved
Components: Library (Lib), Tests Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ajaksu2, catherinedevlin, christian.heimes, daniel.ugra, eric.araujo, georg.brandl, jlgijsbers, mcjeff, pitrou, python-dev, tarek, thomaswaldmann, vajrasky
Priority: normal Keywords: easy, patch

Created on 2007-02-22 16:26 by j_mcneil, last changed 2013-08-16 17:38 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
makedirs_function.patch mcjeff, 2012-03-13 08:36 review
test1666318.patch catherinedevlin, 2013-07-29 20:35 patch to test_shutil.py testing for copytree preserving permissions review
test1666318.patch catherinedevlin, 2013-08-16 16:38 updated patch review
Messages (16)
msg31307 - (view) Author: Jeff McNeil (j_mcneil) Date: 2007-02-22 16:26
I am using shutil.copytree to setup new user home directories within an automated system.  The copy2 function is called in order to copy individual files and preserve stat data. 

However, copytree simply calls os.mkdir and leaves directory creation at the mercy of my current umask (in my case, that's daemon context - 0).

I've got to then iterate through the newly copied tree and set permissions on each individual subdirectory. 

Adding a simple copystat(src, dst) on line 112 of shutil.py fixes the problem. 

The result should be uniform; either preserve permissions across the board, or leave it to the mercy of the caller.  I know there's an enhancement request already open to supply a 'func=' kw argument to copytree.

msg31308 - (view) Author: Jeff McNeil (j_mcneil) Date: 2007-02-22 16:28
python -V
Python 2.4.3

on 

Linux marvin 2.6.18-1.2257.fc5smp #1 SMP Fri Dec 15 16:33:51 EST 2006 i686 i686 i386 GNU/Linux
msg31309 - (view) Author: Thomas Waldmann (thomaswaldmann) Date: 2007-03-24 18:07
I can confirm this bug.

For MoinMoin wiki, we solved that by duplicating this function to our own tree and we did exactly the proposed change:
 
Adding a simple copystat(src, dst) on line 112 of shutil.py (right below os.mkdir) fixes this problem.

I also suggest to remove that XXX comment in that function. With the proposed fix, it gets usable and thus is not only example code any more.

BUT: for avoiding problems on win32, a second fix needs to be done. For MoinMoin, we call this wrapper around shutil.copystat from our copytree function:

def copystat(src, dst):
    """Copy stat bits from src to dst

    This should be used when shutil.copystat would be used on directories
    on win32 because win32 does not support utime() for directories.

    According to the official docs written by Microsoft, it returns ENOACCES if the
    supplied filename is a directory. Looks like a trainee implemented the function.
    """
    if sys.platform == 'win32' and S_ISDIR(os.stat(dst)[ST_MODE]):
        if os.name == 'nt':
            st = os.stat(src)
            mode = S_IMODE(st[ST_MODE])
            if hasattr(os, 'chmod'):
                os.chmod(dst, mode) # KEEP THIS ONE!
        #else: pass # we are on Win9x,ME - no chmod here
    else:
        shutil.copystat(src, dst)

As you see, some special treatment is needed for win32/nt and directories - and directories are exactly the use case used by the fixed copytree, so a fixed copystat is needed or copytree will crash on win32/nt.
msg141822 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-09 14:58
> The result should be uniform; either preserve permissions across the
> board, or leave it to the mercy of the caller.
I’m surprised by this report, as the code does a copystat(sourcedir, targetdir) since 2004 (#1048878).  Anyhow, I think the second option would be better.

>  I know there's an enhancement request already open to supply a
> 'func=' kw argument to copytree.
This is now implemented (copy_function argument), but does not help with directories, only with files.  Recently I needed to call copytree and change the permissions of the destination, so I wanted to use a two-line custom copy_function (one line to call shutil.copy, one line for os.chmod), but I hadn’t understood that the custom function was not used for directory creation.  I found out that shutil does a copystat on the created directories behind my back.  So, I think this problem can be solved in a nice and b/w-compatible way by adding a new keyword argument, makedirs_func.  I would like to fix this in stable versions too, but if another core developer argues that it should be considered a new feature, I won’t argue.
msg155580 - (view) Author: Jeff McNeil (mcjeff) * Date: 2012-03-13 08:36
I made the change suggested in the last comment, patch is attached. Trying to clean up any bugs I've got my name on!
msg155613 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-03-13 14:07
Patch looks good.

I don’t know if I should fix this in 3.3 with your patch or go back to the first idea of adding a copystat(src, dst) after the mkdir call.  I just don’t know if it’s important that this behavior does not change in stable versions, or if it’s clearly a bug and we can change it.
msg155628 - (view) Author: Jeff McNeil (mcjeff) * Date: 2012-03-13 16:20
Ah, understood. I kind of like the idea of having the added functionality behind a custom callable, but if it's generally just a bug then copystat is a good solution, too.
msg193900 - (view) Author: Catherine Devlin (catherinedevlin) Date: 2013-07-29 20:35
Attaching a test that verifies that shutil.copytree is, in fact, preserving permissions (as of Python 3.4.0a0 (default:66a3dc613627, Jul 27 2013, 21:23:10) )

As far as I can tell this can be closed.
msg193904 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-29 20:52
Catherine, I think it would be nice to apply your patch to avoid any further regressions. However, you should first sign a contributor's agreement (http://www.python.org/psf/contrib/). Could you please do so and post here once it is done?
msg194025 - (view) Author: Catherine Devlin (catherinedevlin) Date: 2013-07-31 22:21
Thanks, Antoine - I've signed and submitted the Contributor's Agreement.
msg194028 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-08-01 00:11
Hi Catherine,

the contributor agreement must go through proper channels. The bug tracker is not the right place to post your agreement. Please follow the instructions at http://www.python.org/psf/contrib/ . You can submit the agreement electronically, too.
msg194031 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-08-01 04:35
Hi Catherine,

I saw your patch. It looks good, except you are trying to copy the directory onto its subdirectory.

src_dir = tempfile.mkdtemp()
dst_dir = os.path.join(tempfile.mkdtemp(), 'destination')

I prefer it if you copy it to somewhere else. This would be better:

tmp_dir = self.mkdtemp()
src = os.path.join(tmp_dir, 'foo')
dst = os.path.join(tmp_dir, 'bar')
msg194981 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-12 18:23
Hey Catherine,
Do you want to update your patch to include Vajrasky's suggestion?
msg195355 - (view) Author: Catherine Devlin (catherinedevlin) Date: 2013-08-16 16:38
Attaching new patch incorporating Vajrasky's suggestion
msg195374 - (view) Author: Roundup Robot (python-dev) Date: 2013-08-16 17:36
New changeset c388e93879c4 by Antoine Pitrou in branch '3.3':
Issue #1666318: Add a test that shutil.copytree() retains directory permissions.
http://hg.python.org/cpython/rev/c388e93879c4

New changeset 8906713d5704 by Antoine Pitrou in branch 'default':
Issue #1666318: Add a test that shutil.copytree() retains directory permissions.
http://hg.python.org/cpython/rev/8906713d5704
msg195376 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-08-16 17:38
Patch committed to 3.3 and 3.4. Thank you very much!
History
Date User Action Args
2013-08-16 17:38:07pitrousetstatus: open -> closed
versions: + Python 3.3
messages: + msg195376

resolution: fixed
stage: patch review -> resolved
2013-08-16 17:36:30python-devsetnosy: + python-dev
messages: + msg195374
2013-08-16 16:38:47catherinedevlinsetfiles: + test1666318.patch

messages: + msg195355
2013-08-12 18:23:30pitrousetmessages: + msg194981
2013-08-01 04:35:31vajraskysetnosy: + vajrasky
messages: + msg194031
2013-08-01 00:11:48christian.heimessetfiles: - Python Contributor Agreement Form - signed.pdf
2013-08-01 00:11:39christian.heimessetnosy: + christian.heimes
messages: + msg194028
2013-07-31 22:21:51catherinedevlinsetfiles: + Python Contributor Agreement Form - signed.pdf

messages: + msg194025
2013-07-29 20:52:06pitrousetassignee: tarek ->
type: behavior -> enhancement
components: + Tests
versions: + Python 3.4, - Python 2.7, Python 3.2, Python 3.3
nosy: + pitrou

messages: + msg193904
stage: test needed -> patch review
2013-07-29 20:35:14catherinedevlinsetfiles: + test1666318.patch
nosy: + catherinedevlin
messages: + msg193900

2012-03-13 16:20:46mcjeffsetmessages: + msg155628
2012-03-13 14:07:37eric.araujosetmessages: + msg155613
2012-03-13 08:36:02mcjeffsetfiles: + makedirs_function.patch
keywords: + patch
messages: + msg155580
2011-08-24 13:10:03daniel.ugrasetnosy: + daniel.ugra
2011-08-09 14:58:26eric.araujosetnosy: + eric.araujo, jlgijsbers, georg.brandl
title: shutil.copytree doesn't preserve directory permissions -> shutil.copytree doesn't give control over directory permissions
messages: + msg141822

versions: + Python 3.3, - Python 3.1
2010-08-21 12:16:12BreamoreBoysetversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.6, Python 3.0
2010-04-28 05:00:39mcjeffsetnosy: + mcjeff, - j_mcneil
2010-04-20 09:32:30tareksetassignee: tarek

nosy: + tarek
2009-04-22 14:37:18ajaksu2setkeywords: + easy
2009-03-20 21:43:49ajaksu2linkissue1145257 dependencies
2009-03-20 21:43:02ajaksu2setnosy: + ajaksu2
versions: + Python 2.6, Python 3.0

type: behavior
stage: test needed
2007-02-22 16:26:35j_mcneilcreate