Re: [PATCH 2/2] format-patch: fix leak of rev_info in prepare_bases()

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jul 02, 2026 at 04:58:21AM -0400, Jeff King wrote:
> On Wed, Jul 01, 2026 at 11:01:22AM +0200, Patrick Steinhardt wrote:
> 
> > > > linux-reftable or linux-reftable-leaks? I think it would certainly make
> > > > sense to drop one of these and merge it into linux-TEST-vars. The
> > > > linux-reftable job doesn't provide any benefit over its -leak variant,
> > > > so that would be the candidate I'd personally merge.
> > > 
> > > Both. Fold linux-reftable into linux-TEST-vars, and then drop
> > > linux-reftable-leaks in favor of a new linux-TEST-vars-leaks.
> > 
> > Hm, okay. I guess that should be fine. Do we also want to do a similar
> > thing for macOS and create a macos-TEST-vars job that exercises all of
> > this?
> 
> It could be helpful if we expect the interaction of macOS and those
> test-vars to be interesting, but I'm a bit skeptical. Most of them are
> about feature selection. So I'm doubtful it would turn up anything
> useful. But who knows.
> 
> Likewise I find the dual clang/gcc jobs to be overkill. Compiling with
> both is useful, as they have different warnings. But have we ever seen a
> case where running the tests showed a different result with different
> compilers?

Not that I'd know of. As you say, I think it makes sense to use
different compilers in general. But I don't really think we need to have
this as a full "compiler x tests" matrix.

> I dunno. I guess there is an argument for CI-maximalism; as long as the
> jobs run in parallel and they're "just" CPU-minutes. But those minutes
> eventually have a cost, and I'm not sure I've gotten useful data from
> most of the jobs (i.e., failures that didn't also just happen somewhere
> else).

I'm certainly on board with reducing the test matrix a bit. I'm sure
that we can have a cleverer selection of jobs where we both have the
same test coverage as we have right now while running less jobs overall.

> Anyway, that is all a big tangent/rant. Mostly I think it would be fine
> to cannibalize linux-reftable into linux-TEST-vars if we want to get
> more coverage without increasing the CI cost.

You got to start somewhere :)

> Note that I did find some leaks that would only be hit running
> linux-sha256 with a non-standard backend like OPENSSL_SHA256=1.  But
> that is getting super specific now (even if we ran linux-sha256 with
> leak detection, would we want to do it with openssl and not the default
> backend)?
> 
> > Also, while at it... I really think that job name is just plain awful.
> > While at it, we might rename it to something more sensible like
> > "linux-changed-defaults".
> 
> Yes please. Every time I see the all-caps TEST in the middle I think I'm
> having a stroke.

Heh :P

> change-defaults is OK but not super descriptive. I might call it
> linux-exotic-flags or something. That's not descriptive either, but is a
> little more fun.

I certainly like it more than my suggestion.

Thanks!

Patrick