git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Sixt <j6t@kdbg.org>, 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 17:25:26 -0400	[thread overview]
Message-ID: <20150612212526.GA25447@peff.net> (raw)
In-Reply-To: <20150612180005.GA9242@peff.net>

On Fri, Jun 12, 2015 at 02:00:05PM -0400, Jeff King wrote:

> When I added GIT_TRACE_PACKET long ago, I had always intended to
> follow-up with a GIT_TRACE_PACKFILE. The former stops tracing when we
> get to the binary data, but I had intended the latter to store the pure
> on-the-wire packfile transmission for later debugging to. I never got
> around to it, but I think it is something like the patch below.

Here it is, a bit more cleaned up. I think this is a nice improvement,
and it does fix some minor issues in the existing GIT_TRACE_PACKET code.
But I don't think it will ever work for receive-pack, because we hand
off the socket directly to index-pack.

I can live with that. But another approach would be to make it easier to
capture the stdin of programs with GIT_TRACE, rather than just their
arguments. That makes it more generically usable (e.g., I have hacky
patches on our servers for capturing "pack-objects" input so I can
replay expensive fetch requests). As Augie noted, that's not a full
pack-file due to the --pack-header stuff. But given a full trace (which
has both the arguments and stdin), you could reconstruct the packfile,
which we could do as a post-processing step.

I dunno. I prefer introducing a nicely generic mechanism, but it would
probably be a little more irritating to use in practice.

I guess yet another way to do it would be to put the GIT_TRACE_PACK
intelligence into index-pack and unpack-objects (rather than
fetch-pack). Then it at least applies to both the pushing and fetching
sides.

Anyway, here are the patches I came up with:

  [1/3]: pkt-line: simplify starts_with checks in packet tracing
  [2/3]: pkt-line: tighten sideband PACK check when tracing
  [3/3]: pkt-line: support tracing verbatim pack contents

-Peff

  reply	other threads:[~2015-06-12 21:25 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
2015-06-12 17:02       ` Augie Fackler
2015-06-12 18:00       ` Jeff King
2015-06-12 21:25         ` Jeff King [this message]
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=20150612212526.GA25447@peff.net \
    --to=peff@peff.net \
    --cc=augie@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).