Re: [PATCH 1/6] object.c: parse commit in graph first
- Date: Tue, 3 Apr 2018 14:32:19 -0400
- From: Derrick Stolee <stolee@xxxxxxxxx>
- Subject: Re: [PATCH 1/6] object.c: parse commit in graph first
On 4/3/2018 2:28 PM, Jeff King wrote:
On Tue, Apr 03, 2018 at 11:21:36AM -0700, Jonathan Tan wrote:
On Tue, 3 Apr 2018 12:51:38 -0400
Derrick Stolee <dstolee@xxxxxxxxxxxxx> wrote:
Most code paths load commits using lookup_commit() and then
parse_commit(). In some cases, including some branch lookups, the commit
is parsed using parse_object_buffer() which side-steps parse_commit() in
favor of parse_commit_buffer().
Before adding generation numbers to the commit-graph, we need to ensure
that any commit that exists in the graph is loaded from the graph, so
check parse_commit_in_graph() before calling parse_commit_buffer().
Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
Modifying parse_object_buffer() is the most pragmatic way to accomplish
this, but this also means that parse_object_buffer() now potentially
reads from the local object store (instead of only relying on what's in
memory and what's in the provided buffer). parse_object_buffer() is
called by several callers including in builtin/fsck.c. I would feel more
comfortable if the relevant  caller to parse_object_buffer() was
modified instead of parse_object_buffer(), but I'll let others give
their opinions too.
It's not just you. This seems like a really odd place to put it.
Especially because if we have the buffer to pass to this function, then
we'd already have incurred the cost to inflate the object.
OK. Thanks. I'll try to find the better place to put this check.