Re: [PATCH v4 03/17] commit-graph: use chunk-format write API

From: Derrick Stolee <stolee@gmail.com>
Date: Feb 24, 2021, 17:52 UTC
Message-ID: <bb2ca73b-f60b-0030-2b28-753da48bf7a5@gmail.com>
In-Reply-To: <YDaI9pVzW5wkK1aT@nand.local>

On 2/24/2021 12:12 PM, Taylor Blau wrote:

Show 11 quoted lines

> On Wed, Feb 24, 2021 at 05:52:42PM +0100, SZEDER Gábor wrote:
>> On Thu, Feb 18, 2021 at 02:07:25PM +0000, Derrick Stolee via GitGitGadget wrote:
>>> From: Derrick Stolee <dstolee@microsoft.com>
>>>
>>> The commit-graph write logic is ready to make use of the chunk-format
>>> write API. Each chunk write method is already in the correct prototype.
>>> We only need to use the 'struct chunkfile' pointer and the correct API
>>> calls.
>>
>> This patch series messes up the "Writing out commit graph" progress
>> display, and starting at this commit I get:

Thanks for the report and identifying the exact place that caused the
mistake.

> I can confirm. It looks like we never dropped the 'num_chunks' variable,
> which should have happened in this patch.

Yes, makes sense. Hard to see that 'num_chunks' wasn't used because it _was_ being used, just not as intended.

Show 12 quoted lines

> @@ -1887,11 +1886,11 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)
>  		strbuf_addf(&progress_title,
>  			    Q_("Writing out commit graph in %d pass",
>  			       "Writing out commit graph in %d passes",
> -			       num_chunks),
> -			    num_chunks);
> +			       get_num_chunks(cf)),
> +			    get_num_chunks(cf));
>  		ctx->progress = start_delayed_progress(
>  			progress_title.buf,
> -			num_chunks * ctx->commits.nr);
> +			get_num_chunks(cf) * ctx->commits.nr);

This is obviously correct. Thanks for the quick patch!

-Stolee