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.
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 20, 2019, 20:10 UTC
- Message-ID
- <xmqqlfuiu3sv.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <3bc8677ea7655a3706914f9753c0a3b79dbf7e1f.1568998427.git.gitgitgadget@gmail.com>
"William Baker via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 11 quoted lines
> 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.
Show 5 quoted lines
> @@ -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.
Show 7 quoted lines
> + 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.
Show 9 quoted lines
> + if (flags & MIDX_PROGRESS)
> + progress = start_progress(_("Writing chunks to multi-pack-index"),
> + num_chunks);
> ...
> +
> + display_progress(progress, i + 1);
> }
> + stop_progress(&progress);
> OK.