All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Augie Fackler <augie@google.com>,
	git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: Re: [PATCH v2] fetch-pack: optionally save packs to disk
Date: Fri, 12 Jun 2015 08:07:36 -0700	[thread overview]
Message-ID: <xmqqmw05dkxz.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <557A7ABA.2000404@kdbg.org> (Johannes Sixt's message of "Fri, 12 Jun 2015 08:22:50 +0200")

Johannes Sixt <j6t@kdbg.org> 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?

  reply	other threads:[~2015-06-12 15:07 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAGZ79kaS4utvDbXOo7emmSUH6M-8LY-oA65Ss3PLDkFModkbSg@mail.gmail.com>
2015-06-11 18:59 ` [PATCH v2] fetch-pack: optionally save packs to disk Augie Fackler
2015-06-12  6:22   ` Johannes Sixt
2015-06-12 15:07     ` Junio C Hamano [this message]
2015-06-12 17:02       ` Augie Fackler
2015-06-12 18:00       ` Jeff King
2015-06-12 21:25         ` Jeff King
2015-06-12 21:28           ` [PATCH 1/3] pkt-line: simplify starts_with checks in packet tracing Jeff King
2015-06-12 21:35             ` Stefan Beller
2015-06-12 21:28           ` [PATCH 2/3] pkt-line: tighten sideband PACK check when tracing Jeff King
2015-06-12 21:39             ` Stefan Beller
2015-06-12 21:41               ` Jeff King
2015-06-12 21:43                 ` Stefan Beller
2015-06-12 21:28           ` [PATCH 3/3] pkt-line: support tracing verbatim pack contents Jeff King
2015-06-16 15:38             ` Augie Fackler
2015-06-16 16:39               ` Junio C Hamano
2015-06-16 16:43                 ` Jeff King
2015-06-16 16:52                   ` Augie Fackler
2015-06-16 17:23                     ` Jeff King
2015-06-16 17:10               ` Jeff King
2015-06-16 17:14                 ` Augie Fackler
2015-06-16 17:18                   ` Jeff King
2015-06-16 17:23                     ` Augie Fackler
2015-06-16 19:31                       ` [PATCH/RFC 0/3] add GIT_TRACE_STDIN Jeff King
2015-06-16 19:35                         ` [PATCH 1/3] trace: implement %p placeholder for filenames Jeff King
2015-06-16 19:36                         ` [PATCH 2/3] trace: add pid to each output line Jeff King
2015-06-16 19:37                         ` [PATCH 3/3] trace: add GIT_TRACE_STDIN Jeff King
2015-06-16 19:49                           ` Jeff King
2015-06-16 21:20                             ` Jeff King
2015-06-17 10:04                               ` Duy Nguyen
2015-06-17 19:10                                 ` Jeff King
2015-06-18 10:20                                   ` Duy Nguyen
2015-06-26 18:47                                   ` Junio C Hamano
2015-06-12 16:54     ` [PATCH v2] fetch-pack: optionally save packs to disk Augie Fackler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqmw05dkxz.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=augie@google.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.