[PATCH 1 of 2 v3] util: add progress callback support to copyfiles

Martin von Zweigbergk martinvonz at google.com
Tue Mar 24 16:19:10 CDT 2015


On Tue, Mar 24, 2015 at 2:13 PM Augie Fackler <raf at durin42.com> wrote:

> On Tue, Mar 24, 2015 at 5:08 PM, Adrian Buehlmann <adrian at cadifra.com>
> wrote:
> >> 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 = util.copyfiles(
> >> +                    srcvfs.join(f), dstvfs.join(f), hardlink)
> >>                  num += n
> >>          if hardlink:
> >>              ui.debug("linked %d files\n" % num)
> >
> > This looks like a simple reformatting. Was that intended?
>
>
> Ugh, no. I'll roll a v4. Good catch.
>

If you're rolling a v4 anyway:

* The 'num' didn't seem to need to move, so revert that?
* Sorry I didn't realize before, but wouldn't it be better to pass just a
boolean value (True for hardlink) to the progress callback and keep all the
debug messages out of copyfiles()?
* Does it make sense to move the "linked %d files\n" messages to the "if
pos is None" case? inside the callback? I'm not sure, just a thought. That
would remove the need for the 'closetopic' container.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150324/83685d6f/attachment.html>


More information about the Mercurial-devel mailing list