Patchwork [1,of,2,issue3059,v2] util: add progress callback support to copyfiles

login
register
mail settings
Submitter Augie Fackler
Date March 19, 2015, 5:03 p.m.
Message ID <56319892a54a26947628.1426784590@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/8171/
State Superseded
Headers show

Comments

Augie Fackler - March 19, 2015, 5:03 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1426775062 14400
#      Thu Mar 19 10:24:22 2015 -0400
# Node ID 56319892a54a2694762861f22be751fd28a25813
# Parent  5cb459dc32d209653a3e5d77749cf989ab9a51e4
util: add progress callback support to copyfiles
Martin von Zweigbergk - March 19, 2015, 5:34 p.m.
On Thu, Mar 19, 2015 at 10:07 AM Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1426775062 14400
> #      Thu Mar 19 10:24:22 2015 -0400
> # Node ID 56319892a54a2694762861f22be751fd28a25813
> # Parent  5cb459dc32d209653a3e5d77749cf989ab9a51e4
> util: add progress callback support to copyfiles
>
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -258,8 +258,8 @@ def copystore(ui, srcrepo, destpath):
>                      lockfile = os.path.join(dstbase, "lock")
>                      # lock to avoid premature writing to the target
>                      destlock = lock.lock(dstvfs, lockfile)
> -                hardlink, n = util.copyfiles(srcvfs.join(f),
> dstvfs.join(f),
> -                                             hardlink)
> +                hardlink, n, topic = util.copyfiles(
> +                    srcvfs.join(f), dstvfs.join(f), hardlink)
>                  num += n
>          if hardlink:
>              ui.debug("linked %d files\n" % num)
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -739,20 +739,36 @@ def copyfile(src, dest, hardlink=False):
>          except shutil.Error, inst:
>              raise Abort(str(inst))
>
> -def copyfiles(src, dst, hardlink=None):
> -    """Copy a directory tree using hardlinks if possible"""
> +def copyfiles(src, dst, hardlink=None,
> +              progress=lambda t, pos: None, poffset=None):
> +    """Copy a directory tree using hardlinks if possible.
> +
> +    If progress could be split across multiple copyfiles calls,
> +    poffset. Otherwise, copyfiles() will close its progress topic.
> +    """
> +    if poffset is None:
> +        closeprog = True
> +        poffset = 0
> +    else:
> +        closeprog = False
> +    num = 0
>
>      if hardlink is None:
>          hardlink = (os.stat(src).st_dev ==
>                      os.stat(os.path.dirname(dst)).st_dev)
> +    if hardlink:
> +        topic = _('linking')
> +    else:
> +        topic = _('copying')
>
> -    num = 0
>      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)
> -            hardlink, n = copyfiles(srcname, dstname, hardlink)
> +            hardlink, n, junk_topic = copyfiles(srcname, dstname,
> hardlink,
> +                                                progress=progress,
> +                                                poffset=poffset + num)
>

Dropping the poffset and closeprog stuff and passing something like the
following in the recursive call seems simpler, but I don't know if it's
noticeably slower.

def subprogress(topic, pos):
    if pos is not None:
        progress(topic, num + pos)



