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

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