Fatal error running status in new repo

I just noticed this when creating a new repo for a project.


$ mkdir tmp
$ cd tmp
$ git init
Initialized empty shared Git repository in /home/jhe/projects/tmp/.git/
$ git status
# On branch master
#
# Initial commit
#
warning: ignoring dangling symref HEAD.
fatal: bad revision 'HEAD'
nothing to commit (create/copy files and use "git add" to track)
$ echo $?
0
$ git --version
git version 1.7.0


Seems a bit silly that "git status" should be issuing warnings, and
fatal errors (especially when the exit code is still 0), when run before
the first commit has been created in a brand new repository.

The warnings make sense if you know what's going on behind the scenes,
but seem like the kind of thing that could scare someone new to git when
they haven't actually done anything wrong at this point.

-- 
Jacob Helwig

On Mon, Feb 15, 2010 at 08:19:45PM -0800, Jacob Helwig wrote:

I just noticed this when creating a new repo for a project.

$ mkdir tmp
$ cd tmp
$ git init
Initialized empty shared Git repository in /home/jhe/projects/tmp/.git/
$ git status
# On branch master
#
# Initial commit
#
warning: ignoring dangling symref HEAD.
fatal: bad revision 'HEAD'
nothing to commit (create/copy files and use "git add" to track)
$ echo $?
0
$ git --version
git version 1.7.0

I can't reproduce it here:

  $ mkdir tmp && cd tmp && git init && git status
  Initialized empty Git repository in /home/peff/tmp/.git/
  # On branch master
  #
  # Initial commit
  #
  nothing to commit (create/copy files and use "git add" to track)

Furthermore, the "ignoring dangling symref" message can only come from
one place (sha1_name.c:285), and is specifically protected by a
conditional making sure that the refname is not "HEAD". Which is just
plain weird.

Do you have anything strange in your ~/.gitconfig? Can you try running
with GIT_TRACE=1 to make sure we are not invoking some sub-command or
something? Did you build git yourself? Can you double-check with a "make
clean && make" that a newly built version exhibits the problem?

Seems a bit silly that "git status" should be issuing warnings, and
fatal errors (especially when the exit code is still 0), when run before
the first commit has been created in a brand new repository.

The warnings make sense if you know what's going on behind the scenes,
but seem like the kind of thing that could scare someone new to git when
they haven't actually done anything wrong at this point.

Agreed. These warnings absolutely should not be shown, but I don't
really understand what is showing them.

-Peff

On 01:05 Tue 16 Feb     , Jeff King wrote:

On Mon, Feb 15, 2010 at 08:19:45PM -0800, Jacob Helwig wrote:

quoted

I just noticed this when creating a new repo for a project.

$ mkdir tmp
$ cd tmp
$ git init
Initialized empty shared Git repository in /home/jhe/projects/tmp/.git/
$ git status
# On branch master
#
# Initial commit
#
warning: ignoring dangling symref HEAD.
fatal: bad revision 'HEAD'
nothing to commit (create/copy files and use "git add" to track)
$ echo $?
0
$ git --version
git version 1.7.0

I can't reproduce it here:

  $ mkdir tmp && cd tmp && git init && git status
  Initialized empty Git repository in /home/peff/tmp/.git/
  # On branch master
  #
  # Initial commit
  #
  nothing to commit (create/copy files and use "git add" to track)

Furthermore, the "ignoring dangling symref" message can only come from
one place (sha1_name.c:285), and is specifically protected by a
conditional making sure that the refname is not "HEAD". Which is just
plain weird.

Do you have anything strange in your ~/.gitconfig? Can you try running
with GIT_TRACE=1 to make sure we are not invoking some sub-command or
something? Did you build git yourself? Can you double-check with a "make
clean && make" that a newly built version exhibits the problem?

quoted

Seems a bit silly that "git status" should be issuing warnings, and
fatal errors (especially when the exit code is still 0), when run before
the first commit has been created in a brand new repository.

The warnings make sense if you know what's going on behind the scenes,
but seem like the kind of thing that could scare someone new to git when
they haven't actually done anything wrong at this point.

Agreed. These warnings absolutely should not be shown, but I don't
really understand what is showing them.

-Peff