>              num += n
>      else:
>          if hardlink:
> @@ -764,8 +780,12 @@ def copyfiles(src, dst, hardlink=None):
>          else:
>              shutil.copy(src, dst)
>          num += 1
> +        progress(topic, poffset + num)
>
> -    return hardlink, num
> +    if closeprog:
> +        progress(topic, None)
> +
> +    return hardlink, num, topic
>
>  _winreservednames = '''con prn aux nul
>      com1 com2 com3 com4 com5 com6 com7 com8 com9
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Augie Fackler - March 24, 2015, 8:42 p.m.
On Thu, Mar 19, 2015 at 1:34 PM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
>
>
> On Thu, Mar 19, 2015 at 10:07 AM Augie Fackler <raf@durin42.com> wrote:
>>
>> # HG changeset patch
>> # User Augie Fackler <augie@google.com>
>> # Date 1426775062 14400
>> #      Thu Mar 19 10:24:22 2015 -0400
>> # Node ID 56319892a54a2694762861f22be751fd28a25813
>> # Parent  5cb459dc32d209653a3e5d77749cf989ab9a51e4
>> util: add progress callback support to copyfiles
>>
>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>> --- a/mercurial/hg.py
>> +++ b/mercurial/hg.py
>> @@ -258,8 +258,8 @@ def copystore(ui, srcrepo, destpath):
>>                      lockfile = os.path.join(dstbase, "lock")
>>                      # lock to avoid premature writing to the target
>>                      destlock = lock.lock(dstvfs, lockfile)
>> -                hardlink, n = util.copyfiles(srcvfs.join(f),
>> dstvfs.join(f),
>> -                                             hardlink)
>> +                hardlink, n, topic = util.copyfiles(
>> +                    srcvfs.join(f), dstvfs.join(f), hardlink)
>>                  num += n
>>          if hardlink:
>>              ui.debug("linked %d files\n" % num)
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -739,20 +739,36 @@ def copyfile(src, dest, hardlink=False):
>>          except shutil.Error, inst:
>>              raise Abort(str(inst))
>>
>> -def copyfiles(src, dst, hardlink=None):
>> -    """Copy a directory tree using hardlinks if possible"""
>> +def copyfiles(src, dst, hardlink=None,
>> +              progress=lambda t, pos: None, poffset=None):
>> +    """Copy a directory tree using hardlinks if possible.
>> +
>> +    If progress could be split across multiple copyfiles calls,
>> +    poffset. Otherwise, copyfiles() will close its progress topic.
>> +    """
>> +    if poffset is None:
>> +        closeprog = True
>> +        poffset = 0
>> +    else:
>> +        closeprog = False
>> +    num = 0
>>
>>      if hardlink is None:
>>          hardlink = (os.stat(src).st_dev ==
>>                      os.stat(os.path.dirname(dst)).st_dev)
>> +    if hardlink:
>> +        topic = _('linking')
>> +    else:
>> +        topic = _('copying')
>>
>> -    num = 0
>>      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)
>> -            hardlink, n = copyfiles(srcname, dstname, hardlink)
>> +            hardlink, n, junk_topic = copyfiles(srcname, dstname,
>> hardlink,
>> +                                                progress=progress,
>> +                                                poffset=poffset + num)
>
>
> Dropping the poffset and closeprog stuff and passing something like the
> following in the recursive call seems simpler, but I don't know if it's
> noticeably slower.

I like this. Sending a v3.

>
> def subprogress(topic, pos):
>     if pos is not None:
>         progress(topic, num + pos)
>
>
>>
>>              num += n
>>      else:
>>          if hardlink:
>> @@ -764,8 +780,12 @@ def copyfiles(src, dst, hardlink=None):
>>          else:
>>              shutil.copy(src, dst)
>>          num += 1
>> +        progress(topic, poffset + num)
>>
>> -    return hardlink, num
>> +    if closeprog:
>> +        progress(topic, None)
>> +
>> +    return hardlink, num, topic
>>
>>  _winreservednames = '''con prn aux nul
>>      com1 com2 com3 com4 com5 com6 com7 com8 com9
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -258,8 +258,8 @@  def copystore(ui, srcrepo, destpath):
                     lockfile = os.path.join(dstbase, "lock")
                     # lock to avoid premature writing to the target
                     destlock = lock.lock(dstvfs, lockfile)
-                hardlink, n = util.copyfiles(srcvfs.join(f), dstvfs.join(f),
-                                             hardlink)
+                hardlink, n, topic = util.copyfiles(
+                    srcvfs.join(f), dstvfs.join(f), hardlink)
                 num += n
         if hardlink:
             ui.debug("linked %d files\n" % num)
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -739,20 +739,36 @@  def copyfile(src, dest, hardlink=False):
         except shutil.Error, inst:
             raise Abort(str(inst))
 
-def copyfiles(src, dst, hardlink=None):
-    """Copy a directory tree using hardlinks if possible"""
+def copyfiles(src, dst, hardlink=None,
+              progress=lambda t, pos: None, poffset=None):
+    """Copy a directory tree using hardlinks if possible.
+
+    If progress could be split across multiple copyfiles calls,
+    poffset. Otherwise, copyfiles() will close its progress topic.
+    """
+    if poffset is None:
+        closeprog = True
+        poffset = 0
+    else:
+        closeprog = False
+    num = 0
 
     if hardlink is None:
         hardlink = (os.stat(src).st_dev ==
                     os.stat(os.path.dirname(dst)).st_dev)
+    if hardlink:
+        topic = _('linking')
+    else:
+        topic = _('copying')
 
-    num = 0
     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)
-            hardlink, n = copyfiles(srcname, dstname, hardlink)
+            hardlink, n, junk_topic = copyfiles(srcname, dstname, hardlink,
+                                                progress=progress,
+                                                poffset=poffset + num)
             num += n
     else:
         if hardlink:
@@ -764,8 +780,12 @@  def copyfiles(src, dst, hardlink=None):
         else:
             shutil.copy(src, dst)
         num += 1
+        progress(topic, poffset + num)
 
-    return hardlink, num
+    if closeprog:
+        progress(topic, None)
+
+    return hardlink, num, topic
 
 _winreservednames = '''con prn aux nul
     com1 com2 com3 com4 com5 com6 com7 com8 com9