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

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

Patrick Steinhardt <ps@pks.im> writes:

> We're including a couple of standard headers like <stdint.h> in a bunch
> of locations, which makes it hard for a project to plug in their own
> logic for making required functionality available. For us this is for
> example via "compat/posix.h", which already includes all of the system
> headers relevant to us.

Hmmm.  This is interesting.

> diff --git a/reftable/reftable-system.h b/reftable/reftable-system.h
> new file mode 100644
> index 0000000000..f90c415182
> --- /dev/null
> +++ b/reftable/reftable-system.h
> @@ -0,0 +1,7 @@
> +#ifndef REFTABLE_SYSTEM_H
> +#define REFTABLE_SYSTEM_H
> +
> +#define MINGW_DONT_HANDLE_IN_USE_ERROR
> +#include "compat/posix.h"
> +
> +#endif

This one is clearly tailored to be used in the context of our
system.

> diff --git a/reftable/system.h b/reftable/system.h
> index dffc717bd4..52f964c04b 100644
> --- a/reftable/system.h
> +++ b/reftable/system.h
> @@ -11,8 +11,7 @@
>  
>  /* This header glues the reftable library to the rest of Git */
>  
> -#define MINGW_DONT_HANDLE_IN_USE_ERROR
> -#include "compat/posix.h"
> +#include "reftable-system.h"
>  #include "compat/zlib-compat.h"
>  
>  #define REFTABLE_INLINE(type) static inline type

And so far in this series, I was getting the impression that
reftable/system.c and reftable/system.h are where the target system
specific definitions are stored.

The implementation detail of how we obtain the wallclock time at
millisecond resolution is in reftable/system.c, the implementation
detail of how our mmap() emulation can work to build reftable_mmap()
is in reftable/system.c, for example.

But the corresponding reftable/system.h does not seem to be specific
to the target system at all---it describes the common abstraction,
like "reftable code proper is expected call reftable_mmap() on any
system" and "the way for reftable code is expected to read the
wallclock is by calling reftable_time_ms()".

So <reftable-system.h>, just like <reftable/system.c>, is expected
to have a target platform specific "implementation", and not like
<reftable/system.h> that is expected to be platform neutral (this
neutrality comes from the fact that <reftable/system.c> will
implement the interface specified in <reftable/system.h> for the
target platform).

Which somehow feels confusing.

Besides, the definition of "REFTABLE_INLINE(type)" being "static
inline type", according to the explanation in [1/6], is valid only
in the context of this project, so shouldn't it be done inside
<reftable-system.h>, not <reftable/system.h>"?  For that matter,
what about inclusion of "compat/zlib-compat.h"?  Is it widely
applicable across target platforms, or very specific to our codebase
where this library is used/embedded in?