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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 9, 2022, 16:55 UTC
Message-ID: <xmqqwndpn5dz.fsf@gitster.g>
In-Reply-To: <20220608210537.185094-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 6 quoted lines

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