Re: [PATCH 10/16] object.h: stop depending on cache.h; make cache.h depend on object.h

From: Derrick Stolee <derrickstolee@github.com>
Date: Feb 23, 2023, 14:17 UTC
Message-ID: <e2948915-8505-2657-7283-e845804dde0d@github.com>
In-Reply-To: <1357f18db5601cf4fc68a56a2e846d2aed6ad6b6.1677139522.git.gitgitgadget@gmail.com>

On 2/23/2023 3:05 AM, Elijah Newren via GitGitGadget wrote:

Show 9 quoted lines

> 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.

Show 18 quoted lines

> 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.

Show 13 quoted lines

> 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