add exist_ok to shutil.copytree

[thehesiod]

BPO	20849
Nosy	@ncoghlan, @giampaolo, @tarekziade, @merwok, @bitdancer, @jab, @elias6, @hynek, @thehesiod
PRs	bpo-20849: allow existing directory in shutil.copytree #2977 bpo-20849: add dirs_exist_ok to shutil.copytree #8792
Files	shutil-copytree-exist_ok.patch: Issue 20849: add exist_ok to shutil.copytree

^{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 = 'https://github.com/giampaolo'
closed_at = <Date 2018-12-28.18:05:08.388>
created_at = <Date 2014-03-04.04:55:21.975>
labels = ['type-feature', 'library']
title = 'add exist_ok to shutil.copytree'
updated_at = <Date 2018-12-28.18:05:08.387>
user = 'https://github.com/thehesiod'

bugs.python.org fields:

activity = <Date 2018-12-28.18:05:08.387>
actor = 'giampaolo.rodola'
assignee = 'giampaolo.rodola'
closed = True
closed_date = <Date 2018-12-28.18:05:08.388>
closer = 'giampaolo.rodola'
components = ['Library (Lib)']
creation = <Date 2014-03-04.04:55:21.975>
creator = 'thehesiod'
dependencies = []
files = ['34282']
hgrepos = []
issue_num = 20849
keywords = ['patch']
message_count = 14.0
messages = ['212691', '212692', '212756', '212901', '212945', '212948', '212949', '213611', '213620', '213647', '214055', '323626', '323665', '332670']
nosy_count = 10.0
nosy_names = ['ncoghlan', 'giampaolo.rodola', 'tarek', 'eric.araujo', 'r.david.murray', 'jab', 'elias', 'hynek', 'thehesiod', 'justin.myers']
pr_nums = ['2977', '8792']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20849'
versions = ['Python 3.5']

[thehesiod]

it would be REALLY nice (and REALLY easy) to add a parameter: exist_ok and pass this to os.makedirs with the same parameter name so you can use copytree to append a src dir to an existing dst dir.

[elias6]

Here is a patch that adds the option.

I am not very familiar with the internals of shutil and os so I would recommend that someone else review it.

I haven't added any tests. I can try to if anyone wants but I am not sure how long it will take me or if I will find the time any time soon.

[thehesiod]

awesome, thanks so much!!

[merwok]

Contrary to makedirs, there could be two interpretations for exist_ok in copytree: a) if a directory or file already exists in the destination, ignore it and go ahead b) only do that for directories.

The proposed patch does b), but the cp tool does a). It’s not clear to me which is best. Can you start a discussion on the python-ideas mailing list?

[thehesiod]

how about instead we rename the new parameter to dirs_exists_ok or something like that since the method already allows for existing files.

[elias6]

I am not sure. I am not on the python-ideas mailing list, and I am not sure
what adding and maintaining the discussion would entail, or if I would have
the time to do it or want to deal with the clutter in my inbox. I just
committed this patch because it seemed like it would be quick and easy.

I can start the discussion if anyone specifically wants me to, but I don't
want to let anyone down.

On Fri, Mar 7, 2014 at 1:41 PM, Éric Araujo <report@bugs.python.org> wrote:

Éric Araujo added the comment:

Contrary to makedirs, there could be two interpretations for exist_ok in
copytree: a) if a directory or file already exists in the destination,
ignore it and go ahead b) only do that for directories.

The proposed patch does b), but the cp tool does a). It's not clear to me
which is best. Can you start a discussion on the python-ideas mailing list?

----------
nosy: +eric.araujo

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue20849\>

---

[thehesiod]

I personally dont think this is worth investing the time for a discussion.
If the maintainers dont want to accept this or a minor variation without a
discussion ill just keep my local monkeypatch :) thanks again for the
quick patch Elias!
On Mar 8, 2014 4:03 PM, "Elias Zamaria" <report@bugs.python.org> wrote:

Elias Zamaria added the comment:

I am not sure. I am not on the python-ideas mailing list, and I am not sure
what adding and maintaining the discussion would entail, or if I would have
the time to do it or want to deal with the clutter in my inbox. I just
committed this patch because it seemed like it would be quick and easy.

I can start the discussion if anyone specifically wants me to, but I don't
want to let anyone down.

On Fri, Mar 7, 2014 at 1:41 PM, Éric Araujo <report@bugs.python.org>
wrote:

>
> Éric Araujo added the comment:
>
> Contrary to makedirs, there could be two interpretations for exist_ok in
> copytree: a) if a directory or file already exists in the destination,
> ignore it and go ahead b) only do that for directories.
>
> The proposed patch does b), but the cp tool does a). It's not clear to
me
> which is best. Can you start a discussion on the python-ideas mailing
list?
>
> ----------
> nosy: +eric.araujo
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue20849\>
> _______________________________________
>

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue20849\>

---

[merwok]

Adding some core devs to get their opinion on this proposal.

[bitdancer]

I don't know what "the method already allows for existing files" means. Since the target directory can't exist, there can be no existing files.

In unix, this kind of capability is provided by a combination of shell globbing and 'cp -r', and by default it does replace existing files. So it would be reasonable for exists_ok to mean exactly that: replace anything that currently exists, if it does.

I think that would be a reasonable API, but the implementation isn't as simple as just passing through the exists_ok flag to makedirs.

I do not think that *just* making it OK for the destination directory to exist would be a good API.

[thehesiod]

Ya. The original request I think is ok because by allowing that flag it
will replace files and dirs.
On Mar 14, 2014 7:15 PM, "R. David Murray" <report@bugs.python.org> wrote:

R. David Murray added the comment:

I don't know what "the method already allows for existing files" means.
Since the target directory can't exist, there can be no existing files.

In unix, this kind of capability is provided by a combination of shell
globbing and 'cp -r', and by default it does replace existing files. So it
would be reasonable for exists_ok to mean exactly that: replace anything
that currently exists, if it does.

I think that would be a reasonable API, but the implementation isn't as
simple as just passing through the exists_ok flag to makedirs.

I do not think that *just* making it OK for the destination directory to
exist would be a good API.

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue20849\>

---

[thehesiod]

btw, I believe the solution is as simple as stated as that's what I'm doing locally and its behaving exactly as intended.

[giampaolo]

I think this feature request is reasonable for 2 reasons:

1. As it stands if dst directory exists copytree() cannot be used. The only possible workaround is using rmtree(dst) first, but that doesn't seem to make much sense. The change may look extremely easy but the fact that copytree() cannot be used in a specific circumstance justifies the cost of introducing a new argument, basically because there is no other way around it.

2. "cp -r" does this by default, which is probably indicative (I don't think the default should be True though).

I think the new argument should be called "dirs_exists_ok" instead of "exists_ok" though, so that it's clear that the behavior does not affect files.

[ R. David Murray ]

I do not think that *just* making it OK for the destination directory to exist would be a good API.

I don't think that refusing (or allowing) to copy existing files is a common enough use case to justify the addition of a new parameter (note: also "cp" does not provide this use case). If such a behavior is desired the user can simply provide its own custom "copy_function".

[jab]

I submitted a new PR in #8792 that addresses the outstanding concerns in @ofekmeister's original PR. It includes passing tests and passes all the GitHub PR status checks. Does it look ok to merge? Thanks.

[giampaolo]

New changeset 9e00d9e by Giampaolo Rodola (jab) in branch 'master':
bpo-20849: add dirs_exist_ok arg to shutil.copytree (patch by Josh Bronson)
9e00d9e