On Thu, Feb 23, 2023 at 6:17 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > On 2/23/2023 3:05 AM, Elijah Newren via GitGitGadget wrote: > > From: Elijah Newren <newren@xxxxxxxxx> > > > > Things should be able to depend on object.h without pulling in all of > > cache.h. Move an enum to allow this. > > > > Note that a couple files previously depended on things brought in > > through cache.h indirectly (revision.h -> commit.h -> object.h -> > > cache.h). As such, this change requires making existing dependencies > > more explicit in two files. > > > diff --git a/diff.h b/diff.h > > index 41eb2c3d428..b90036f5294 100644 > > --- a/diff.h > > +++ b/diff.h > > @@ -8,6 +8,7 @@ > > #include "pathspec.h" > > #include "object.h" > > #include "oidset.h" > > +#include "strbuf.h" > > > > /** > > * The diff API is for programs that compare two sets of files (e.g. two trees, > > @@ -71,7 +72,6 @@ struct oid_array; > > struct option; > > struct repository; > > struct rev_info; > > -struct strbuf; > > struct userdiff_driver; > > This inclusion of strbuf.h in diff.h seems like a step in the > opposite direction. What caused you to need to do this? > > Looking ahead at the patch titles, I see you will revisit diff.h > in the final patch, so I'll try to see if there are effects there. Later in diff.h there is a struct defined which includes a direct 'struct strbuf'. As such, a forward declaration is incorrect, and the code only worked previously because diff.h included strbuf.h indirectly (through object.h -> cache.h -> strbuf.h). Now that object.h no longer includes cache.h, we have to correct this error. > > diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h > > index 1fe393f4473..ef03b45132e 100644 > > --- a/list-objects-filter-options.h > > +++ b/list-objects-filter-options.h > > @@ -1,9 +1,10 @@ > > #ifndef LIST_OBJECTS_FILTER_OPTIONS_H > > #define LIST_OBJECTS_FILTER_OPTIONS_H > > > > -#include "cache.h" > > +#include "object.h" > > #include "parse-options.h" > > #include "string-list.h" > > +#include "strbuf.h" > > Here's another case of including strbuf.h instead of declaring > 'struct strbuf'. However, it makes a bit more sense because you > are deleting the cache.h include in this change. It would still > be nice if we didn't need to do it. Same issue; struct list_objects_filter_options includes an embedded struct strbuf, so the strbuf header needs to be included. I'll call these out specifically in the commit message.
