Re: [PATCH 6/6] reftable: introduce "reftable-system.h" header

[Patrick Steinhardt <ps@pks.im>]

On Wed, Apr 01, 2026 at 09:37:11AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > It overall is a tiny bit confusing, agreed. The reftable interfaces are
> > split into two parts:
> >
> >   - "reftable/foo.h" contains the library-internal API surface.
> >
> >   - "reftable/reftable-foo.h" contains the external API surface as it
> >     should be consumed by the project that embeds the reftable library.
> >
> > Now for most of the part, headers in the "reftable/reftable-*.h"
> > namespace are self-contained. But naturally, we also use some types
> > there that require us to include headers, like `uint32_t` et al.
> > The requirements that we have here are significantly smaller though than
> > what we expose via "reftable/system.h".
> >
> > So ultimately, the idea was to have "reftable/reftable-system.h" expose
> > the POSIX-like environment that is project-specific to make the other
> > public headers compile as standalone units. And then have the
> > implementeation sit in "reftable/system.c". But I agree that
> > "reftable/system.h" itself still sits somewhere in between of being
> > platform specific and containing project-specific stuff, which isn't
> > great.
> >
> > I'm overall not a 100% happy myself with the split and agree that it's
> > somewhat confusing. An alternative would be to say that it's the
> > caller's responsibility to ensure that our public-facing headers have
> > all dependencies satisfied, which I think is in practice only <stdint.h>.
> >
> > I'm very open to alternative suggestions though.
> 
> So your assessment starts as "tiny bit" and in the end ends ujp with
> "somewhat" confusing ;-)?
> 
> I am OK with two level:
> 
>  - A C source file that will be customized for platform and the
>    project that embeds the library to implement a neutral interface.
> 
>  - A C header file that defines what that neutral interface looks
>    like, without having any platform/project specific implementation
>    details.
> 
> But that header file, in order to define an interface in a neutral
> way, would need to be able to see common types and things like
> "inline", so it is inevitable to have the third thing that is a
> header file that will be customzed for platform and the project that
> embeds the library.
> 
> And in that context making reftable/system.c in this series the C
> source that implements the neutral API for the platform and the
> project, reftable/system.h the C header that declares the neutral
> API, and reftable-system.h the platform specific shim to show a
> common definition to reftable/system.h, may be a good division of
> labor.  So, while I found the _naming_ confusing initially, at least
> between reftable/system.[ch], I no longer see the naming of the
> files a problem.
> 
> The division of labor is not quite honored in the current
> implementation, though, as I pointed out that just like MINGW
> specific things that are moved out of reftable/system.h and to the
> reftable-system.h, other things like inline and compat/zlib-compat.h
> are quite specific to our project and belong to reftable-system.h at
> the layer that exposes a certain system services to reftable/system.h
> and other "more common" parts of the library.

Makes sense, thanks for your input! Will send a new version soonish.

Patrick