On Sat, Apr 22 2023, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@xxxxxxxxx> > > Signed-off-by: Elijah Newren <newren@xxxxxxxxx> > --- > object-file.c | 1 + > object-name.c | 1 + > object-store.h | 8 ++++---- > submodule-config.c | 1 + > 4 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/object-file.c b/object-file.c > index 8e0df7360ae..921a717d8a5 100644 > --- a/object-file.c > +++ b/object-file.c > @@ -38,6 +38,7 @@ > #include "packfile.h" > #include "object-file.h" > #include "object-store.h" > +#include "oidtree.h" > #include "promisor-remote.h" > #include "setup.h" > #include "submodule.h" > diff --git a/object-name.c b/object-name.c > index 5ccbe854b60..88d839f70bc 100644 > --- a/object-name.c > +++ b/object-name.c > @@ -14,6 +14,7 @@ > #include "remote.h" > #include "dir.h" > #include "oid-array.h" > +#include "oidtree.h" > #include "packfile.h" > #include "pretty.h" > #include "object-store.h" > diff --git a/object-store.h b/object-store.h > index f9d225783ae..23ea86d3702 100644 > --- a/object-store.h > +++ b/object-store.h > @@ -2,16 +2,16 @@ > #define OBJECT_STORE_H > > #include "object.h" > -#include "oidmap.h" > #include "list.h" > -#include "oid-array.h" It seems to me that this should be split up, there does seem to be an unnecessary include here, as the subject claims, at least the "oid-array.h include in object-store.h seems to qualify. Maybe the same applies to thread-utils.h below? > -#include "strbuf.h" > #include "thread-utils.h" > #include "khash.h" > #include "dir.h" > -#include "oidtree.h" > #include "oidset.h" > > +struct oidmap; > +struct oidtree; > +struct strbuf; But as this shows at least three of these weren't unnecessary, you're just replacing them with forward-decls. I think that's probably sensible, but I think it should at least be discussed. It's also not clear why we want to just stop at forward-declaring structs, maybe replacing the dir.h include with: int fspatheq(const char *a, const char *b); unsigned int fspathhash(const char *str); Would be too verbose, but if we did that we'd spot that e.g. match-trees.c is relying on this header to get its "struct strbuf" definition. I'm perfectly fine to leave that can of worms for some later date though...
