Subject: Re: [PATCH 12/25] progress.c: add & assert a "global_progress" variable

From: SZEDER Gábor <[email-masked]>
Date: Thu, 16 Sep 2021 20:31:37 +0200

On Wed, Jun 23, 2021 at 07:48:12PM +0200, Ævar Arnfjörð Bjarmason wrote:
> The progress.c code makes a hard assumption that only one progress bar
> be active at a time (see [1] for a bug where this wasn't the case),
> but nothing has asserted that that's the case. Let's add a BUG()
> that'll trigger if two progress bars are active at the same time.

I very much dislike the idea of any BUG() in the progress code that
can trigger outside of the test suite.

As the number of progress-related fixes clearly show, we often misuse
the progress API, and, arguably, a bug is a bug is a bug, so strictly
speaking a BUG() is not wrong here.

However, the progress line is merely a UI gimmick, not a crucial part
of Git, and none of those progress bugs affected the correctness of
the operation itself.  Worse, calling BUG() during some operations
(e.g. 'git commit-graph write', the worst offender when it comes to
progress bugs) can leave a lockfile behind, resulting in scary errors
and requiring manual cleanup in the .git directory, which is a much
worse UX than showing some bogus values or out of order progress
lines.