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

login
register
mail settings
Submitter Augie Fackler
Date March 14, 2015, 12:02 a.m.
Message ID <1eae0d483e9f5cf412fa.1426291325@106.17.16.172.in-addr.arpa>
Download mbox | patch
Permalink /patch/8078/
State Superseded
Headers show

Comments

Augie Fackler - March 14, 2015, 12:02 a.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1426285633 14400
#      Fri Mar 13 18:27:13 2015 -0400
# Node ID 1eae0d483e9f5cf412fa72a481fe641b78d0b248
# Parent  2b7ab29627fd93ca7f5cb838403c2f6c728469bd
util: add progress callback support to copyfiles
Martin von Zweigbergk - March 14, 2015, 4:38 a.m.
On Fri, Mar 13, 2015 at 5:03 PM Augie Fackler <raf@durin42.com> wrote:

> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1426285633 14400
> #      Fri Mar 13 18:27:13 2015 -0400
> # Node ID 1eae0d483e9f5cf412fa72a481fe641b78d0b248
> # Parent  2b7ab29627fd93ca7f5cb838403c2f6c728469bd
> util: add progress callback support to copyfiles
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -739,12 +739,16 @@ def copyfile(src, dest, hardlink=False):
>          except shutil.Error, inst:
>              raise Abort(str(inst))
>
> -def copyfiles(src, dst, hardlink=None):
> +def copyfiles(src, dst, hardlink=None, progress=lambda t, pos: None):
>      """Copy a directory tree using hardlinks if possible"""
>
>      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):
> @@ -754,6 +758,7 @@ def copyfiles(src, dst, hardlink=None):
>              dstname = os.path.join(dst, name)
>              hardlink, n = copyfiles(srcname, dstname, hardlink)
>

Should we pass a (different) callback into this one recursively? Otherwise
we're only reporting progress over number of items in the root directory,
no? I.e, if all files in the repo are in a subdirectory, would you see
progress as files are copied/linked in that directory or just one update
when it's done?

             num += n
> +            progress(topic, num)
>      else:
>          if hardlink:
>              try:
> @@ -764,6 +769,7 @@ def copyfiles(src, dst, hardlink=None):
>          else:
>              shutil.copy(src, dst)
>          num += 1
> +        progress(topic, num)
>
>      return hardlink, num
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Augie Fackler - March 16, 2015, 11:25 p.m.
On Mar 14, 2015, at 12:38 AM, Martin von Zweigbergk <martinvonz@google.com> wrote:

> 
> 
> On Fri, Mar 13, 2015 at 5:03 PM Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1426285633 14400
> #      Fri Mar 13 18:27:13 2015 -0400
> # Node ID 1eae0d483e9f5cf412fa72a481fe641b78d0b248
> # Parent  2b7ab29627fd93ca7f5cb838403c2f6c728469bd
> util: add progress callback support to copyfiles
> 
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -739,12 +739,16 @@ def copyfile(src, dest, hardlink=False):
>          except shutil.Error, inst:
>              raise Abort(str(inst))
> 
> -def copyfiles(src, dst, hardlink=None):
> +def copyfiles(src, dst, hardlink=None, progress=lambda t, pos: None):
>      """Copy a directory tree using hardlinks if possible"""
> 
>      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):
> @@ -754,6 +758,7 @@ def copyfiles(src, dst, hardlink=None):
>              dstname = os.path.join(dst, name)
>              hardlink, n = copyfiles(srcname, dstname, hardlink)
> 
> Should we pass a (different) callback into this one recursively? Otherwise we're only reporting progress over number of items in the root directory, no? I.e, if all files in the repo are in a subdirectory, would you see progress as files are copied/linked in that directory or just one update when it's done?

Yup. I’ll address this in a round two (and the i18n thing you noticed in the other one.)

> 
>              num += n
> +            progress(topic, num)
>      else:
>          if hardlink:
>              try:
> @@ -764,6 +769,7 @@ def copyfiles(src, dst, hardlink=None):
>          else:
>              shutil.copy(src, dst)
>          num += 1
> +        progress(topic, num)
> 
>      return hardlink, num
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -739,12 +739,16 @@  def copyfile(src, dest, hardlink=False):
         except shutil.Error, inst:
             raise Abort(str(inst))
 
-def copyfiles(src, dst, hardlink=None):
+def copyfiles(src, dst, hardlink=None, progress=lambda t, pos: None):
     """Copy a directory tree using hardlinks if possible"""
 
     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):
@@ -754,6 +758,7 @@  def copyfiles(src, dst, hardlink=None):
             dstname = os.path.join(dst, name)
             hardlink, n = copyfiles(srcname, dstname, hardlink)
             num += n
+            progress(topic, num)
     else:
         if hardlink:
             try:
@@ -764,6 +769,7 @@  def copyfiles(src, dst, hardlink=None):
         else:
             shutil.copy(src, dst)
         num += 1
+        progress(topic, num)
 
     return hardlink, num