| Message ID | 1357f18db5601cf4fc68a56a2e846d2aed6ad6b6.1677139522.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Header cleanups | expand |
On 2/23/2023 3:05 AM, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > 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. > 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. Thanks, -Stolee
On Thu, Feb 23, 2023 at 6:17 AM Derrick Stolee <derrickstolee@github.com> wrote: > > On 2/23/2023 3:05 AM, Elijah Newren via GitGitGadget wrote: > > From: Elijah Newren <newren@gmail.com> > > > > 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.
diff --git a/alloc.c b/alloc.c index 27f697e4c87..2886aa93543 100644 --- a/alloc.c +++ b/alloc.c @@ -8,7 +8,7 @@ * up with maximal alignment because it doesn't know what the object alignment * for the new allocation is. */ -#include "cache.h" +#include "git-compat-util.h" #include "object.h" #include "blob.h" #include "tree.h" diff --git a/blame.h b/blame.h index 38bde535b3d..b60d1d81e30 100644 --- a/blame.h +++ b/blame.h @@ -1,7 +1,6 @@ #ifndef BLAME_H #define BLAME_H -#include "cache.h" #include "commit.h" #include "xdiff-interface.h" #include "revision.h" diff --git a/blob.c b/blob.c index 8f83523b0cd..888e28a5594 100644 --- a/blob.c +++ b/blob.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "git-compat-util.h" #include "blob.h" #include "repository.h" #include "alloc.h" diff --git a/cache-tree.h b/cache-tree.h index bd97caa07b0..faae88be63c 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -1,7 +1,6 @@ #ifndef CACHE_TREE_H #define CACHE_TREE_H -#include "cache.h" #include "tree.h" #include "tree-walk.h" diff --git a/cache.h b/cache.h index 63ffd197a2a..af15701b063 100644 --- a/cache.h +++ b/cache.h @@ -15,6 +15,7 @@ #include "hash.h" #include "hex.h" #include "path.h" +#include "object.h" #include "oid-array.h" #include "repository.h" #include "mem-pool.h" @@ -454,26 +455,6 @@ void prefetch_cache_entries(const struct index_state *istate, extern struct index_state the_index; #endif -#define TYPE_BITS 3 - -/* - * Values in this enum (except those outside the 3 bit range) are part - * of pack file format. See gitformat-pack(5) for more information. - */ -enum object_type { - OBJ_BAD = -1, - OBJ_NONE = 0, - OBJ_COMMIT = 1, - OBJ_TREE = 2, - OBJ_BLOB = 3, - OBJ_TAG = 4, - /* 5 for future expansion */ - OBJ_OFS_DELTA = 6, - OBJ_REF_DELTA = 7, - OBJ_ANY, - OBJ_MAX -}; - static inline enum object_type object_type(unsigned int mode) { return S_ISDIR(mode) ? OBJ_TREE : diff --git a/diff-merges.c b/diff-merges.c index faa7bc73a34..ec97616db1d 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -1,6 +1,7 @@ #include "git-compat-util.h" #include "diff-merges.h" +#include "gettext.h" #include "revision.h" typedef void (*diff_merges_setup_func_t)(struct rev_info *); 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; typedef int (*pathchange_fn_t)(struct diff_options *options, diff --git a/diffcore-delta.c b/diffcore-delta.c index 18d8f766d70..c30b56e983b 100644 --- a/diffcore-delta.c +++ b/diffcore-delta.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "git-compat-util.h" #include "diff.h" #include "diffcore.h" diff --git a/fsck.h b/fsck.h index 668330880ef..e17730e9da9 100644 --- a/fsck.h +++ b/fsck.h @@ -1,6 +1,7 @@ #ifndef GIT_FSCK_H #define GIT_FSCK_H +#include "object.h" #include "oidset.h" enum fsck_msg_type { diff --git a/help.c b/help.c index 5f84a50b948..216777d2bf4 100644 --- a/help.c +++ b/help.c @@ -5,6 +5,7 @@ #include "exec-cmd.h" #include "run-command.h" #include "levenshtein.h" +#include "gettext.h" #include "help.h" #include "command-list.h" #include "string-list.h" 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" /* * The list of defined filters for list-objects. diff --git a/negotiator/noop.c b/negotiator/noop.c index 60569b83501..7b729376867 100644 --- a/negotiator/noop.c +++ b/negotiator/noop.c @@ -1,4 +1,4 @@ -#include "cache.h" +#include "git-compat-util.h" #include "noop.h" #include "../commit.h" #include "../fetch-negotiator.h" diff --git a/object.h b/object.h index 31ebe114585..fc45b158da0 100644 --- a/object.h +++ b/object.h @@ -1,7 +1,7 @@ #ifndef OBJECT_H #define OBJECT_H -#include "cache.h" +#include "hash.h" struct buffer_slab; @@ -81,6 +81,26 @@ struct object_array { */ #define FLAG_BITS 28 +#define TYPE_BITS 3 + +/* + * Values in this enum (except those outside the 3 bit range) are part + * of pack file format. See gitformat-pack(5) for more information. + */ +enum object_type { + OBJ_BAD = -1, + OBJ_NONE = 0, + OBJ_COMMIT = 1, + OBJ_TREE = 2, + OBJ_BLOB = 3, + OBJ_TAG = 4, + /* 5 for future expansion */ + OBJ_OFS_DELTA = 6, + OBJ_REF_DELTA = 7, + OBJ_ANY, + OBJ_MAX +}; + /* * The object type is stored in 3 bits. */ diff --git a/shallow.h b/shallow.h index aba6ff58294..e9ca7e4bc80 100644 --- a/shallow.h +++ b/shallow.h @@ -6,6 +6,8 @@ #include "repository.h" #include "strbuf.h" +struct oid_array; + void set_alternate_shallow_file(struct repository *r, const char *path, int override); int register_shallow(struct repository *r, const struct object_id *oid); int unregister_shallow(const struct object_id *oid); diff --git a/t/helper/test-bloom.c b/t/helper/test-bloom.c index 6c900ca6684..010ea5148e4 100644 --- a/t/helper/test-bloom.c +++ b/t/helper/test-bloom.c @@ -1,4 +1,4 @@ -#include "git-compat-util.h" +#include "cache.h" #include "bloom.h" #include "test-tool.h" #include "commit.h" diff --git a/t/helper/test-example-decorate.c b/t/helper/test-example-decorate.c index b9d1200eb98..7c7fc8efc13 100644 --- a/t/helper/test-example-decorate.c +++ b/t/helper/test-example-decorate.c @@ -1,5 +1,5 @@ #include "test-tool.h" -#include "cache.h" +#include "git-compat-util.h" #include "object.h" #include "decorate.h" diff --git a/worktree.h b/worktree.h index 9dcea6fc8c1..2baeca2a8a6 100644 --- a/worktree.h +++ b/worktree.h @@ -1,7 +1,6 @@ #ifndef WORKTREE_H #define WORKTREE_H -#include "cache.h" #include "refs.h" struct strbuf;