While working on my commit-graph patch [1] and using a local build in my usual workflows, I found a bug in my branch.

Essentially, when calling `git rev-list --header`, the header information is actually missing for many commits except the first. This was not caught in my testing since t6000-rev-list-misc.sh creates a test repo with only one commit.

The root cause is that the serialized commit graph gets its speedup by not loading buffers for every commit. For many use-cases (merge bases, --topo-order, etc.) we do not need the buffer for most of the commits. In the rev-list example, the buffer is loaded due to a side-effect of being referenced by HEAD or a branch. A similar effect happened in `git log`, hence the following change in my patch:

diff --git a/log-tree.c b/log-tree.c
index 580b3a9..14735d4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -647,8 +647,7 @@ void show_log(struct rev_info *opt)
                show_mergetag(opt, commit);
        }
- if (!get_cached_commit_buffer(commit, NULL))
-               return;
+       get_commit_buffer(commit, NULL);
if (opt->show_notes) {
                int raw;


In rev-list, the "--header" option outputs a value and expects the buffer to be cached. It outputs the header info only if get_cached_commit_buffer() returns a non-null buffer, giving incorrect output. If it called get_commit_buffer() instead, it would immediately call get_cached_commit_buffer() and on failure actually load the buffer.

This has not been a problem before, since the buffer was always loaded at some point for each commit (and saved because of the save_commit_buffer global).

I propose to make get_cached_commit_buffer() static to commit.c and convert all callers to get_commit_buffer(). Is there any reason to _not_ do this? It seems that there is no functional or performance change.

After the serialized commit graph exists and is used, the only change is that we delay loading the buffer until we need to read the commit metadata that is not stored in the graph (message, author/committer).

Thanks,
-Stolee

[1] https://public-inbox.org/git/4d1ee202-7d79-d73c-6e05-d0fc85db9...@gmail.com/T/#m381bfd3f2eafbd254e35a5147cd198bc35055e92
    [Patch v4 00/14] Serialized Git Commit Graph

[2] https://public-inbox.org/git/20140610214039.gj19...@sigill.intra.peff.net/
    [Patch 10/17] provide helpers to access the commit buffer

Reply via email to