Subject: Re: [PATCH v3 05/20] commit-graph: load a root tree from specific graph

From: Jakub Narebski <[email-masked]>
Date: Sun, 27 May 2018 21:12:25 +0200

Derrick Stolee <[email-masked]> writes:

> When lazy-loading a tree for a commit, it will be important to select
> the tree from a specific struct commit_graph. Create a new method that
> specifies the commit-graph file and use that in
> get_commit_tree_in_graph().

Is this for the same reason why parse_commit_in_graph_one() was created
in ptch 03/20?  Why it would be important?

>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  commit-graph.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Simple and straightforward refactoring in the same vein as
parse_commit_in_graph_one() one.

>
> diff --git a/commit-graph.c b/commit-graph.c
> index 78ba0edc80..25893ec096 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -358,14 +358,20 @@ static struct tree *load_tree_for_commit(struct commit_graph *g, struct commit *
>  	return c->maybe_tree;
>  }
>  
> -struct tree *get_commit_tree_in_graph(const struct commit *c)
> +static struct tree *get_commit_tree_in_graph_one(struct commit_graph *g,
> +						 const struct commit *c)
>  {
>  	if (c->maybe_tree)
>  		return c->maybe_tree;
>  	if (c->graph_pos == COMMIT_NOT_FROM_GRAPH)
> -		BUG("get_commit_tree_in_graph called from non-commit-graph commit");
> +		BUG("get_commit_tree_in_graph_one called from non-commit-graph commit");

Sidenote: I wonder if it would be better or worse to use __func__ magic
costant variable here (part of C99 and C++11 standards).

> +
> +	return load_tree_for_commit(g, (struct commit *)c);
> +}
>  
> -	return load_tree_for_commit(commit_graph, (struct commit *)c);
> +struct tree *get_commit_tree_in_graph(const struct commit *c)
> +{
> +	return get_commit_tree_in_graph_one(commit_graph, c);
>  }
>  
>  static void write_graph_chunk_fanout(struct hashfile *f,

Looks good to me.