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

From: Stefan Beller <sbeller@google.com>
Date: Mar 30, 2017, 18:02 UTC
Message-ID: <CAGZ79kbnq0uh9VGwru0JjW8_gJvUtvnt2fsCHNAyBiM=u6ENmw@mail.gmail.com>
In-Reply-To: <xmqqmvc37120.fsf@gitster.mtv.corp.google.com>

On Wed, Mar 29, 2017 at 11:04 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> 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