On 2024/01/17 17:52:14 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> On Wed, Jan 17, 2024 at 02:13:20PM +0100, Omar Polo wrote:
> > On 2024/01/17 13:10:39 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > > On Wed, Jan 17, 2024 at 12:58:01PM +0100, Stefan Sperling wrote:
> > > > On Wed, Jan 17, 2024 at 12:01:06PM +0100, Omar Polo wrote:
> > > > > Oh, this also means that got can't be built on the latest release.
> > > > 
> > > > The main branch already fails to build on 7.4 now.
> > > > 
> > > > I would like to preserve our ability to provide -stable packages on 7.4.
> > > > Is the hack below too horrible or can we use this until 7.5?
> > > 
> > > New diff which also fixes the build of gotd, gotsh, and gotwebd
> > > on 7.4 by adding imsg_get_fd() stubs in additional places.
> > 
> > I concur with the sentiment but the implementation is wrong.  We leak
> > fds if imsg_free() doesn't close them, which is the case in 7.4.
> > 
> > Here's another way to do this, with less fiddling.  Still ugly, but
> > should avoid the leaks.  We'll need to augment this with
> > imsg_get_data() and _len() soon.
> 
> This reaching into libutil internals feels a bit too risky to me.
> And it is probably not worth the extra effort.
> Right now there is nothing critical to fix relative to 0.95.
> These fd leaks in privsep helpers have always been there and they
> don't seem to really hurt anyone.

that leak is not a big deal since we don't reuse the got-read-pack
processes.

> How about we shelve this effort, for now. If something comes up that
> absolutely needs to be patched in -stable, we can always apply a patch
> against the 0.95 package in -stable while moving -current to 0.96.

Agreed.  If we really must update -stable we could also consider just
bundling imsg{,-buffer}.c and imsg.h.

It's also quite difficult to "port" imsg_get_data(), imsg_get_len(),
etc. as shims on top of previous versions of libutil.

I'll just give up and update my server to -current one of these days :)

> We should just make sure to move our regression buildbots to -current.