On Tue, Jun 22, 2021 at 09:52:32PM -0400, Taylor Blau wrote: > On Mon, Jun 21, 2021 at 08:32:56PM +0200, René Scharfe wrote: > > Am 20.06.21 um 22:03 schrieb SZEDER Gábor: > > > RFC!! Alas, not calling stop_progress() on error has drawbacks: > > > > > > - All memory allocated for the progress bar is leaked. > > > - This progress line remains "active", in the sense that if we were > > > to start a new progress later in the same git process, then with > > > GIT_TEST_CHECK_PROGRESS it would trigger the other BUG() catching > > > nested/overlapping progresses. > > > > > > Do we care?! TBH I don't :) > > > Anyway, if we do, then we might need some sort of an abort_progress() > > > function... > > > > I think the abort_progress() idea makes sense; to clean up allocations, > > tell the user what happened and avoid the BUG(). Showing just > > "aborted" instead of "done" should suffice here -- the explanation is > > given a few lines later ("'foo' was not filtered properly"). > > Very well put. I concur that having an abort_progress() API makes sense > for all of the reasons that you suggest, but also because we shouldn't > encourage not using what seems like an appropriate API in order to not > fail tests when GIT_TEST_CHECK_PROGRESS is set. Ah, damn, I was hoping that I can get away with it :)