Re: [PATCH 1/5] commit-graph: use repo_parse_commit
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Feb 1, 2021, 17:32 UTC
- Message-ID
- <YBg7RAhkvoe/iV1q@nand.local>
- In-Reply-To
- <9c605c99f66cae3c66bcdd4cbeefbfa1ec2be192.1612199707.git.gitgitgadget@gmail.com>
On Mon, Feb 01, 2021 at 05:15:03PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 5 quoted lines
> From: Derrick Stolee <dstolee@microsoft.com> > > The write_commit_graph_context has a repository pointer, so use it. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Everything here looks good to me, but...
Show 24 quoted lines
> ---
> commit-graph.c | 10 +++++-----
> commit.h | 5 +++--
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index f3bde2ad95a..03e5a987968 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1098,7 +1098,7 @@ static int write_graph_chunk_data(struct hashfile *f,
> uint32_t packedDate[2];
> display_progress(ctx->progress, ++ctx->progress_cnt);
>
> - if (parse_commit_no_graph(*list))
> + if (repo_parse_commit_no_graph(ctx->r, *list))
> die(_("unable to parse commit %s"),
> oid_to_hex(&(*list)->object.oid));
> tree = get_commit_tree_oid(*list);
> @@ -1411,11 +1411,11 @@ static void close_reachable(struct write_commit_graph_context *ctx)
> if (!commit)
> continue;
> if (ctx->split) {
> - if ((!parse_commit(commit) &&
> + if ((!repo_parse_commit(ctx->r, commit) &&I know that this has nothing to do with your patch, but it really would be nice to unify all of these parse_commit() vs parse_commit_no_graph() calls.
As I recall this dates back to 43d3561805 (commit-graph write: don't die if the existing graph is corrupt, 2019-03-25), and there was some discussion on the list at the time about revisiting this if we ever were able to write incremental commit graphs:
https://lore.kernel.org/git/20190221223753.20070-8-avarab@gmail.com/
Now may be a good time to revisit that, although I don't want to detract from another fix (that we really do need to land before v2.31.0).
Thanks, Taylor