patch, 5 partsgit-status: submodule summary support

This patch series introduces summary support for submodule.

The first three patches teach git-submodule subcommand  'summary', and the last two patches teach git-status/git-commit 'status.submodulesummary' configuration to enable/disable submodule summary.

Following patches teach git-submodule a new subcommand 'summary' to show commit summary of user-cared (i.e. checked out) submodules since a given commit (default HEAD) of super project.

For a submodule in question, a series of commits will be shown as the path from the src commit to the dst commit, where the src commit is from the given super project commit, and the dst commit is from the index or working tree (switched by --cached).

.Example: a super project with modified/deleted submodules sm1 to sm3.
--------------------------------------------
 $ git submodule summary
 # Submodules modifiled: sm1 sm2 sm3
 #
 # * sm1 354cd45...3f751e5:
 #   <one line message for C
 #   <one line message for B
 #   >one line message for D
 #   >one line message for E
 #
 # * sm2 5c8bfb5...000000:
 #   <one line message for F
 #
 # * sm3 354cd45...3f751e5:
 #   Warn: sm3 doesn't contain commit 354cd45
 #
--------------------------------------------

sm1 has commit C as src (given commit or HEAD) and commit E as dst (index
or working tree) as following picture shows.
	--A-->B-->C (in src:354cd45)
	  \
	   -->D-->E (in dst:3f751e5)

The 'Submodules modified' section for sm1 shows how to change sm1 from src commit C to dst commit E: firstly backward (<) to commit A from commit C via commit B, and then forward (>) to commit E via commit D.

Illustration for output of deleted sm2 is similar.

If the src/dst commit for a submodule is not found in the work tree (say the submodule respository in the work tree), a warning will be issued. sm3 falls into this case.

This patch just gives the framework. It just finds the submodules to be shown as follows.

--------------------------------------------
 $ git submodule summary
 # Submodules modifiled: sm1 sm2 sm3
 #
--------------------------------------------

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-submodule.sh |   57 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 51 insertions(+), 6 deletions(-)

This patch does the hard work of submodule summary and finally gives output shown in last patch.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-submodule.sh |   68 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 67 insertions(+), 1 deletions(-)

This patches teaches git-submodule an option '--summary-limit|-n <number>' to limit number of commits for the summary. Number 0 will disable summary and minus number will not limit the summary size.

For beauty and clarification, the last commit for each section (backward and forward) will always be shown disregarding the given limit. So actual summary size may be greater than the given limit.

In the same super project of these patch series, 'git submodule -n 2 summary sm1' and 'git submodule -n 3 summary sm1' will show the same.

---------------------------------------
 $ git submodule -n 2 summary sm1
 # Submodules modifiled: sm1
 #
 # * sm1 354cd45...3f751e5:
 #   <one line message for C
 #   <one line message for B
 #   >... (1 more)
 #   >one line message for E
 #
---------------------------------------

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 git-submodule.sh |   40 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 37 insertions(+), 3 deletions(-)

By introducing this commit, 'git commit/status' will additionally show 'Submodules modified' section given by 'git --cached submodule summary' just before 'Untracked files' section.

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 wt-status.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

Add config variable status.submodulesummary which is passed as the arg for '--summary-limit' of 'git submodule' to limit the submodule summary size.

The summary function is disabled by default by this config (say status.submodulesummary is 0 by default).

Signed-off-by: Ping Yin <pkufranky@gmail.com>
---
 wt-status.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Ping Yin <pkufranky@gmail.com> writes:

Why a million?

This is doubly bashism. Variables must be referenced with $, and $(( conditional ? iftrue : iffalse )) is not POSIX.

On Jan 12, 2008 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:

Because i think a million is big enough. I'd better define a constant for unlimited number.

Ok, i'll fix this.

"Ping Yin" <pkufranky@gmail.com> writes:

I think that is a wrong approach to begin with. You are assuming that you will always limit and by using improbably large limit to pretend it is unlimited. Why not making the summary list generator truely capable of produce an unlimited list?

I also think using 100 or so as a sane default, allowing the user to override to say "I do not want any limitation", is a much better default.

On Jan 13, 2008 3:17 AM, Junio C Hamano <gitster@pobox.com> wrote:

I used a pseudo unlimited number just to make code shorter. After considering it again, i find that the code is still brief by using a real unlimited number. So i'll correct it.

