Re: [PATCH v2 2/6] midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set.

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

"William Baker via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/midx.c b/midx.c
> index b2673f52e8..54e4e93b2b 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -449,6 +449,8 @@ struct pack_list {
>  	uint32_t nr;
>  	uint32_t alloc;
>  	struct multi_pack_index *m;
> +	struct progress *progress;
> +	uint32_t pack_paths_checked;
>  };

What is the reason behind u32 here?  Do we want to be consistently
limited to 4G on all platforms?

I have a feeling that we do not care too deeply all that much for
the purpose of progress output, in which case, just an ordinary
"unsigned" may be much less misleading.

FWIW, the function that uses this field is display_progress(), which
takes uint64_t there.

> @@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,
>  	struct pack_list *packs = (struct pack_list *)data;
>  
>  	if (ends_with(file_name, ".idx")) {
> +		display_progress(packs->progress, ++packs->pack_paths_checked);

OK, "git grep display_progress" tells me that we are expected to
pass the value of the counter that is already incremented, so this
is also in line with that practice.

> +	if (flags & MIDX_PROGRESS)
> +		packs.progress = start_progress(_("Adding packfiles to multi-pack-index"), 0);
> +	else
> +		packs.progress = NULL;
>
>  	for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);
> +	stop_progress(&packs.progress);

OK.

> +	if (flags & MIDX_PROGRESS)
> +		progress = start_progress(_("Writing chunks to multi-pack-index"),
> +					  num_chunks);
> ...
> +
> +		display_progress(progress, i + 1);
>  	}
> +	stop_progress(&progress);
>  

OK.