[PATCH 1 of 2] util.copyfiles: don't try os_link() again if it failed before

[Adrian Buehlmann]

On 28.05.2010 18:15, Adrian Buehlmann wrote:
> # HG changeset patch
> # User Adrian Buehlmann <adrian at cadifra.com>
> # Date 1275060514 -7200
> # Node ID 28b2dffa519466fc93bcd6fd5746ebdce5bec21a
> # Parent  b9e89fc5c7f11ab8e4a9f74b9c266d8f1d11b91f
> util.copyfiles: don't try os_link() again if it failed before
> 
> If the os_link() call on the first file in the directory fails [1],
> we switch mode to using shutil.copy() for all remaining files.
> 
> [1] happens for example on Windows for every file when cloning from a UNC
> path without specifying --pull.
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -458,7 +458,7 @@ def copyfiles(src, dst, hardlink=None):
>          for name, kind in osutil.listdir(src):
>              srcname = os.path.join(src, name)
>              dstname = os.path.join(dst, name)
> -            copyfiles(srcname, dstname, hardlink)
> +            hardlink = copyfiles(srcname, dstname, hardlink)
>      else:
>          if hardlink:
>              try:
> @@ -469,6 +469,8 @@ def copyfiles(src, dst, hardlink=None):
>          else:
>              shutil.copy(src, dst)
>  
> +    return hardlink
> +
>  class path_auditor(object):
>      '''ensure that a filesystem path contains no banned components.
>      the following properties of a path are checked:

The same patch shown with 'diff --unified 5' (might be a bit easier to
review in this form):

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -456,21 +456,23 @@ def copyfiles(src, dst, hardlink=None):
     if os.path.isdir(src):
         os.mkdir(dst)
         for name, kind in osutil.listdir(src):
             srcname = os.path.join(src, name)
             dstname = os.path.join(dst, name)
-            copyfiles(srcname, dstname, hardlink)
+            hardlink = copyfiles(srcname, dstname, hardlink)
     else:
         if hardlink:
             try:
                 os_link(src, dst)
             except (IOError, OSError):
                 hardlink = False
                 shutil.copy(src, dst)
         else:
             shutil.copy(src, dst)

+    return hardlink
+
 class path_auditor(object):
     '''ensure that a filesystem path contains no banned components.
     the following properties of a path are checked:

     - under top-level .hg


The interesting thing is: 'hardlink' was already assigned to False in
the exception case, but that assignment had no influence on control flow
of the program, so the os_link() try call was done again for every file
in the tree.