Thread (8 messages) flat view 8 messages, 3 authors, 2016-06-15
DORMANTno replies

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

From: Johan Herland <hidden>
Date: 2016-06-15 22:48:17
Subsystem: the rest · Maintainer: Linus Torvalds

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 <redacted>
Signed-off-by: Johan Herland <redacted>
---

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.
quoted hunk ↗ jump to hunk
---
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help