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

[Matt Harbison]

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 06f51c9f9a9b5c47b9d763949f9559dafe5437a3
>> # Parent a35f6d98d65f24d5d6f1a7928342b2bc479ef44c
>> 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.  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.  The raw diff 
is noisier that what really happened.  Essentially what happened was the 
orig(...) line with the full parameter list from hgclean was brought 
over, the 3 filelist building lines in hgupdate() were conditionalized, 
and the (now gutted) hgclean was dropped.

Maybe that would have been more clear if I dropped hgclean() in a 
subsequent patch, but I'm not sure if the tests would have run properly 
without at least taking out the orig() line there.  I can try that if 
you think that's useful though.

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.

>> - 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
>