[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