Re: [PATCH v5 0/3] add a test mode for SANITIZE=leak, run it in CI

[Emily Shaffer <emilyshaffer@google.com>]

On Wed, Sep 08, 2021 at 02:03:30PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> 
> On Wed, Sep 08 2021, Junio C Hamano wrote:
> 
> > Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> >
> >> We can compile git with SANITIZE=leak, and have had various efforts in
> >> the past such as 31f9acf9ce2 (Merge branch 'ah/plugleaks', 2021-08-04)
> >> to plug memory leaks, but have had no CI testing of it to ensure that
> >> we don't get regressions. This series adds a GIT_TEST_* mode for
> >> checking those regressions, and runs it in CI.
> >>
> >> Since I submitted v2 the delta between origin/master..origin/seen
> >> broke even t0001-init.sh when run under SANITIZE=leak, so this series
> >> will cause test smoke on "seen".
> >>
> >> That failure is due to a bug in es/config-based-hooks [1] and the
> >> hn/reftable topic, i.e. these patches are legitimately catching
> >> regressions in "seen" from day 1.
> >
> > So is there a point in sending this out to the list, before sending
> > fixes to these broken topic and making sure they get corrected?
> >
> > Because the CI does not "bisect" to tell us "ok, up to this point in
> > 'seen', all the topics merged play well together", the overall
> > effect in the bigger picture is that 'seen' with this series would
> > cause CI to stay in failed state.
> >
> > For now, I'll keep this near the tip of 'seen'.
> 
> The breakages with it are in combination with:
> 
>     ab/config-based-hooks-base
>     es/config-based-hooks
>     hn/reftable
> 
> You've got v4 of ab/config-based-hooks-base, the v5 is at [1], but we've
> been waiting on emily to re-roll hers on top. As noted in that E-Mail
> I've got a working re-roll of it as
> avar-nasamuffin/config-based-hooks-restart-3 in my repo.
> 
> That'll leave hn/reftable, which given [2] I thought you were planning
> to eject, and wiht the number of fixups for it / the planned re-doing of
> it by Han-Wen[3] maybe it's better to do that now?
> 
> What do you think about that plan?
> 
> I.e. ejecting hn/reftable while waiting on a re-roll, and either
> ejecting es/config-based-hooks while waiting, or I can submit the
> avar-nasamuffin/config-based-hooks-restart-3 I've got pending Emily's
> own re-roll (which may or may not be different from that).

My own reroll is waiting on some feedback internally and probably won't
show up this week at all, so I suggest to kick mine out and prioritize
the reftable stuff for now.

 - Emily

> 
> That along with picking up the v5 of my ab/config-based-hooks-base
> should make "seen" pass with SANITIZE=leak on these tests, unless
> there's other just-introduced regressions. I tried re-building it a few
> days ago, I haven't done that just now.
> 
> 1. https://lore.kernel.org/git/cover-v5-00.36-00000000000-20210902T125110Z-avarab@gmail.com/
> 2. https://lore.kernel.org/git/xmqq4kaxe5dt.fsf@gitster.g/
> 3. https://lore.kernel.org/git/CAFQ2z_N8pUsp3cdBpybHBD-V9_1sARCZvSxr0UkMfcwCoQfCbw@mail.gmail.com/