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

[Junio C Hamano <gitster@pobox.com>]

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.