Re: [PATCH 1/5] commit-graph: use repo_parse_commit

[Taylor Blau <me@ttaylorr.com>]

On Mon, Feb 01, 2021 at 05:15:03PM +0000, Derrick Stolee via GitGitGadget wrote:
> 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...

> ---
>  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