| Message ID | xmqqwmqtli18.fsf@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 2ca6c07db24fb6760ac38e3c7d45ee7b6c07f24e |
| Headers | show |
| Series | compat: drop inclusion of <git-compat-util.h> | expand |
On Sat, Feb 24, 2024 at 12:33 PM Junio C Hamano <gitster@pobox.com> wrote: > > These two header files are included from ordinary source files that > already include <git-compat-util.h> as the first header file as they > should. There is no need to include the compat-util in these > headers. > > "make hdr-check" is not affected, as it is designed to assume that > what <git-compat-util.h> offers is available to everybody without > being included. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > > * There is an obvious alternative that goes in the complete > opposite direction possible, to update "make hdr-check" to ensure > that things that are depended upon in each header file > (e.g. pager.h refers to uintmax_t) are brought in by the header > file to include the compat-util in it, i.e. > > diff --git c/Makefile w/Makefile > index 78e874099d..d7b360f15e 100644 > --- c/Makefile > +++ w/Makefile > @@ -3259,7 +3259,7 @@ HCO = $(patsubst %.h,%.hco,$(CHK_HDRS)) > HCC = $(HCO:hco=hcc) > > %.hcc: %.h > - @echo '#include "git-compat-util.h"' >$@ > + @echo '/* #include "git-compat-util.h" */' >$@ > @echo '#include "$<"' >>$@ > > $(HCO): %.hco: %.hcc FORCE > > which would require in a noisy diff to add inclusion of > git-compat-util.h to many header files. For purposes of folks > who may want to carve out only pieces of our source tree, such an > approach might work better, but for that to happen and yield any > useful result, I suspect that compat-util header needs to be > split into "compatibility essentials" and other "it is convenient > if these are available everywhere, even though they do not have > much to do with hiding system dependencies from the sources" > parts first. > > compat/compiler.h | 1 - > compat/disk.h | 1 - > 2 files changed, 2 deletions(-) LG, thanks. I agree with the direction from this patch: I think we should not be including git-compat-util.h in header files, whether they're "internal only" or part of the "external interface" of a library. It does far too much and makes too many assumptions about when it's being included. I plan on writing up some more thoughts on this on a different thread, but it's taking some time to get those into a shareable state. > > diff --git c/compat/compiler.h w/compat/compiler.h > index 10dbb65937..e9ad9db84f 100644 > --- c/compat/compiler.h > +++ w/compat/compiler.h > @@ -1,7 +1,6 @@ > #ifndef COMPILER_H > #define COMPILER_H > > -#include "git-compat-util.h" > #include "strbuf.h" > > #ifdef __GLIBC__ > diff --git c/compat/disk.h w/compat/disk.h > index 6c979c27d8..23bc1bef86 100644 > --- c/compat/disk.h > +++ w/compat/disk.h > @@ -1,7 +1,6 @@ > #ifndef COMPAT_DISK_H > #define COMPAT_DISK_H > > -#include "git-compat-util.h" > #include "abspath.h" > #include "gettext.h" > >
diff --git c/compat/compiler.h w/compat/compiler.h index 10dbb65937..e9ad9db84f 100644 --- c/compat/compiler.h +++ w/compat/compiler.h @@ -1,7 +1,6 @@ #ifndef COMPILER_H #define COMPILER_H -#include "git-compat-util.h" #include "strbuf.h" #ifdef __GLIBC__ diff --git c/compat/disk.h w/compat/disk.h index 6c979c27d8..23bc1bef86 100644 --- c/compat/disk.h +++ w/compat/disk.h @@ -1,7 +1,6 @@ #ifndef COMPAT_DISK_H #define COMPAT_DISK_H -#include "git-compat-util.h" #include "abspath.h" #include "gettext.h"
These two header files are included from ordinary source files that already include <git-compat-util.h> as the first header file as they should. There is no need to include the compat-util in these headers. "make hdr-check" is not affected, as it is designed to assume that what <git-compat-util.h> offers is available to everybody without being included. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * There is an obvious alternative that goes in the complete opposite direction possible, to update "make hdr-check" to ensure that things that are depended upon in each header file (e.g. pager.h refers to uintmax_t) are brought in by the header file to include the compat-util in it, i.e. diff --git c/Makefile w/Makefile index 78e874099d..d7b360f15e 100644 --- c/Makefile +++ w/Makefile @@ -3259,7 +3259,7 @@ HCO = $(patsubst %.h,%.hco,$(CHK_HDRS)) HCC = $(HCO:hco=hcc) %.hcc: %.h - @echo '#include "git-compat-util.h"' >$@ + @echo '/* #include "git-compat-util.h" */' >$@ @echo '#include "$<"' >>$@ $(HCO): %.hco: %.hcc FORCE which would require in a noisy diff to add inclusion of git-compat-util.h to many header files. For purposes of folks who may want to carve out only pieces of our source tree, such an approach might work better, but for that to happen and yield any useful result, I suspect that compat-util header needs to be split into "compatibility essentials" and other "it is convenient if these are available everywhere, even though they do not have much to do with hiding system dependencies from the sources" parts first. compat/compiler.h | 1 - compat/disk.h | 1 - 2 files changed, 2 deletions(-)