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

login
register
mail settings
Submitter Matt Harbison
Date Jan. 6, 2013, 10:16 p.m.
Message ID <06f51c9f9a9b5c47b9d7.1357510599@Envy>
Download mbox | patch
Permalink /patch/404/
State Changes Requested
Headers show

Comments

Matt Harbison - Jan. 6, 2013, 10:16 p.m.
# 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.
Mads Kiilerich - Jan. 10, 2013, 2:26 p.m.
On 01/06/2013 11:16 PM, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@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?

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

> -    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
Matt Harbison - Jan. 11, 2013, 4:08 a.m.
Mads Kiilerich wrote:
> On 01/06/2013 11:16 PM, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@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
>
Mads Kiilerich - Jan. 11, 2013, 11:55 p.m.
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@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.

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
>>
>
tonfa - Jan. 21, 2013, 10:50 p.m.
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@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@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@selenic.com
> http://selenic.com/mailman/**listinfo/mercurial-devel<http://selenic.com/mailman/listinfo/mercurial-devel>
>
Matt Harbison - Jan. 22, 2013, 12:49 a.m.
Benoit Boissinot wrote:
> FYI: I have a minimal fix for this, partly based on patch 2 from Matt,
> which should be suitable for the freeze.

Feel free to send.  I didn't mean to abandon this, but got busy for a 
few days and then sick, and it may be another day or two before I get 
back to this.  All I was going to do was the few tweaks Mads suggested 
(possibly without the clean filelist, but I wanted to dig into this 
first).  IIRC, there was something in there not suitable for stable, so 
your version might be better.

--Matt
tonfa - Jan. 22, 2013, 6:15 p.m.
On Tue, Jan 22, 2013 at 1:49 AM, Matt Harbison <matt_harbison@yahoo.com>wrote:

> Benoit Boissinot wrote:
>
>> FYI: I have a minimal fix for this, partly based on patch 2 from Matt,
>> which should be suitable for the freeze.
>>
>
> Feel free to send.  I didn't mean to abandon this, but got busy for a few
> days and then sick, and it may be another day or two before I get back to
> this.  All I was going to do was the few tweaks Mads suggested (possibly
> without the clean filelist, but I wanted to dig into this first).  IIRC,
> there was something in there not suitable for stable, so your version might
> be better.


I sent it earlier, would be nice to have a more comprehensive fix
post-freeze.

Cheers,

Benoit

Patch

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)
+
     lfcommands.updatelfiles(repo.ui, repo, filelist=filelist, printmessage=True)
-    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)
diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
--- a/tests/test-largefiles.t
+++ b/tests/test-largefiles.t
@@ -1703,6 +1703,18 @@ 
   lf_subrepo_archive/subrepo/large.txt
   lf_subrepo_archive/subrepo/normal.txt
 
+Test that largefiles in subrepos are checked out (issue3752)
+
+  $ hg clone . ../update_on_clone
+  updating to branch default
+  cloning subrepo subrepo from $TESTTMP/statusmatch/subrepo
+  getting changed largefiles
+  1 largefiles updated, 0 removed
+  5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  getting changed largefiles
+  1 largefiles updated, 0 removed
+  $ hg -R ../update_on_clone status -S
+
 Test archiving a revision that references a subrepo that is not yet
 cloned (see test-subrepo-recursion.t):