Re: [PATCH 1/1] fetch: set size_multiple in split_commit_graph_opts

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This problem is due to two failures:
>
>  1. It is unclear that we can add the flag COMMIT_GRAPH_WRITE_SPLIT
>     with a NULL split_opts.
>  2. If we have a non-NULL split_opts, then we override the default
>     values even if a zero value is given.
>
> Correct both of these issues. First, do not override size_mult when
> the options provide a zero value. Second, stop creating a split_opts
> in the fetch builtin.

OK, so there is the hardcoded default 2 in the code, and split_opts
structure *can* override it, but 0 in the field of the structure is
meant to signal "no, I do not have any value to override the
default", not "I do want to set the multiple to 0"?

Makes sense.  Will queue.

Thanks.


> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/fetch.c | 4 +---
>  commit-graph.c  | 4 +++-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index f8765b385b..b4c6d921d0 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1866,15 +1866,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  	    (fetch_write_commit_graph < 0 &&
>  	     the_repository->settings.fetch_write_commit_graph)) {
>  		int commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT;
> -		struct split_commit_graph_opts split_opts;
> -		memset(&split_opts, 0, sizeof(struct split_commit_graph_opts));
>  
>  		if (progress)
>  			commit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS;
>  
>  		write_commit_graph_reachable(get_object_directory(),
>  					     commit_graph_flags,
> -					     &split_opts);
> +					     NULL);
>  	}
>  
>  	close_object_store(the_repository->objects);
> diff --git a/commit-graph.c b/commit-graph.c
> index e771394aff..b205e65ed1 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1542,7 +1542,9 @@ static void split_graph_merge_strategy(struct write_commit_graph_context *ctx)
>  
>  	if (ctx->split_opts) {
>  		max_commits = ctx->split_opts->max_commits;
> -		size_mult = ctx->split_opts->size_multiple;
> +
> +		if (ctx->split_opts->size_multiple)
> +			size_mult = ctx->split_opts->size_multiple;
>  	}
>  
>  	g = ctx->r->objects->commit_graph;