Reasonable

Ping Yin <pkufranky@gmail.com> writes:

If you are willing to lose precision forever like this, I think you can run "git diff" with --abbrev and lose this cut.

I'd loosen the above grep (see my comments to your 1/5) and also add this:

		*)
			continue ;# punt
			;;

so that the rest of the code won't break when seeing a path that was submodule in the HEAD but is a blob in the index.

And the precision of $sha1_src matter here. Be it done with "diff --abbrev" at the toplevel or your "cut", that may not be unique enough in the submodule.

I think you would want to read full 40-char sha1_src and sha1_dst with "while read", and keep that full 40-char in these variables, and use them when calling rev-parse here.

If you are checking if that the object exists in the submodule, use "rev-parse --verify", which was designed for exactly that purpose. If you also want to verify if the object is a commit, which may be a good idea anyway, "rev-parse --verify $sha1_src^0".

To be portable, use traditional ">/dev/null 2>&1", not ">&/dev/null".

When reporting errors, you would want to give full 40-chars...

While reporting like this, you would want the shortened form, perhaps produced your "cut -c1-7".

I'd prefer to always have "-e" before the sed expression.

Any reason why you want separate invocation of sed inside the while loop? IOW, why isn't it like this?

	git diff --raw |
        while read ...
        do
        	...
	done | sed -e 's/^/# /'

Hmmm.

On Jan 12, 2008 4:32 PM, Junio C Hamano <gitster@pobox.com> wrote:

Right. I thought 'M' should be the default case because I missed the case 'T' (head blob but index submodule, or the reverse) .

Hmm, precision is really a problem. However, "git diff --raw" will not always give full 40-char sha1, instead it will give sha1 with enough length. So maybe i can use the sha1 from "git diff --raw" ?

Yes, --verify is better.
                   ;;

As said before, enough is ok?

Is it not portable without "-e"?

Just because i'm stupid. :)

Oh, I'm wrong. It seems 'git diff --raw' will always give full 40-char sha1 for submodule entry and abbreviated sha1 for blob entry.

"Ping Yin" <pkufranky@gmail.com> writes:

It is not recommended to use "git diff" in scripts when you can use one of the "git diff-*" plumbing. In this case I think you would want "git-diff-index". Also see --abbrev option.

You can never determine how many hexdigits are "enough" from the containing project, as it does not have to have access to the submodule object store. That's the reason I suggested to read full object name from diff-index and use it for error reporting and object retrieval, and shorten it in the UI for normal status noise.

If it's a type change (head submodule but index blob, or the the reverse), $sha1_dst or $sha1_src will be the sha1 of the blob. It's inapprociate to be shown as if it's a commit in the submodule. May 00000000 should be shown instead of the blob sha1? However, any good way to tell user the type change info? Is it ok to add an additional line such as ""Add file sm1" in the summary and also change the summary description as follows?

-------------------------------------------------------------------------------------
 $ git submodule summary sm1
 # Submodules modifiled: sm1
 #
 # * sm1 354cd45...0000000(3f751e5):
 #   <one line message for C
 #   <one line message for B
 #   >Add file sm1
-------------------------------------------------------------------------------------

"Ping Yin" <pkufranky@gmail.com> writes:

I do not think that adds much value. When A or B is a non-commit, you know that A...B notation does not apply, and because it is probably a rare situation you would want to make it even more clearer to the reader by using a different notation. Like

    echo "* $name have changed from submodule $sha1_src to blob $sha1_dst!!".

perhaps in red bold letter in larger font ;-)

On Jan 13, 2008 3:25 AM, Junio C Hamano <gitster@pobox.com> wrote:

OK, i will think over.

Ping Yin <pkufranky@gmail.com> writes:

You are listing paths that were already submodule in HEAD, or newly added submodule. What about a path that used to be a blob but is being made into submodule with the next commit (i.e. RHS is 160000 but LHS is not 000000)?

Yes, you would need to worry about *un*quoting them.

Hmmmmmm.... 

This is simply unsustainable.

Please see the other thread with Imran M Yousuf regarding the command dispatcher. I think that should be the first thing to fix before doing any change.

On Jan 12, 2008 4:18 PM, Junio C Hamano <gitster@pobox.com> wrote:

Hmm, i had ignored such a case.

Ok, i will resend my patches after this fix