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

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

Kyle Lippincott <spectral@google.com> writes:

> As far as I can tell, we need pager.h because of the `pager_in_use`
> symbol. We need that symbol because of its use in date.c's
> `parse_date_format`. I wonder if we can side step the `#include
> <stdint.h>` concerns by splitting pager.h into pager.h and
> pager_in_use.h, and have pager.h include pager_in_use.h instead. This
> way pager.h (and its [unused] forward declarations) aren't part of
> git-std-lib at all.

Step back a bit.  Why do you even need to touch pager.h in the first
place?  Whatever thing that needs to define a mock version of
pager_in_use() would need to be able to find out that it is supposed
to take nothing as arguments and return an integer, and it can
include <pager.h> without modification.  Just like everybody else,
it has to include <git-compat-util.h> so that the system header that
gives us uintmax_t gets include appropriately in platform-dependent
way, no?  Why do we even need to butcher pager.h into two pieces in
the first place?

If you just include <git-compat-util.h> and then <pager.h> in
stubs/pager.c and you're OK, no?

If anything, as I already said, I think it is more reasonable to
tweak what <git-compat-util.h> does.  For example, it might be
unwieldy for gitstdlib's purpose that it unconditionally overrides
exit(), in which case it may be OK to introduce some conditional
compilation macros to omit that override when building stub code.
Or even split parts of the <git-compat-util.h> that both Git's use
and gitstdlib's purpose are OK with into a separate header file
<git-compat-core.h>, while leaving (hopefully a very minor) other
parts in <git-compat-util.h> *and* include <git-compat-core.h> in
<git-compat-util.h>.  That way, the sources of Git can continue
including <git-compat-util.h> while stub code can include
<git-compat-core.h>, and we will get system library symbols and
system defined types like uintmax_t in a consistent way, both in Git
itself and in gitstdlib.

But once such a sanitization is done on the compat-util header,
other "ordinary" header files that should not have to care about
portability (because they can assume that inclusion of
git-compat-util.h will give them access to system types and symbols
without having to worry about portability issues) and should not
have to include system header files themselves.

At least, that is the idea behind <git-compat-util.h> in the first
place.  Including any system headers directly in ordinary headers,
or splitting ordinary headers at an arbitrary and artificial
boundary, should not be necessary.  I'd have to say that such
changes are tail wagging the dog.

I do not have sufficient cycles to spend actually splitting
git-compat-util.h into two myself, but as an illustration, here is
how I would tweak cw/git-std-lib topic to make it build without
breaking our headers and including system header files directly.

 git-compat-util.h | 2 ++
 pager.h           | 2 --
 stubs/misc.c      | 4 ++--
 stubs/pager.c     | 1 +
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git c/git-compat-util.h w/git-compat-util.h
index 7c2a6538e5..981d526d18 100644
--- c/git-compat-util.h
+++ w/git-compat-util.h
@@ -1475,12 +1475,14 @@ static inline int is_missing_file_error(int errno_)
 
 int cmd_main(int, const char **);
 
+#ifndef _GIT_NO_OVERRIDE_EXIT
 /*
  * Intercept all calls to exit() and route them to trace2 to
  * optionally emit a message before calling the real exit().
  */
 int common_exit(const char *file, int line, int code);
 #define exit(code) exit(common_exit(__FILE__, __LINE__, (code)))
+#endif
 
 /*
  * You can mark a stack variable with UNLEAK(var) to avoid it being
diff --git c/pager.h w/pager.h
index 015bca95e3..b77433026d 100644
--- c/pager.h
+++ w/pager.h
@@ -1,8 +1,6 @@
 #ifndef PAGER_H
 #define PAGER_H
 
-#include <stdint.h>
-
 struct child_process;
 
 const char *git_pager(int stdout_is_tty);
diff --git c/stubs/misc.c w/stubs/misc.c
index 8d80581e39..d0379dcb69 100644
--- c/stubs/misc.c
+++ w/stubs/misc.c
@@ -1,5 +1,5 @@
-#include <assert.h>
-#include <stdlib.h>
+#define _GIT_NO_OVERRIDE_EXIT
+#include <git-compat-util.h>
 
 #ifndef NO_GETTEXT
 /*
diff --git c/stubs/pager.c w/stubs/pager.c
index 4f575cada7..04517aad4c 100644
--- c/stubs/pager.c
+++ w/stubs/pager.c
@@ -1,3 +1,4 @@
+#include <git-compat-util.h>
 #include "pager.h"
 
 int pager_in_use(void)