$ GIT_TRACE=1 git status
trace: built-in: git 'status'
# On branch master
#
# Initial commit
#
trace: run_command: 'submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'
trace: exec: 'git' 'submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'
trace: exec: 'git-submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'
trace: run_command: 'git-submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--show-cdup'
trace: built-in: git 'rev-parse' '-q' '--git-dir'
trace: built-in: git 'rev-parse' '--is-inside-work-tree'
trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD^0'
warning: ignoring dangling symref HEAD.
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'diff-index' '--cached' '--raw' 'HEAD' '--' 'HEAD'
fatal: bad revision 'HEAD'
trace: run_command: 'submodule' 'summary' '--files' '--for-status' '--summary-limit' '-1'
trace: exec: 'git' 'submodule' 'summary' '--files' '--for-status' '--summary-limit' '-1'
trace: exec: 'git-submodule' 'summary' '--files' '--for-status' '--summary-limit' '-1'
trace: run_command: 'git-submodule' 'summary' '--files' '--for-status' '--summary-limit' '-1'
trace: built-in: git 'rev-parse' '--git-dir'
trace: built-in: git 'rev-parse' '--show-cdup'
trace: built-in: git 'rev-parse' '-q' '--git-dir'
trace: built-in: git 'rev-parse' '--is-inside-work-tree'
trace: built-in: git 'rev-parse' '-q' '--verify' '^0'
trace: built-in: git 'rev-parse' '--show-toplevel'
trace: built-in: git 'diff-files' '--raw' '--'
nothing to commit (create/copy files and use "git add" to track)

I did build git myself, and "make clean && make" does exhibit the same
behavior. (Specifically: git checkout v1.7.0 && git clean -xfd && make
clean && make)

It looks like this is all because I have status.submodulesummary = true
in my ~/.gitconfig.

-- 
Jacob Helwig

On Mon, Feb 15, 2010 at 10:24:22PM -0800, Jacob Helwig wrote:

$ GIT_TRACE=1 git status
trace: built-in: git 'status'
# On branch master
#
# Initial commit
#
trace: run_command: 'submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'

Ah, OK. That is why I did not see it; I don't have submodulesummary
turned on.

I can reproduce your problem. The messages are actually from two
different spots. The first is actually a bug in the code I mentioned
earlier, and I'm still tracking down the second. Patches in a moment.

-Peff

If we encounter a symref that is dangling, in most cases we
will warn about it. The one exception is a dangling HEAD, as
that indicates a branch yet to be born.

However, the check in dwim_ref was not quite right. If we
were fed something like "HEAD^0" we would try to resolve
"HEAD", see that it is dangling, and then check whether the
_original_ string we got was "HEAD" (which it wasn't in this
case). And that makes no sense; the dangling thing we found
was not "HEAD^0" but rather "HEAD".

Fixing this squelches a scary warning from "submodule
summary HEAD" (and consequently "git status" with
status.submodulesummary set) in an empty repo, as the
submodule script calls "git rev-parse -q --verify HEAD^0".

Signed-off-by: Jeff King <redacted>
---
 sha1_name.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 7729925..43884c6 100644
--- a/sha1_name.c
+++ b/sha1_name.c

@@ -280,8 +280,7 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 				*ref = xstrdup(r);
 			if (!warn_ambiguous_refs)
 				break;
-		} else if ((flag & REF_ISSYMREF) &&
-			   (len != 4 || strcmp(str, "HEAD")))
+		} else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD"))
 			warning("ignoring dangling symref %s.", fullref);
 	}
 	free(last_branch);

-- 
1.7.0.25.g1cf14

On Mon, Feb 15, 2010 at 10:24:22PM -0800, Jacob Helwig wrote:

[in an empty repo]

$ GIT_TRACE=1 git status
trace: built-in: git 'status'
# On branch master
#
# Initial commit
#

[...]

trace: run_command: 'git-submodule' 'summary' '--cached' '--for-status' '--summary-limit' '-1' 'HEAD'

[...]

trace: built-in: git 'rev-parse' '-q' '--verify' 'HEAD^0'
warning: ignoring dangling symref HEAD.

[...]

trace: built-in: git 'diff-index' '--cached' '--raw' 'HEAD' '--' 'HEAD'
fatal: bad revision 'HEAD'

The patch I just posted elsewhere in the thread fixes the "ignoring
dangling symref" message. The "bad revision 'HEAD'" comes from
diff-index. But as you can see, that diff-index invocation is somewhat
bogus.

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?

-Peff

---

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" ]

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.

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

On Tue, Feb 16, 2010 at 11:21:14AM +0100, Johan Herland wrote:

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.

Ah, right. I was testing "git status" which calls "git submodule summary
HEAD", but your test uses the assumed HEAD. And both need fixing.

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".

Your patch looks correct to me. I don't really see how dropping the "^0"
will have any effect. rev-parse --verify already prefers revisions to
files, and diff-index should peel if needed.

Which, btw, seems like a mini-bug here. The point of this code is to say
"if we have an argument, is it a revision? If so, shift it off.
Otherwise, put it (and any other arguments) after the -- as a file
limiter".

Usually if I have a branch "master" and a file "master", git will
complain about the ambiguity unless I use an explicit "--" separator.
But here it always takes it as a revision, no questions asked. Or if I
am in "--files" mode, that argument is simply removed and ignored (see
just below where we unset $head).

Probably it should be re-ordered as

  if test -n "$files"; then
     ...
  else
     if rev=$(git rev-parse -q --verify --default HEAD "$1")
       ...
  fi

It just doesn't come up much because usually having branch names and
filenames the same is sufficiently annoying that nobody does it.

-Peff