Patchwork [4,of,4,V2] largefiles: don't print largefile changes in updaterepo() without regular stats

login
register
mail settings
Submitter Matt Harbison
Date Jan. 6, 2013, 10:16 p.m.
Message ID <e0a36600a4b62df52004.1357510601@Envy>
Download mbox | patch
Permalink /patch/405/
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 1357504524 18000
# Node ID e0a36600a4b62df52004337736409469a4700d3b
# Parent  3e9e34fd72948c311e032778c75a9fab2bb5d399
largefiles: don't print largefile changes in updaterepo() without regular stats

This eliminates standalone "getting largefiles" messages when the corresponding
updated/merged/removed/unresolved line has been explicitly suppressed.  Only
hg.clean() has the capability to suppress these messages.  Arguably the
updated..resolved line should be present in the two test cases changed here,
but at least the output is consistent for now.

Note that this also affects (the apparently largefiles untested) bisect and
backput commands, as well as hgsubrepo.remove().
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 1357504524 18000
> # Node ID e0a36600a4b62df52004337736409469a4700d3b
> # Parent  3e9e34fd72948c311e032778c75a9fab2bb5d399
> largefiles: don't print largefile changes in updaterepo() without regular stats
>
> This eliminates standalone "getting largefiles" messages when the corresponding
> updated/merged/removed/unresolved line has been explicitly suppressed.

Getting largefiles often takes some time. Both because they are big and 
because they might have to be fetched somewhere else. The "getting 
largefiles" message is thus usually useful and should usually be shown 
to inform the user what is going on.

The statistics afterwards is less useful, but also serve the purpose of 
informing the user that it is done getting largefiles ... and perhaps 
justify the time it took by showing how much it did.

So I think I'm -1 on this one.

... but what I said only applies to getting largefiles. I think it would 
make sense if updatelfiles stayed quiet on removed largefiles.

> Only hg.clean() has the capability to suppress these messages.

Just to be clear: That is behaviour that is introduced here, right?

>   Arguably the
> updated..resolved line should be present in the two test cases changed here,
> but at least the output is consistent for now.
>
> Note that this also affects (the apparently largefiles untested) bisect and
> backput commands, as well as hgsubrepo.remove().
>
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -659,7 +659,8 @@
>           newstandins = lfutil.getstandinsstate(repo)
>           filelist = lfutil.getlfilestoupdate(oldstandins, newstandins)
>   
> -    lfcommands.updatelfiles(repo.ui, repo, filelist=filelist, printmessage=True)
> +    lfcommands.updatelfiles(repo.ui, repo, filelist=filelist,
> +                            printmessage=showstats)
>   
>       return result
>   
> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
> --- a/tests/test-largefiles.t
> +++ b/tests/test-largefiles.t
> @@ -1708,8 +1708,6 @@
>     $ 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
> @@ -1723,8 +1721,6 @@
>     $ hg -R ../update_on_clone status -S
>     M subrepo/large.txt
>     $ hg -R ../update_on_clone update -C
> -  getting changed largefiles
> -  1 largefiles updated, 0 removed
>     1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>     getting changed largefiles
>     0 largefiles updated, 0 removed

These examples shows that the information is almost useless when using 
subrepos. It should show which subrepo it is visiting ... or in this 
case: inherit the quietness to the subrepo.

/Mads
Matt Harbison - Jan. 11, 2013, 5:15 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 1357504524 18000
>> # Node ID e0a36600a4b62df52004337736409469a4700d3b
>> # Parent 3e9e34fd72948c311e032778c75a9fab2bb5d399
>> largefiles: don't print largefile changes in updaterepo() without
>> regular stats
>>
>> This eliminates standalone "getting largefiles" messages when the
>> corresponding
>> updated/merged/removed/unresolved line has been explicitly suppressed.
>
> Getting largefiles often takes some time. Both because they are big and
> because they might have to be fetched somewhere else. The "getting
> largefiles" message is thus usually useful and should usually be shown
> to inform the user what is going on.
>
> The statistics afterwards is less useful, but also serve the purpose of
> informing the user that it is done getting largefiles ... and perhaps
> justify the time it took by showing how much it did.
>
> So I think I'm -1 on this one.

