Subject: [PATCH 6/7] [RFC] entry: don't show "Filtering content: ... done." line in case of errors

From: SZEDER Gábor <[email-masked]>
Date: Sun, 20 Jun 2021 22:03:02 +0200

The test 'missing file in delayed checkout' in 't0021-conversion.sh'
fails when run with GIT_TEST_CHECK_PROGRESS=1, because the final value
of the "Filtering content" progress counter doesn't match the expected
total, triggering BUG().  This is not caused by a bug in how we count
progress, but because the test involves a purposefully buggy filter
process that doesn't process any paths, so the progress counter
doesn't have a chance to reach the expected total.

Arguably, it is wrong to show "done" at the end of the progress
line when not all work was done.

So let's check whether there were any errors while processing or that
there are still unprocessed paths at the end (which a few lines later
will in fact be considered as error) and don't show the final "done"
line, i.e. don't call stop_progress(), if there were any.  And if we
don't call stop_progress(), then we won't verify that the progress
counter matches the expected total, won't trigger BUG() on mismatch,
and t0021 will succeed even with GIT_TEST_CHECK_PROGRESS=1.

After this change the test suite passes with
GIT_TEST_CHECK_PROGRESS=1.

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...

Signed-off-by: SZEDER Gábor <[email-masked]>
---
 entry.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/entry.c b/entry.c
index bc4b8fcc98..38baefe22a 100644
--- a/entry.c
+++ b/entry.c
@@ -232,7 +232,8 @@ int finish_delayed_checkout(struct checkout *state, int *nr_checkouts)
 		}
 		string_list_remove_empty_items(&dco->filters, 0);
 	}
-	stop_progress(&progress);
+	if (!errs && !dco->paths.nr)
+		stop_progress(&progress);
 	string_list_clear(&dco->filters, 0);
 
 	/* At this point we should not have any delayed paths anymore. */
-- 
2.32.0.289.g44fbea0957