Re: Cryptic error messages?

[Jeff King <peff@peff.net>]

On Mon, Apr 20, 2009 at 04:18:09PM -0400, John Dlugosz wrote:

> $ git push
> Counting objects: 9, done.
> Compressing objects: 100% (8/8), done.
> Writing objects: 100% (8/8), 3.62 KiB, done.
> Total 8 (delta 4), reused 0 (delta 0)
> Unpacking objects: 100% (8/8), done.
> fatal: unresolved deltas left after unpacking
> error: unpack failed: unpacker exited with error code
> To //tx01fs01/sys/dev/git/repositories/aardvark.git
>  ! [remote rejected] dev -> dev (n/a (unpacker error))
> error: failed to push some refs to
> '//tx01fs01/sys/dev/git/repositories/aardvark
> .git'
> 
> Huh?  I'm having trouble defending git's reputation.

Yeah, that is horribly cryptic. What is happening is:

  1. send-pack on the local system spawns receive-pack on the
     remote, which in turn spawns unpack-objects as a helper

  2. unpack-objects barfs with

       fatal: unresolved deltas left after unpacking

     to stderr which is the actual useful bit.

  3. receive-pack notices that the unpacker failed, and spews

       error: unpack failed: unpacker exited with error code

     to stderr, in case unpack-objects didn't say anything.

  4. receive-pack also marks the "status" passed back to send-pack
     as "n/a (unpacker error)"

  5. send-pack gives you the usual nice status table with the ugly
     status from receive-pack marked in it, and then says "OK, I failed
     to push".

So making it better is not quite as simple as you might hope, since
there are three processes involved, and none knows that the other has
spewed to stderr already. But I think there is some low-hanging fruit:

  1. There is no point in receive-pack saying anything to stderr about
     the unpacker failing; in most cases, the unpacker already said
     something, and even if it didn't, we are reporting the problem to
     send-pack in the status field.

  2. "n/a (unpacker error)" is unnecessarily cryptic. Yes, the specifics
     of the message are "not available" (which is presumably what the
     n/a stands for), but the user doesn't care. I think something like
     "failed to unpack objects" would be better.

That leaves only the fact that the _specific_ reason the unpacker failed
is not part of the usual status table. Fixing that is actually a little
tricky because of the multiple processes involved (which do not already
have a string-based communications channel between them).

And of course, it's still a bit cryptic to get "unresolved deltas after
unpacking". However, that is one of those messages that _should_ never
come up, unless the sender is pushing a bogus pack. I wouldn't be
surprised if it an msysgit bug.

-Peff