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

[Jeff King <peff@peff.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