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

[Martin von Zweigbergk]

On Tue, Mar 24, 2015 at 2:19 PM Martin von Zweigbergk <martinvonz at google.com>
wrote:

> 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.
>
>
Never mind that last comment. I didn't realize  copyfiles() is called
several times in sequence.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150324/466e99ac/attachment.html>