commit: drop uses of get_cached_commit_buffer()
authorJeff King <peff@peff.net>
Wed, 21 Feb 2018 23:13:38 +0000 (18:13 -0500)
committerJunio C Hamano <gitster@pobox.com>
Thu, 22 Feb 2018 20:12:16 +0000 (12:12 -0800)
The "--show-all" revision option shows UNINTERESTING
commits. Some of these commits may be unparsed when we try
to show them (since we may or may not need to walk their
parents to fulfill the request).

Commit 3131b71301 (Add "--show-all" revision walker flag for
debugging, 2008-02-09) resolved this by just skipping
pretty-printing for commits without their object contents
cached, saying:

Because we now end up listing commits we may not even have been parsed
at all "show_log" and "show_commit" need to protect against commits
that don't have a commit buffer entry.

That was the easy fix to avoid the pretty-printer segfaulting,
but:

1. It doesn't work for all formats. E.g., --oneline
prints the oid for each such commit but not a trailing
newline, leading to jumbled output.

2. It only affects some commits, depending on whether we
happened to parse them or not (so if they were at the
tip of an UNINTERESTING starting point, or if we
happened to traverse over them, you'd see more data).

3. It unncessarily ties the decision to show the verbose
header to whether the commit buffer was cached. That
makes it harder to change the logic around caching
(e.g., if we could traverse without actually loading
the full commit objects).

These days it's safe to feed such a commit to the
pretty-print code. Since be5c9fb904 (logmsg_reencode: lazily
load missing commit buffers, 2013-01-26), we'll load it on
demand in such a case. So let's just always show the verbose
headers.

This does change the behavior of plumbing, but:

a. The --show-all option was explicitly introduced as a
debugging aid, and was never documented (and has rarely
even been mentioned on the list by git devs).

b. Avoiding the commits was already not deterministic due
to (2) above. So the caller might have seen full
headers for these commits anyway, and would need to be
prepared for it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/rev-list.c
log-tree.c
index d5345b6a2e237b550e5cee69e6d46ff8271ade22..d95acaa40e5ca0fe0755e87437bd98e52457dcc4 100644 (file)
@@ -132,7 +132,7 @@ static void show_commit(struct commit *commit, void *data)
        else
                putchar('\n');
 
-       if (revs->verbose_header && get_cached_commit_buffer(commit, NULL)) {
+       if (revs->verbose_header) {
                struct strbuf buf = STRBUF_INIT;
                struct pretty_print_context ctx = {0};
                ctx.abbrev = revs->abbrev;
index fca29d4799da27164394a8c59da067a0522241fe..5c6b09034c5398758cdd33beb630f219768be79c 100644 (file)
@@ -659,9 +659,6 @@ void show_log(struct rev_info *opt)
                show_mergetag(opt, commit);
        }
 
-       if (!get_cached_commit_buffer(commit, NULL))
-               return;
-
        if (opt->show_notes) {
                int raw;
                struct strbuf notebuf = STRBUF_INIT;