bisect: "Needed a single revision" message is confusing

Daniel Bonniot· Nov 2, 2012, 13:23 UTC · lore

Hi,

Suppose I'm doing a git bisect, say:

$ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b0

That works fine. The sha1 could also be a substring, as long as it's unambiguous, e.g.:

$ git bisect good 8c7a786b6c

Now if it's ambiguous, I get an error message:

$ git bisect good 8 fatal: Needed a single revision Bad rev input: 8

All fine and good. But what if I somehow input a non-existing sha1 (in my case see [1]), e,g:

$ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 fatal: Needed a single revision Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1

I understand that technically both "no revision" and "multiple revisions" qualify as "not a single revision", but they correspond to quite different situations. I think it would be more helpful to get different error messages, something like:

Bad rev input: 8 refers to multiple revisions Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 does not refer to a valid revision

(and avoid outputing the "fatal: Needed a single revision" message).

Is this a good idea? Anybody can think of better error messages?

I'm not familiar with the code base at all, but I could give a try at implementing it, unless it's trivial enough that someone does it earlier. After a quick look, it looks like either git-bisect itself or rev-parse would need to be touched, any pointers and hints welcome.

Cheers,

Daniel

[1] if you want to know, I got a sha1 from one repository and used it in another, which probably should work, except that when using git-svn they don't. And the "single revision" error message lead me on a tangent.

Michael J Gruber· Nov 2, 2012, 14:10 UTC · re: Daniel Bonniot · lore

Re: bisect: "Needed a single revision" message is confusing

Daniel Bonniot venit, vidit, dixit 02.11.2012 14:23:

Show 51 quoted lines

> Hi,
> 
> Suppose I'm doing a git bisect, say:
> 
> $ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b0
> 
> That works fine. The sha1 could also be a substring, as long as it's
> unambiguous, e.g.:
> 
> $ git bisect good 8c7a786b6c
> 
> Now if it's ambiguous, I get an error message:
> 
> $ git bisect good 8
> fatal: Needed a single revision
> Bad rev input: 8
> 
> All fine and good. But what if I somehow input a non-existing sha1 (in
> my case see [1]), e,g:
> 
> $ git bisect good 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1
> fatal: Needed a single revision
> Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1
> 
> I understand that technically both "no revision" and "multiple
> revisions" qualify as "not a single revision", but they correspond to
> quite different situations. I think it would be more helpful to get
> different error messages, something like:
> 
> Bad rev input: 8 refers to multiple revisions
> Bad rev input: 8c7a786b6c8eae8eac91083cdc9a6e337bc133b1 does not refer
> to a valid revision
> 
> (and avoid outputing the "fatal: Needed a single revision" message).
> 
> Is this a good idea? Anybody can think of better error messages?
> 
> I'm not familiar with the code base at all, but I could give a try at
> implementing it, unless it's trivial enough that someone does it
> earlier. After a quick look, it looks like either git-bisect itself or
> rev-parse would need to be touched, any pointers and hints welcome.
> 
> Cheers,
> 
> Daniel
> 
> [1] if you want to know, I got a sha1 from one repository and used it
> in another, which probably should work, except that when using git-svn
> they don't. And the "single revision" error message lead me on a
> tangent.
>

The error comes from rev-parse, which is called by bisect. The problem is that

git rev-parse deadbeef git rev-parse --verify deadbeef

give two very different error messages (if there's no dead beef there), and that "git ref-parse --verify" gives the same error message for non-existing as for ambiguous revs.

There are 3 places in builtins/rev-parse.c which call die_no_single_rev(), and at least some of them should probably choose the error message more carefully.

Cheers, Michael