From: Jeff King <peff@peff.net>
Date: Wed, 02 Mar 2016 09:22:36 GMT
Subject: Re: [PATCH] bundle doc: 'verify' is not about verifying the bundle
Message-ID: <20160302092236.GB30065@sigill.intra.peff.net>
In-Reply-To: <xmqqd1rd7uey.fsf@gitster.mtv.corp.google.com>

On Tue, Mar 01, 2016 at 01:36:05PM -0800, Junio C Hamano wrote:

> Even though the command does read the bundle header and checks to
> see if it looks reasonable, the thin-pack data stream that follows
> the header in the bundle file is not checked.  More importantly,
> because the thin-pack data does not have a trailing checksum like
> on-disk packfiles do, there isn't much "verification" the command
> can do without unpacking the objects from the stream even if it
> wanted to.

Are you sure about that trailing checksum thing?

  $ git tag foo $(echo foo | git hash-object -w --stdin)
  $ git bundle create foo.bundle foo
  $ perl -ne '
      BEGIN { binmode STDIN; binmode STDOUT }
      $print = 1 if /^PACK/;
      print if $print;
    ' <foo.bundle >foo.pack

  $ tail -c 20 foo.pack | xxd -p -c 20
  aa4e77be3695e8003bf4fbd2139b3fe0a9a22c8f

  $ git index-pack --stdin <foo.pack
  aa4e77be3695e8003bf4fbd2139b3fe0a9a22c8f

So I think we could pretty easily check for bit errors in the packfile
(we can't in the ref data; including a separate sha1 there might be a
nice improvement).

But...

> The documentation gives an incorrect impression that the thin-pack
> data contained in the bundle is validated, but the command is to
> validate that the receiving repository is ready to accept the
> bundle, not to check the validity of a bundle file.  Rephrase the
> paragraph to clarify this.

I still think this is a good change, though. "verify" is doing a
separate thing here than asking "did I get bit errors". We might _also_
want that check, but it is probably less interesting than "does the
bundle have all of the objects it claims to". And you can only find that
by expanding the objects, which would _also_ notice bit errors.

And the simplest way to do that is "git bundle apply". :)

-Peff