Good point.  I had seen extra largefile messages from time to time and 
wondered what was going on.  Now that its print value is hardcoded in 
the same function that is given a print parameter, it just looked wrong. 
  The trivial tests weren't enough to clue me in to this.  I'll withdraw 
this.

> ... but what I said only applies to getting largefiles. I think it would
> make sense if updatelfiles stayed quiet on removed largefiles.

Maybe, but then you would have messages print out or not depending upon 
what is present in a changeset.  That might be confusing.

>> Only hg.clean() has the capability to suppress these messages.
>
> Just to be clear: That is behaviour that is introduced here, right?

Yes.  I see the ambiguity here because technically the _capability_ 
comes from the fact that hg.clean() takes in show_stats, but this is the 
first time it is being paid attention to regarding largefiles, and it is 
only conditional with clean().

>> Arguably the
>> updated..resolved line should be present in the two test cases changed
>> here,
>> but at least the output is consistent for now.
>>
>> Note that this also affects (the apparently largefiles untested)
>> bisect and
>> backput commands, as well as hgsubrepo.remove().
>>
>> diff --git a/hgext/largefiles/overrides.py
>> b/hgext/largefiles/overrides.py
>> --- a/hgext/largefiles/overrides.py
>> +++ b/hgext/largefiles/overrides.py
>> @@ -659,7 +659,8 @@
>> newstandins = lfutil.getstandinsstate(repo)
>> filelist = lfutil.getlfilestoupdate(oldstandins, newstandins)
>> - lfcommands.updatelfiles(repo.ui, repo, filelist=filelist,
>> printmessage=True)
>> + lfcommands.updatelfiles(repo.ui, repo, filelist=filelist,
>> + printmessage=showstats)
>> return result
>> diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
>> --- a/tests/test-largefiles.t
>> +++ b/tests/test-largefiles.t
>> @@ -1708,8 +1708,6 @@
>> $ 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
>> @@ -1723,8 +1721,6 @@
>> $ hg -R ../update_on_clone status -S
>> M subrepo/large.txt
>> $ hg -R ../update_on_clone update -C
>> - getting changed largefiles
>> - 1 largefiles updated, 0 removed
>> 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
>> getting changed largefiles
>> 0 largefiles updated, 0 removed
>
> These examples shows that the information is almost useless when using
> subrepos.

Agreed (other than the potential for delays that you pointed out, which 
could happen in the subrepo before any other lines are printed).

> It should show which subrepo it is visiting ... or in this
> case: inherit the quietness to the subrepo.

IOW, don't print the updated...unresolved lines in subrepos?  (Since 
this is removing the largefile messages in subrepos, I assume you don't 
mean that.)  I'm conflicted on this, in part because if most development 
goes on in a subrepo and it only ever says 1 file updated, my reaction 
would be ?!?!.

The easy short term fix is to raise the "getting subrepo %s" debug line 
in hgsubrepo.get() to something normally displayed to make these useful, 
but I'm guessing it was assigned that level purposely.  If that is done, 
then the stats can be printed in subrepos even on clean(), and it would 
be useful.

> /Mads
>

Patch

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -659,7 +659,8 @@ 
         newstandins = lfutil.getstandinsstate(repo)
         filelist = lfutil.getlfilestoupdate(oldstandins, newstandins)
 
-    lfcommands.updatelfiles(repo.ui, repo, filelist=filelist, printmessage=True)
+    lfcommands.updatelfiles(repo.ui, repo, filelist=filelist,
+                            printmessage=showstats)
 
     return result
 
diff --git a/tests/test-largefiles.t b/tests/test-largefiles.t
--- a/tests/test-largefiles.t
+++ b/tests/test-largefiles.t
@@ -1708,8 +1708,6 @@ 
   $ 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
@@ -1723,8 +1721,6 @@ 
   $ hg -R ../update_on_clone status -S
   M subrepo/large.txt
   $ hg -R ../update_on_clone update -C
-  getting changed largefiles
-  1 largefiles updated, 0 removed
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   getting changed largefiles
   0 largefiles updated, 0 removed