Re: [BUG] in rev-parse

From: Jeff King <peff@peff.net>
Date: Dec 15, 2011, 07:05 UTC
Message-ID: <20111215070521.GB1327@sigill.intra.peff.net>
In-Reply-To: <7vk45yplkm.fsf@alter.siamese.dyndns.org>

On Wed, Dec 14, 2011 at 07:20:41PM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > On the other hand, it has been like this since it was introduced in
> > 2006, and I wonder if scripts rely on the --verify side effect.
> 
> It would have been nicer if it did not to imply --verify at all; a long
> hexdigit that do not name an existing object at all will be shortened to
> its prefix that still do not collide with an abbreviated object name of an
> existing object, and even in such a case, the command should not error out
> only because it was fed a non-existing object (of course, if "--verify" is
> given at the same time, its "one input that names existing object only"
> rule should also kick in).

Dropping the implied verify is easy (see below). But handling non-existant sha1s is a much more complicated change, as the regular abbreviation machinery assumes that they exist. E.g., with the patch below:

  $ good=73c6b3575bc638b7096ec913bd91193707e2265d
  $ bad=${good#d}e
  $ git rev-parse --short $good
  73c6b35
  $ git rev-parse --short $bad
  [no output]

Anyway, I'm not sure it's worth changing at this point. It's part of the plumbing API that has been that way forever, it's kind of a rare thing to ask for, and I've already shown a workaround using rev-list.

-Peff

---

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 98d1cbe..b365ca0 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -545,7 +545,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 			if (!strcmp(arg, "--short") ||
 			    !prefixcmp(arg, "--short=")) {
 				filter &= ~(DO_FLAGS|DO_NOREV);
-				verify = 1;
 				abbrev = DEFAULT_ABBREV;
 				if (arg[7] == '=')
 					abbrev = strtoul(arg + 8, NULL, 10);