From: Taylor Blau <me@ttaylorr.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Patrick Steinhardt <ps@pks.im>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 1/2] lookup_commit_in_graph(): use prepare_commit_graph() to check for graph
Date: Tue, 6 Sep 2022 17:16:55 -0400 [thread overview]
Message-ID: <Yxe4xxF/yWTLAtI3@nand.local> (raw)
In-Reply-To: <Yxe1VbyYovwprPgQ@coredump.intra.peff.net>
On Tue, Sep 06, 2022 at 05:02:13PM -0400, Jeff King wrote:
> I couldn't find any other reason to avoid calling prepare_commit_graph()
> here (especially since it ends up lazy-loaded as discussed above). The
> cost of the call should not be high; after the first call, it is
> simplified down to a few integer checks.
Neither could I. Obviously f559d6d45e (revision: avoid hitting packfiles
when commits are in commit-graph, 2021-08-09) is adding a call to look
for a commit by object ID in the commit-graph where there wasn't one
before, so there isn't anything to compare to there.
But the next-closest function `load_commit_graph_info()` calls
`prepare_commit_graph()` (via `repo_find_commit_pos_in_graph()`).
And that all matches my understanding that `r->objects->commit_graph` is
lazily loaded. Perhaps it should be made more difficult to access via
the struct member, and instead done behind a function like
`prepare_commit_graph()` (modified to return the `struct commit_graph*`
if one was found).
But that's for another day :-). This is obviously correct in the
meantime.
Thanks,
Taylor
next prev parent reply other threads:[~2022-09-06 21:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-06 20:58 [PATCH 0/2] bug with rev-list --verify-objects and commit-graph Jeff King
2022-09-06 21:02 ` [PATCH 1/2] lookup_commit_in_graph(): use prepare_commit_graph() to check for graph Jeff King
2022-09-06 21:16 ` Taylor Blau [this message]
2022-09-06 21:04 ` [PATCH 2/2] rev-list: disable commit graph with --verify-objects Jeff King
2022-09-06 21:20 ` Taylor Blau
2022-09-07 16:38 ` Junio C Hamano
2022-09-06 21:20 ` [PATCH 0/2] bug with rev-list --verify-objects and commit-graph Taylor Blau
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=Yxe4xxF/yWTLAtI3@nand.local \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.