Re: [ANNOUNCE] Git v2.29.0-rc0

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 7, 2020, 17:38 UTC
Message-ID: <xmqqlfgi2axr.fsf@gitster.c.googlers.com>
In-Reply-To: <20201007154533.GA186764@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 23 quoted lines

> Hmm. That commit removes the call to display_progress() from the main
> loop of threaded_second_pass(), and doesn't appear to add another one
> anywhere. Is the solution really as simple as adding it back in? I.e.
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 8acd078aa0..6dbb4317e0 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1028,6 +1028,10 @@ static void *threaded_second_pass(void *data)
>  		struct object_entry *child_obj;
>  		struct base_data *child;
>  
> +		counter_lock();
> +		display_progress(progress, nr_resolved_deltas);
> +		counter_unlock();
> +
>  		work_lock();
>  		if (list_empty(&work_head)) {
>  			/*
>
> That _seems_ to work fine, but I'm not sure why it was removed in the
> first place (for a good reason, or simply as an accident when rewriting
> the variable declarations at the top of the loop?).

The above looks like an obvious and trivial fix to go back closer to the original.

We seem to have removed find_unresolved_deltas() helper function in that series, but there remains a mention to it in a comment, which we would probably should rethink (it just may be the matter of removing the mention, or if "just like in ..." may have been the last example of doing what the comment suggests all code to do, it may reveal a need for larger clean-up---I dunno).

Thanks.