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

[Jakub Narebski]

Derrick Stolee  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 
> ---
>  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, , );
>   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)
>  {

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

[Derrick Stolee]

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.

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

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)
 {
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, , );
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)
 {
-- 
2.16.2.329.gfb62395de6