Subject: Re: [PATCH v2] fetch-pack: optionally save packs to disk

From: Junio C Hamano <[email-masked]>
Date: Fri, 12 Jun 2015 08:07:36 -0700

Johannes Sixt <[email-masked]> writes:

> What is the problem with the current fetch-pack implementation? Does
> it remove a bogus packfile after download? Does it abort during
> download when it detects a broken packfile? Does --keep not do what
> you need?

Doesn't the incoming data still go through the fattening process,
though?  You will not be able to inspect the byte-for-byte identical
stream that came out of the server end whose packfile generation
logic is suspect.

For the purpose of debugging your own new server implementation, it
might be a better approach to capture the pack as it comes out at
the server end, instead of doing it at the fetch-pack end as it
comes in. But the approach to add this "dump" at the fetch-pack side
is that it gives us a tool to diagnose problems that come from
broken server (re)implementations by other people we cannot debug,
i.e. "you are spewing this corrupt pack against this request; here
is a dump we took to help you go fix your server".

> Instead of your approach (which forks off tee to dump a copy of the
> packfile), would it not be simpler to add an option --debug-pack
> (probably not the best name) that skips the cleanup step when a broken
> packfile is detected and prints the name of the downloaded packfile?

As long as we need to debug a thin pack that comes from the other
end, that approach is not sufficient, I am afraid.

I anticipated that you'd have problem with its use of "tee".  It
probably can do this internally with async interface, perhaps,
instead?