[PATCH 2 of 4 V2] largefiles: update largefiles in a subrepo when updating the parent (issue3752)

[Benoit Boissinot]

FYI: I have a minimal fix for this, partly based on patch 2 from Matt,
which should be suitable for the freeze.


On Sat, Jan 12, 2013 at 12:55 AM, Mads Kiilerich <mads at kiilerich.com> wrote:

> On 01/11/2013 05:08 AM, Matt Harbison wrote:
>
>> Mads Kiilerich wrote:
>>
>>> On 01/06/2013 11:16 PM, Matt Harbison wrote:
>>>
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>>> # Date 1357450348 18000
>>>> # Node ID 06f51c9f9a9b5c47b9d763949f9559**dafe5437a3
>>>> # Parent a35f6d98d65f24d5d6f1a7928342b2**bc479ef44c
>>>> largefiles: update largefiles in a subrepo when updating the parent
>>>> (issue3752)
>>>>
>>>> Starting with 17c030014ddf, largefiles in a subrepo weren't checked
>>>> out when
>>>> updating the parent repo. That cset changed hg.clean() and hg.update()
>>>> to call
>>>> the newly introduced hg.updaterepo(), and also changed hgsubrepo.get()
>>>> to call
>>>> updaterepo() directly. Since only clean and update were overridden,
>>>> largefiles
>>>> in subrepos were no longer being updated.
>>>>
>>>> Since clean() and update() delegate to updaterepo() to do their work,
>>>> the latter
>>>> is the only override required.
>>>>
>>>> Backing out 17c030014ddf and implementing hgsubrepo.get() with
>>>> hg.clean() and
>>>> hg.update() caused extra _showstats() lines to be printed, without the
>>>> context
>>>> of which (sub)repo it was related too. Multiple of these lines in a
>>>> row looks
>>>> confusing.
>>>>
>>>> Overriding hgsubrepo.get() would have made it necessary to duplicate the
>>>> largefiles handling in both clean() and update(), while adding yet
>>>> another
>>>> override.
>>>>
>>>> diff --git a/hgext/largefiles/overrides.**py
>>>> b/hgext/largefiles/overrides.**py
>>>> --- a/hgext/largefiles/overrides.**py
>>>> +++ b/hgext/largefiles/overrides.**py
>>>> @@ -647,18 +647,20 @@
>>>> finally:
>>>> wlock.release()
>>>> -def hgupdate(orig, repo, node):
>>>> - # Only call updatelfiles the standins that have changed to save time
>>>> - oldstandins = lfutil.getstandinsstate(repo)
>>>> - result = orig(repo, node)
>>>> - newstandins = lfutil.getstandinsstate(repo)
>>>> - filelist = lfutil.getlfilestoupdate(**oldstandins, newstandins)
>>>> +def hgupdaterepo(orig, repo, node, overwrite, showstats=False):
>>>> + if not overwrite:
>>>> + # Only call updatelfiles on the standins that have changed to save
>>>> time
>>>> + oldstandins = lfutil.getstandinsstate(repo)
>>>> +
>>>> + result = orig(repo, node, overwrite, showstats)
>>>> + filelist = None
>>>> +
>>>> + if not overwrite:
>>>> + newstandins = lfutil.getstandinsstate(repo)
>>>> + filelist = lfutil.getlfilestoupdate(**oldstandins, newstandins)
>>>>
>>>
>>> This function is complicated a bit by hgclean not using filelist. But
>>> isn't that a bug that it would be better to fix first? Or should it be
>>> the way it is to make clean more stable?
>>>
>>
>> I'm not sure I understand what the bug is.  It looks like
>> lfcommands.updatelfiles() always builds a full standin list, whether or not
>> one is provided to it.
>>
>
> Yes, but it is reading and hashing every largefile on 'hg up -C'. That is
> of course very safe but not very efficient. A normal 'up -C' do for
> performance reasons not do that, AFAIK. Largefiles should even less do
> that. That is of course a different problem, but it is annoying to add more
> complexity to work around / preserve that.
>
>
>    So I'm not sure if that should be duplicated, or what that buys.  I was
>> just putting the clean + update overrides together, trying to touch the
>> existing code as little as possible.
>>
>>  +
>>>> lfcommands.updatelfiles(repo.**ui, repo, filelist=filelist,
>>>> printmessage=True)
>>>>
>>>
>>> printmessage=True is the default and not specified in the other
>>> updatelfiles command you are merging here. It would be cleaner to leave
>>> it out.
>>>
>>
>> I didn't change that line- it came from overrides.hgupdate() like that,
>> so I left it alone thinking that was an extraneous change.
>>
>
> Yes, but the patch was merging two updatelfiles invocations. I just
> suggest picking the best parts from both.
>
>
>  Aside: Am I reading updatelfiles() properly?  If printmessage is False,
>> it not only doesn't print the message, but doesn't cache large files
>> either?  That seems like a problem.
>>
>
> Yeah, that looks like a bug.
>
>
>
>>  - return result
>>>> -def hgclean(orig, repo, node, show_stats=True):
>>>> - result = orig(repo, node, show_stats)
>>>> - lfcommands.updatelfiles(repo.**ui, repo)
>>>> return result
>>>> def hgmerge(orig, repo, node, force=None, remind=True):
>>>> diff --git a/hgext/largefiles/uisetup.py b/hgext/largefiles/uisetup.py
>>>> --- a/hgext/largefiles/uisetup.py
>>>> +++ b/hgext/largefiles/uisetup.py
>>>> @@ -109,11 +109,7 @@
>>>> entry = extensions.wrapfunction(**commands, 'revert',
>>>> overrides.overriderevert)
>>>> - # clone uses hg._update instead of hg.update even though they are the
>>>> - # same function... so wrap both of them)
>>>> - extensions.wrapfunction(hg, 'update', overrides.hgupdate)
>>>> - extensions.wrapfunction(hg, '_update', overrides.hgupdate)
>>>> - extensions.wrapfunction(hg, 'clean', overrides.hgclean)
>>>> + extensions.wrapfunction(hg, 'updaterepo', overrides.hgupdaterepo)
>>>> extensions.wrapfunction(hg, 'merge', overrides.hgmerge)
>>>> extensions.wrapfunction(**archival, 'archive',
>>>> overrides.overridearchive)
>>>>
>>>
>>> /Mads
>>>
>>>
>>
> ______________________________**_________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/**listinfo/mercurial-devel<http://selenic.com/mailman/listinfo/mercurial-devel>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130121/921fc798/attachment.html>