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: [PATCH 3/3] pkt-line: support tracing verbatim pack contents
Date: Fri, 12 Jun 2015 17:28:28 -0400 [thread overview]
Message-ID: <20150612212827.GC25757@peff.net> (raw)
In-Reply-To: <20150612212526.GA25447@peff.net>
When debugging the pack protocol, it is sometimes useful to
store the verbatim pack that we sent or received on the
wire. Looking at the on-disk result is often not helpful for
a few reasons:
1. If the operation is a clone, we destroy the repo on
failure, leaving nothing on disk.
2. If the pack is small, we unpack it immediately, and the
full pack never hits the disk.
3. If we feed the pack to "index-pack --fix-thin", the
resulting pack has the extra delta bases added to it.
We already have a GIT_TRACE_PACKET mechanism for tracing
packets. Let's extend it with GIT_TRACE_PACK to dump the
verbatim packfile.
There are a few other positive fallouts that come from
rearranging this code:
- We currently disable the packet trace after seeing the
PACK header, even though we may get human-readable lines
on other sidebands; now we include them in the trace.
- We currently try to print "PACK ..." in the trace to
indicate that the packfile has started. But because we
disable packet tracing, we never printed this line. We
will now do so.
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/git.txt | 13 +++++++++++-
pkt-line.c | 59 ++++++++++++++++++++++++++++++++++++++-------------
t/t5601-clone.sh | 7 ++++++
trace.c | 7 ++++++
trace.h | 1 +
5 files changed, 71 insertions(+), 16 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 45b64a7..8c44d14 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1000,9 +1000,20 @@ Unsetting the variable, or setting it to empty, "0" or
Enables trace messages for all packets coming in or out of a
given program. This can help with debugging object negotiation
or other protocol issues. Tracing is turned off at a packet
- starting with "PACK".
+ starting with "PACK" (but see 'GIT_TRACE_PACK' below).
See 'GIT_TRACE' for available trace output options.
+'GIT_TRACE_PACK'::
+ Enables tracing of packfiles sent or received by a
+ given program. Unlike other trace output, this trace is
+ verbatim: no headers, and no quoting of binary data. You almost
+ certainly want to direct into a file (e.g.,
+ `GIT_TRACE_PACK=/tmp/my.pack`) rather than displaying it on the
+ terminal or mixing it with other trace output.
++
+Note that this is currently only implemented for the client side
+of clones and fetches.
+
'GIT_TRACE_PERFORMANCE'::
Enables performance related trace messages, e.g. total execution
time of each Git command.
diff --git a/pkt-line.c b/pkt-line.c
index e75af02..2e122c0 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -4,16 +4,51 @@
char packet_buffer[LARGE_PACKET_MAX];
static const char *packet_trace_prefix = "git";
static struct trace_key trace_packet = TRACE_KEY_INIT(PACKET);
+static struct trace_key trace_pack = TRACE_KEY_INIT(PACK);
void packet_trace_identity(const char *prog)
{
packet_trace_prefix = xstrdup(prog);
}
+static int packet_trace_pack(const char *buf, unsigned int len, int sideband)
+{
+ if (!sideband) {
+ trace_verbatim(&trace_pack, buf, len);
+ return 1;
+ } else if (len && *buf == '\1') {
+ trace_verbatim(&trace_pack, buf + 1, len - 1);
+ return 1;
+ } else {
+ /* it's another non-pack sideband */
+ return 0;
+ }
+}
+
static void packet_trace(const char *buf, unsigned int len, int write)
{
int i;
struct strbuf out;
+ static int in_pack, sideband;
+
+ if (!trace_want(&trace_packet) && !trace_want(&trace_pack))
+ return;
+
+ if (in_pack) {
+ if (packet_trace_pack(buf, len, sideband))
+ return;
+ } else if (starts_with(buf, "PACK") || starts_with(buf, "\1PACK")) {
+ in_pack = 1;
+ sideband = *buf == '\1';
+ packet_trace_pack(buf, len, sideband);
+
+ /*
+ * Make a note in the human-readable trace that the pack data
+ * started.
+ */
+ buf = "PACK ...";
+ len = strlen(buf);
+ }
if (!trace_want(&trace_packet))
return;
@@ -24,21 +59,15 @@ static void packet_trace(const char *buf, unsigned int len, int write)
strbuf_addf(&out, "packet: %12s%c ",
packet_trace_prefix, write ? '>' : '<');
- if (starts_with(buf, "PACK") || starts_with(buf, "\1PACK")) {
- strbuf_addstr(&out, "PACK ...");
- trace_disable(&trace_packet);
- }
- else {
- /* XXX we should really handle printable utf8 */
- for (i = 0; i < len; i++) {
- /* suppress newlines */
- if (buf[i] == '\n')
- continue;
- if (buf[i] >= 0x20 && buf[i] <= 0x7e)
- strbuf_addch(&out, buf[i]);
- else
- strbuf_addf(&out, "\\%o", buf[i]);
- }
+ /* XXX we should really handle printable utf8 */
+ for (i = 0; i < len; i++) {
+ /* suppress newlines */
+ if (buf[i] == '\n')
+ continue;
+ if (buf[i] >= 0x20 && buf[i] <= 0x7e)
+ strbuf_addch(&out, buf[i]);
+ else
+ strbuf_addf(&out, "\\%o", buf[i]);
}
strbuf_addch(&out, '\n');
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index bfdaf75..795ece0 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -496,4 +496,11 @@ test_expect_success 'shallow clone locally' '
( cd ddsstt && git fsck )
'
+test_expect_success 'GIT_TRACE_PACK produces a usable pack' '
+ rm -rf dst.git &&
+ GIT_TRACE_PACK=$PWD/tmp.pack git clone --no-local --bare src dst.git &&
+ git init --bare replay.git &&
+ git -C replay.git index-pack -v --stdin <tmp.pack
+'
+
test_done
diff --git a/trace.c b/trace.c
index 3c3bd8f..7393926 100644
--- a/trace.c
+++ b/trace.c
@@ -120,6 +120,13 @@ static int prepare_trace_line(const char *file, int line,
return 1;
}
+void trace_verbatim(struct trace_key *key, const void *buf, unsigned len)
+{
+ if (!trace_want(key))
+ return;
+ write_or_whine_pipe(get_trace_fd(key), buf, len, err_msg);
+}
+
static void print_trace_line(struct trace_key *key, struct strbuf *buf)
{
strbuf_complete_line(buf);
diff --git a/trace.h b/trace.h
index ae6a332..179b249 100644
--- a/trace.h
+++ b/trace.h
@@ -18,6 +18,7 @@ extern int trace_want(struct trace_key *key);
extern void trace_disable(struct trace_key *key);
extern uint64_t getnanotime(void);
extern void trace_command_performance(const char **argv);
+extern void trace_verbatim(struct trace_key *key, const void *buf, unsigned len);
#ifndef HAVE_VARIADIC_MACROS
--
2.4.2.752.geeb594a
next prev parent reply other threads:[~2015-06-12 21:28 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
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 ` Jeff King [this message]
2015-06-16 15:38 ` [PATCH 3/3] pkt-line: support tracing verbatim pack contents 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=20150612212827.GC25757@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 read-only IMAP folder(s) and NNTP newsgroup(s).