git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff Hostetler <git@jeffhostetler.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, peff@peff.net, jonathantanmy@google.com,
	Jeff Hostetler <jeffhost@microsoft.com>
Subject: [PATCH v6 13/14] fetch-pack: restore save_commit_buffer after use
Date: Tue,  5 Dec 2017 17:02:53 +0000	[thread overview]
Message-ID: <20171205170254.65293-14-git@jeffhostetler.com> (raw)
In-Reply-To: <20171205170254.65293-1-git@jeffhostetler.com>

From: Jonathan Tan <jonathantanmy@google.com>

In fetch-pack, the global variable save_commit_buffer is set to 0, but
not restored to its original value after use.

In particular, if show_log() (in log-tree.c) is invoked after
fetch_pack() in the same process, show_log() will return before printing
out the commit message (because the invocation to
get_cached_commit_buffer() returns NULL, because the commit buffer was
not saved). I discovered this when attempting to run "git log -S" in a
partial clone, triggering the case where revision walking lazily loads
missing objects.

Therefore, restore save_commit_buffer to its original value after use.

An alternative to solve the problem I had is to replace
get_cached_commit_buffer() with get_commit_buffer(). That invocation was
introduced in commit a97934d ("use get_cached_commit_buffer where
appropriate", 2014-06-13) to replace "commit->buffer" introduced in
commit 3131b71 ("Add "--show-all" revision walker flag for debugging",
2008-02-13). In the latter commit, the commit author seems to be
deciding between not showing an unparsed commit at all and showing an
unparsed commit without the message (which is what the commit does), and
did not mention parsing the unparsed commit, so I prefer to preserve the
existing behavior.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 fetch-pack.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fetch-pack.c b/fetch-pack.c
index 3c5f064..773453c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -717,6 +717,7 @@ static int everything_local(struct fetch_pack_args *args,
 {
 	struct ref *ref;
 	int retval;
+	int old_save_commit_buffer = save_commit_buffer;
 	timestamp_t cutoff = 0;
 
 	save_commit_buffer = 0;
@@ -786,6 +787,9 @@ static int everything_local(struct fetch_pack_args *args,
 		print_verbose(args, _("already have %s (%s)"), oid_to_hex(remote),
 			      ref->name);
 	}
+
+	save_commit_buffer = old_save_commit_buffer;
+
 	return retval;
 }
 
-- 
2.9.3


  parent reply	other threads:[~2017-12-05 17:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-05 17:02 [PATCH v6 00/14] Parial clone part 3: clone, fetch, fetch-pack, upload-pack, and tests Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 01/14] upload-pack: add object filtering for partial clone Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 02/14] fetch-pack, index-pack, transport: " Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 03/14] fetch-pack: add --no-filter Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 04/14] fetch-pack: test support excluding large blobs Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 05/14] fetch: refactor calculation of remote list Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 06/14] fetch: support filters Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 07/14] fixup: fetch: update error messages from --blob-max-bytes to --filter Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 08/14] fixup: fetch: update --blob-max-bytes to --fitler Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 09/14] fixup: connected: conditionally pass --exclude-promisor-objects to rev-list Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 10/14] partial-clone: define partial clone settings in config Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 11/14] clone: partial clone Jeff Hostetler
2017-12-05 17:02 ` [PATCH v6 12/14] unpack-trees: batch fetching of missing blobs Jeff Hostetler
2017-12-05 17:02 ` Jeff Hostetler [this message]
2017-12-05 17:02 ` [PATCH v6 14/14] t5616: end-to-end tests for partial clone Jeff Hostetler

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20171205170254.65293-14-git@jeffhostetler.com \
    --to=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=peff@peff.net \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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).