[PATCH 4 of 7] util.copyfiles: do not try to create destination folders which already exist

[Gregory Szorc]

On Tue, Nov 24, 2015 at 4:22 PM, Angel Ezquerra <angel.ezquerra at gmail.com>
wrote:

> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1448320493 -3600
> #      Tue Nov 24 00:14:53 2015 +0100
> # Node ID f275c1ff2148e806e45ea308139b0ff28b74b270
> # Parent  fd4449f9e1239bc269e7a842168ace286ee4dd9e
> util.copyfiles: do not try to create destination folders which already
> exist
>
> Before this the behavior of util.copyfiles was inconsistent. It would
> silently
> overwrite existing files, but it would balk at copying over an existing
> folder.
> It seems to make more sense to not try to create existing directories (but
> copy
> their contents anyway). We will use this new behavior to unshare full
> repository
> shares.
>
> This change is potentially dangerous since it could change the behavior of
> users
> of util.copyfiles. Before this change trying to copy into an existing
> directory
> would result in an error, whereas now it will not. This seems to be a
> better
> behavior, and the test suite works fine so it does not seem to be a
> problem in
> practice.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -812,7 +812,8 @@
>          topic = _('copying')
>
>      if os.path.isdir(src):
> -        os.mkdir(dst)
> +        if not os.path.exists(dst):
> +            os.mkdir(dst)
>

This is an anti-pattern because it performs 2 filesystem calls. The proper
way to do this is to attempt to mkdir() and catch failures due to
error.EEXIST:

try:
    os.mkdir(p)
except OSError as e:
    if e.errno != errno.EEXIST:
        raise

If this doesn't exist as a utility function in util.py or scmutil.py, IMO
it should be made to exist.


>          for name, kind in osutil.listdir(src):
>              srcname = os.path.join(src, name)
>              dstname = os.path.join(dst, name)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151128/74b60e09/attachment.html>