[PATCH v2 0/3] rev-parse and "--"

[Jeff King <peff@peff.net>]

On Fri, Dec 06, 2013 at 04:15:09PM -0500, Jeff King wrote:

> If you have both a file and a branch named "foo", running:
> 
>   git log foo
> 
> will complain. We should do the same in rev-parse, and
> demand that it be disambiguated with:
> 
>   git rev-parse foo --
> 
> or
> 
>   git rev-parse -- foo
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Hmm, looking at this again, I guess we need to give the same treatment
> to the try_difference code path above, as "foo..bar" can be ambiguous
> with a filename (and "git log" seems to diagnose that).

Here's a revised series which handles that:

  [1/3]: rev-parse: correctly diagnose revision errors before "--"
  [2/3]: rev-parse: be more careful with munging arguments
  [3/3]: rev-parse: diagnose ambiguous revision/filename arguments

While fixing this commit, I noticed another bug whose fix is a
prerequisite. That's now patch 2 in the series. I think the first two
patches are correct and should be applied.

Patch 3 is the revised version of this patch which notices ambiguity.
However, I'm having second thoughts on it. I think it's the right thing
to do if you want to help people build something like "git log"
themselves. But it does mean that we are breaking somebody who does:

  echo foo >HEAD
  commit=$(git rev-parse HEAD)

I'm tempted to say that people who did that are stupid and wrong (and
ugly, too). They should probably be using "--verify" in this case. But
it has been that way for a long time, and there are two instances in our
test scripts that are broken by the patch.

Is it better for "rev-parse" to be more careful, and to behave more like
the rest of git? Or is better to be historically compatible?

One thing to note is that "git rev-parse HEAD" is slightly broken there
already. Because "git rev-parse $some_branch" may do very different
things than what the caller expects if $some_branch does not exist (but
there is a file with the same name). So maybe we are doing a favor by
calling out the problem; if they want a rev, they should be using
"--verify" (or "--").

-Peff