Subject: Re: [PATCH v3 04/20] commit: force commit to parse from object database

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

Derrick Stolee <[email-masked]> writes:

> In anticipation of verifying commit-graph file contents against the
> object database, create parse_commit_internal() to allow side-stepping
> the commit-graph file and parse directly from the object database.
>
> Due to the use of generation numbers, this method should not be called
> unless the intention is explicit in avoiding commits from the
> commit-graph file.

A straightforward addition of a parameter to parse_commit() and renaming
it to parse_commit_internal(), while changing parse_commit() to be a
simple wrapper around newly introduced parse_commit_internal(), passing
the default arguments.

>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  commit.c | 9 +++++++--
>  commit.h | 1 +
>  2 files changed, 8 insertions(+), 2 deletions(-)

Nice and simple refactoring in preparation for future changes.

>
> diff --git a/commit.c b/commit.c
> index 1d28677dfb..6eaed0174c 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -392,7 +392,7 @@ int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long s
>  	return 0;
>  }
>  
> -int parse_commit_gently(struct commit *item, int quiet_on_missing)
> +int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_commit_graph)

I guess that the "new" parse_commit_internal() function was not made
static despite the *_internal() in the name because it would need to be
used from commit-graph.c, isn't it?

I don't think we would need more similar options, so *_with_flags()
would be YAGNI overkill.

>  {
>  	enum object_type type;
>  	void *buffer;
> @@ -403,7 +403,7 @@ int parse_commit_gently(struct commit *item, int quiet_on_missing)
>  		return -1;
>  	if (item->object.parsed)
>  		return 0;
> -	if (parse_commit_in_graph(item))
> +	if (use_commit_graph && parse_commit_in_graph(item))
>  		return 0;
>  	buffer = read_sha1_file(item->object.oid.hash, &type, &size);
>  	if (!buffer)
> @@ -424,6 +424,11 @@ int parse_commit_gently(struct commit *item, int quiet_on_missing)
>  	return ret;
>  }
>  
> +int parse_commit_gently(struct commit *item, int quiet_on_missing)
> +{
> +	return parse_commit_internal(item, quiet_on_missing, 1);
> +}
> +
>  void parse_commit_or_die(struct commit *item)
>  {
>  	if (parse_commit(item))
> diff --git a/commit.h b/commit.h
> index b5afde1ae9..5fde74fcd7 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -73,6 +73,7 @@ struct commit *lookup_commit_reference_by_name(const char *name);
>  struct commit *lookup_commit_or_die(const struct object_id *oid, const char *ref_name);
>  
>  int parse_commit_buffer(struct commit *item, const void *buffer, unsigned long size, int check_graph);
> +int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_commit_graph);
>  int parse_commit_gently(struct commit *item, int quiet_on_missing);
>  static inline int parse_commit(struct commit *item)
>  {