Re: [RFC PATCH 04/12] commit-graph: parse commit from chosen graph

From: Jakub Narebski <jnareb@gmail.com>
Date: Apr 19, 2018, 17:21 UTC
Message-ID: <86bmeff6yf.fsf@gmail.com>
In-Reply-To: <20180417181028.198397-5-dstolee@microsoft.com>

Derrick Stolee <dstolee@microsoft.com> writes:

> Before checking a commit-graph file against the object database, we

Actually there is quite a few checks more that can be done without accessing the object database... I'll take a look at later commits why this one is that relatively early in the series.

Show 24 quoted lines

> need to parse all commits from the given commit-graph file. Create
> parse_commit_in_graph_one() to target a given struct commit_graph.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  commit-graph.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/commit-graph.c b/commit-graph.c
> index c5e5a0f860..6d0d303a7a 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -308,17 +308,27 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin
>  	}
>  }
>  
> -int parse_commit_in_graph(struct commit *item)
> +int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item)
>  {
>  	uint32_t pos;
>  
>  	if (item->object.parsed)
> -		return 0;
> +		return 1;

I am confused and befuddled by those apparent changes between returning 0 or returning 1 when object was parsed.

Show 19 quoted lines

> +
> +	if (find_commit_in_graph(item, g, &pos))
> +		return fill_commit_in_graph(item, g, pos);
> +
> +	return 0;
> +}
> +
> +int parse_commit_in_graph(struct commit *item)
> +{
>  	if (!core_commit_graph)
>  		return 0;
> +
>  	prepare_commit_graph();
> -	if (commit_graph && find_commit_in_graph(item, commit_graph, &pos))
> -		return fill_commit_in_graph(item, commit_graph, pos);
> +	if (commit_graph)
> +		return parse_commit_in_graph_one(commit_graph, item);
>  	return 0;
>  }

Seems all right.