Re: [PATCH v5 1/3] pager: include stdint.h because uintmax_t is used

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 27, 2024, 02:45 UTC
Message-ID: <xmqqzfvmhbfs.fsf@gitster.g>
In-Reply-To: <CAO_smVjeNYFTgh4MZjRM9U1coY=UJxo-E6bD9OfdS1A1Xf6vcQ@mail.gmail.com>

Kyle Lippincott <spectral@google.com> writes:

Show 10 quoted lines

>> In any case, your sources should not include a standard library
>> header directly yourself, period.  Instead let <git-compat-util.h>
>> take care of the details of how we need to obtain what we need out
>> of the system on various platforms.
>
> I disagree with this statement. We _can't_ use a magic compatibility
> header file in the library interfaces, for the reasons I outlined
> further below in my previous message. For those headers, the ones that
> might be included by code that's not under the Git project's control,
> they need to be self-contained, minimal, and maximally compatible.

Note that I am not talking about your random outside program that happens to link with gitstdlib.a; it would want to include a header file <gitstdlib.h> that comes with the library.

Earlier I suggested that you may want to take a subset of <git-compat-util.h>, because <git-compat-util.h> may have a lot more than what is minimally necessary to allow our sources to be insulated from details of platform dependence. You can think of that subset as a good starting point to build the <gitstdlib.h> header file to be given to the library customers.

But the sources that go to the library, as gitstdlib.a is supposed to serve as a subset of gitlib.a to our internal codebase when building the git binary, should still follow our header inclusion rules.

Because we would want to make sure that the sources that are made into gitstdlib.a, the sources to the rest of libgit.a, and the sources to the rest of git, all agree on what system features we ask from the system, feature macros that must be defined to certain values before we include system library files (like _XOPEN_SOURCE and _FILE_OFFSET_BITS) must be defined consistently across all of these three pieces. One way to do so may be to ensure that the definition of them would be migrated to <gitstdlib.h> when we separate a subset out of <git-compat-util.h> to it (and of course, we make <git-compat-util.h> to include <gitstdlib.h> so that it would be still sufficient for our in-tree users to include the <git-compat-util.h>)

<gitstdlib.h> may have to expose an API function that uses some extended types only available by including system header files, e.g. some function may return ssize_t as its value or take an off_t value as its argument.

If our header should include system headers to make these types available to our definitions is probably open to discussion. It is harder to do so portably, unless your world is limited to POSIX.1 and ISO C, than making it the responsibility of library users.

But if the platform headers and libraries support feature macros that allows you to tweak these sizes (e.g. the size of off_t may be controlled by setting the _FILE_OFFSET_BITS to an appropriate value), it may be irresponsible to leave that to the library users, as they MUST make sure to define such feature macros exactly the same way as we define for our code, which currently is done in <git-compat-util.h>, before they include their system headers to obtain off_t so that they can use <gitstdlib.h>.

So the rules for library clients (random outside programs that happen to link with gitstdlib.a) may not be that they must include <git-compat-util.h> as the first thing, but they probably still have to include <gitstdlib.h> fairly early before including any of their system headers, I would suspect, unless they are willing to accept such responsibility fully to ensure they compile the same way as the gitstdlib library, I would think.