Re: [RFC PATCH] change default for status.submoduleSummary to true

[Stefan Beller <sbeller@google.com>]

On Wed, Mar 29, 2017 at 11:04 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> A user complained about the workflow with submodules:
>>> Re submodules pain, I've seen a lot of people get confused about
>>> how and when to commit submodule changes. The main thing missing
>>> in the related UIs is some way to summarize the subproject commit
>>> diff in a human readable way. Maybe last log message would be better
>>> than just sha?
>>
>> We could advise all the confused users to turn on
>> status.submoduleSummary.  However there is no downside from turning
>> it on by default apart from a slight change in behavior and bit
>> longer output of git-status and the help in git-commit.
>
> Is "there is no downside" substantiated or just hand-waving?

It's the best kind of hand waving.

> Any pros-and-cons analysis, e.g. performance implications, etc.?

Performance will be terrible. The submodule summary is gathered by
running "git submodule summary", which is shell (that is slower than
the C code in status).

In gerrit (which has 6 submodules, that I modified for the test by adding
one commit on top of each), we find

$ time git -c status.submoduleSummary=false status
...
real 0m0.043s

$ time git -c status.submoduleSummary=true status
...
real 0m0.359s

So it is slower by an order of magnitude.
Maybe it is not the right time to propose this patch then, but we'
rather want to wait until "git submodule summary" is converted to C.
(Maybe we can even parallelize the data gathering before output.)

Even for git.git (which has no submodules), we have

$ time git -c status.submoduleSummary=false status
...
real 0m0.014s
$ time git -c status.submoduleSummary=true status
...
real 0m0.125s

So additionally to porting that part to C, we may want to condition it
to a new function "int do_we_have_submodules_at_all(void)",
which has a similar heuristic as the once proposed patch
to recurse into submodules all the time
https://public-inbox.org/git/20161006193725.31553-3-sbeller@google.com/

Now that I explained why this patch is unacceptable as-is,
let's think of potential upsides:

git-status is the one command to deliver a summary on the state
of a repo with the granularity on the commit
(e.g. it reports the relation to its upstream tracking branch)
and file level (which files are change, untracked).

And I think a submodule is a weird between-state of these
two as it is seen like a file in the superproject, but it also
has commits, which are the smallest unit of logical change.

So maybe we'd want to report just a number in git-status,
which indicates how much the current detached head
is behind/before the remote tracking branch, and if it can be
fast forwarded.

Thanks,
Stefan