Subject: Re: [PATCH] connected: distinguish local/remote bad objects

From: Junio C Hamano <[email-masked]>
Date: Thu, 09 Jun 2022 09:55:20 -0700

Jonathan Tan <[email-masked]> writes:

>  builtin/fetch.c              |  2 +-
>  connected.c                  |  1 +
>  revision.c                   | 16 ++++++++++++--
>  revision.h                   |  3 +++
>  t/t5518-fetch-exit-status.sh | 43 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 62 insertions(+), 3 deletions(-)

This seems to break linux-leaks CI job by making 5518, which was
marked in some topic in flight to expect to be leak-free, fail.

Because of the way linux-leaks test framework is done, it is not
easy to tell if the code changes essential to this topic introduced
new leaks, in which case we would want to fix that.

Note that this may not the fault of the code changes in this patch.
If the tests added by the patch started using git commands that are
known to leak (i.e. not ready to be subjected to the "leaks" test)
in order to prepare the scenario or to inspect the result, even if
the code changes in this topic did not introduce any leak, we can
see the same breakage in linux-leaks CI job.  An easy way out would
be to disable leak-check CI for the entire 5518, but that is not
very satisfactory, as the earlier part of that script should still
be leak-free.  Another way out might be to add these two tests in a
new script, which is not marked as not-leaking.  After all, what the
new topic adds is not about exit status but how that exit status
comes about, so it might not be a bad idea even without the CI leak
stuff anyway.

Ævar, does the internal state used for revision walking count as
leaking when it is still held by the time we hit die() in
bad_object(), or anything on stack when we die() are still reachable
and won't be reported as a failure?

Thanks.