Re: [PATCH v2 0/6] Improve tag checking in fsck and with transfer.fsckobjects

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Wed, 10 Sep 2014, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> >
> >> This patch series introduces detailed checking of tag objects when calling
> >> git fsck, and also when transfer.fsckobjects is set to true.
> >>
> >> To this end, the fsck machinery is reworked to accept the buffer and size
> >> of the object to check, and for commit and tag objects, we verify that the
> >> buffers contain an end of header (i.e. an empty line) to guarantee that our
> >> checks do not run beyond the buffer.
> >
> > Overall they looked sensible and I am trying to queue them on 'pu'
> > for today's pushout.
> >
> > Thanks.
> 
> I was expecting to see interesting interactions with the "oops, fsck
> was not exiting with non-zero status in some cases" fix by Peff with
> the patch 5/6 of this series that adds two new tests to t1450, but
> because the breakage in these two cases are both only warning-events
> and not error-events, I didn't prefix "git fsck" with test_must_fail
> after all.

There were a couple of issues, thanks for pointing out that I missed
something. Short story: hash-object, fsck *and* pack-objects parse tag
objects as they are encountered. Therefore, it would be a bit hard to test
because we would have to emulate broken hash-object and pack-objects to
generate a pack containing an invalid tag object.

As a compromise, I now test for the warnings to make sure that the code
path is triggered, but do not explicitly test with a pack that would make
index-pack --strict *fail*.

Okay?

Ciao,
Dscho

P.S.: I squashed your changes in slightly modified form.