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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]