Re: [PATCH] t1092: use GIT_PROGRESS_DELAY for consistent results

From

Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Date

May 25, 2021, 07:39 UTC

Message-ID

<87y2c3mezw.fsf@evledraar.gmail.com>

In-Reply-To

<YKxBIP4lkoPeCYvb@nand.local>

On Mon, May 24 2021, Taylor Blau wrote:

> On Tue, May 25, 2021 at 12:57:52AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> On Mon, May 24 2021, Taylor Blau wrote:
>>
>> > But I don't think this isolated instance should push in the direction of
>> > adding support for either of the above, regardless of how easy it might
>> > be.
>>
>> I don't see why we wouldn't just tweak GIT_PROGRESS_DELAY to support -1
>> or something for "inf".
>
> Ironically, I think that this already works, since we parse the value of
> GIT_PROGRESS_DELAY as unsigned, and don't bother checking for if the
> input is negative (since we eventually call git_parse_unsigned(), which
> doesn't have any extra checks other than for overflow).
>
> So we silently convert -1 to 2^64-1, and call it a day.

I mean why do just that when we can also do this:

diff --git a/progress.c b/progress.c
index 680c6a8bf93..191c62cbbfb 100644
--- a/progress.c
+++ b/progress.c
@@ -252,7 +252,13 @@ void display_progress(struct progress *progress, uint64_t n)
 static struct progress *start_progress_delay(const char *title, uint64_t total,
 					     unsigned delay, unsigned sparse)
 {
-	struct progress *progress = xmalloc(sizeof(*progress));
+	struct progress *progress;
+
+	/* GIT_PROGRESS_DELAY=-1 */
+	if (delay == (unsigned)-1)
+		return NULL;
+
+	progress = xmalloc(sizeof(*progress));
 	progress->title = title;
 	progress->total = total;
 	progress->last_value = -1;

Which will cause the progress code to abort early in that case, and IMO
is less magical if we're going to have this GIT_PROGRESS_DELAY=-1 in the
codebase and relying on the wrap-around of -1.