Re: propagating repo corruption across clone

[Jeff Mitchell <jeffrey.mitchell@gmail.com>]

On Mon, Mar 25, 2013 at 11:56 AM, Jeff King <peff@peff.net> wrote:
> On Mon, Mar 25, 2013 at 10:31:04PM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> On Mon, Mar 25, 2013 at 9:56 PM, Jeff King <peff@peff.net> wrote:
>> > There are basically three levels of transport that can be used on a
>> > local machine:
>> >
>> >   1. Hard-linking (very fast, no redundancy).
>> >
>> >   2. Byte-for-byte copy (medium speed, makes a separate copy of the
>> >      data, but does not check the integrity of the original).
>> >
>> >   3. Regular git transport, creating a pack (slowest, but should include
>> >      redundancy checks).
>> >
>> > Using --no-hardlinks turns off (1), but leaves (2) as an option.  I
>> > think the documentation in "git clone" could use some improvement in
>> > that area.
>>
>> Not only git-clone. How git-fetch and git-push verify the new pack
>> should also be documented. I don't think many people outside the
>> contributor circle know what is done (and maybe how) when data is
>> received from outside.
>
> I think it's less of a documentation issue there, though, because they
> _only_ do (3). There is no option to do anything else, so there is
> nothing to warn the user about in terms of tradeoffs.
>
> I agree that in general git's handling of corruption could be documented
> somewhere, but I'm not sure where.

Hi there,

First of all, thanks for the analysis, it's much appreciated. It's
good to know that we weren't totally off-base in thinking that a naive
copy may be out of sync, as small as the chance are (certainly we
wouldn't have known the right ordering).

I think what was conflating the issue in my testing is that with
--mirror it implies --bare, so there would be checking of the objects
when the working tree was being created, hence --mirror won't show the
error a normal clone will -- it's not a transport question, it's just
a matter of the normal clone doing more and so having more data run
through checks.

However, there are still problems. For blob corruptions, even in this
--no-hardlinks, non --mirror case where an error was found, the exit
code from the clone was 0. I can see this tripping up all sorts of
automated scripts or repository GUIs that ignore the output and only
check the error code, which is not an unreasonable thing to do.

For commit corruptions, the --no-hardlinks, non --mirror case refused
to create the new repository and exited with an error code of 128. The
--no-hardlinks, --mirror case spewed errors to the console, yet
*still* created the new clone *and* returned an error code of zero.

It seems that when there is an "error" as opposed to a "fatal" it
doesn't affect the status code on a clone; I'd argue that it ought to.
If Git knows that the source repository has problems, it ought to be
reflected in the status code so that scripts performing clones have a
normal way to detect this and alert a user/sysadmin/whoever. Even if a
particular cloning method doesn't perform all sanity checks, if it
finds something in the sanity checks it *does* perform, this should be
trumpeted, loudly, regardless of transport mechanism and regardless of
whether a user is watching the process or a script is.

Thanks,
Jeff