[PATCH] submodule summary: Don't barf when invoked in an empty repo
- From
Johan Herland <johan@herland.net>
- Date
- Feb 16, 2010, 10:21 UTC
- Message-ID
- <201002161121.14613.johan@herland.net>
- In-Reply-To
- <20100216072154.GF2169@coredump.intra.peff.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:
Show 16 quoted lines
> 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.
Show 18 quoted lines
> ---
> 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