[PATCH] submodule summary: Don't barf when invoked in an empty repo

[Johan Herland <johan@herland.net>]

When invoking "git submodule summary" in an empty repo (which can be
indirectly done by setting status.submodulesummary = true), it currently
emits an error message (via "git diff-index") since HEAD points to an
unborn branch.

This patch adds handling of the HEAD-points-to-unborn-branch special case,
so that "git submodule summary" no longer emits this error message.

The patch also adds a test case that verifies the fix.

Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Johan Herland <johan@herland.net>
---

On Tuesday 16 February 2010, Jeff King wrote:
> It looks like this code (git-submodule.sh:556-562):
> 
>         if rev=$(git rev-parse -q --verify "$1^0")
>         then
>                 head=$rev
>                 shift
>         else
>                 head=HEAD
>         fi
> 
> is meant to guess whether the argument is a revision or a file limiter,
> and if the latter, assume HEAD was meant. Which obviously breaks down
> when the argument is HEAD and it is invalid. The patch below seems to
> fix it for me, but I have no idea if I am breaking something else.
> 
> Can somebody more clueful about the submodule script take a look?

I don't know this code very well, but from looking at the commit introducing
this code (28f9af5: git-submodule summary: code framework), your analysis
makes sense. However, your fix doesn't work well for me.

> ---
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 664f217..4332992 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -555,10 +555,12 @@ cmd_summary() {
> 
>  	if rev=$(git rev-parse -q --verify "$1^0")
>  	then
>  		head=$rev
>  		shift
> +	elif test "$1" = "HEAD"; then
> +		return
>  	else
>  		head=HEAD
>  	fi
> 
>  	if [ -n "$files" ]

I'm working from the simple test case in the below patch, I get the
following output with your proposed fix:

  [...]
  trace: built-in: git 'rev-parse' '-q' '--verify' '^0'
  [...]
  trace: built-in: git 'diff-index' '--raw' 'HEAD' '--'
  fatal: bad revision 'HEAD'
  [...]

I.e. your fix doesn't work because $1 is empty (not "HEAD") at this point.

My alternative patch (below) does pass my test case (and all the other
tests as well)

I'd still like an ACK from the original author (Ping Yin) as well, as I'm
not sure if I overlooked something by removing the "$1^0".


Have fun! :)

...Johan


 git-submodule.sh             |    7 +++++--
 t/t7401-submodule-summary.sh |    7 +++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 664f217..906b7b2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -553,12 +553,15 @@ cmd_summary() {
 
 	test $summary_limit = 0 && return
 
-	if rev=$(git rev-parse -q --verify "$1^0")
+	if rev=$(git rev-parse -q --verify --default HEAD $1)
 	then
 		head=$rev
 		shift
+	elif test -z "$1" -o "$1" = "HEAD"
+	then
+		return
 	else
-		head=HEAD
+		head="HEAD"
 	fi
 
 	if [ -n "$files" ]
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index d3c039f..cee319d 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -227,4 +227,11 @@ test_expect_success 'fail when using --files together with --cached' "
     test_must_fail git submodule summary --files --cached
 "
 
+test_expect_success 'should not fail in an empty repo' "
+    git init xyzzy &&
+    cd xyzzy &&
+    git submodule summary >output 2>&1 &&
+    test_cmp output /dev/null
+"
+
 test_done
-- 
1.7.0.rc1.141.gd3fd


-- 
Johan Herland, <johan@herland.net>
www.herland.net