Additional header cleanups (removing unnecessary includes) #1617
Conversation
newren
force-pushed
the
header-cleanup-6
branch
2 times, most recently
from
a88f2d3
to
c9d409c
Compare
There are three kinds of unnecessary includes:
* includes which aren't directly needed, but which include some other
forgotten include
* includes which could be replaced by a simple forward declaration of
some structs
* includes which aren't needed at all
Remove the third kind of include. Subsequent commits (and a subsequent
series) will work on removing some of the other kinds of includes.
Signed-off-by: Elijah Newren <newren@gmail.com>
newren
force-pushed
the
header-cleanup-6
branch
from
c9d409c
to
57f9da7
Compare
newren
changed the title
|
/submit |
|
Submitted as pull.1617.git.1701585682.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| @@ -12,7 +12,6 @@ | |||
| #include "strvec.h" | |||
There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/trace2.c b/trace2.c
> index 6dc74dff4c7..d4220af9ae1 100644
> --- a/trace2.c
> +++ b/trace2.c
> @@ -1,12 +1,8 @@
> #include "git-compat-util.h"
> -#include "config.h"
> -#include "json-writer.h"
> -#include "quote.h"
> #include "repository.h"
> #include "run-command.h"
> #include "sigchain.h"
> #include "thread-utils.h"
> -#include "version.h"
> #include "trace.h"
> #include "trace2.h"
> #include "trace2/tr2_cfg.h"
An in-flight topic seem to want to see git_env_bool() that is
declared in parse.h that is pulled in via inclusion of config.h
hence this hunk breaks 'seen'.
> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
> index d5ca0046c89..a0032ee3964 100644
> --- a/t/helper/test-trace2.c
> +++ b/t/helper/test-trace2.c
> @@ -2,7 +2,6 @@
> #include "strvec.h"
> #include "run-command.h"
> #include "exec-cmd.h"
> -#include "config.h"
> #include "repository.h"
> #include "trace2.h"
An in-flight topic starts using "struct key_value_info" that is
available via the inclusion of "config.h", hence this hunk breaks
the build of 'seen'.
> diff --git a/t/helper/test-fast-rebase.c b/t/helper/test-fast-rebase.c
> index cac20a72b3f..f9472c99143 100644
> --- a/t/helper/test-fast-rebase.c
> +++ b/t/helper/test-fast-rebase.c
> @@ -24,7 +24,6 @@
> #include "read-cache-ll.h"
> #include "refs.h"
> #include "revision.h"
> -#include "sequencer.h"
> #include "setup.h"
> #include "strvec.h"
> #include "tree.h"
I'll register the following evil merge as the merge-fix/ for this
topic.
In addition, t/helper/test-fast-rebase.c that is touched by this
step will simply disappear with the cc/git-replay topic, so it may
not be a bad idea to exclude it from the patchset.
Thanks.
diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
index 16089f04e1..55c06e4269 100644
--- a/t/helper/test-trace2.c
+++ b/t/helper/test-trace2.c
@@ -1,4 +1,5 @@
#include "test-tool.h"
+#include "config-parse.h"
#include "strvec.h"
#include "run-command.h"
#include "exec-cmd.h"
diff --git a/trace2.c b/trace2.c
index 4fa059199c..452428b09b 100644
--- a/trace2.c
+++ b/trace2.c
@@ -1,4 +1,5 @@
#include "git-compat-util.h"
+#include "parse.h"
#include "repository.h"
#include "run-command.h"
#include "sigchain.h"There was a problem hiding this comment.
On the Git mailing list, Junio C Hamano wrote (reply to this):
Junio C Hamano <gitster@pobox.com> writes:
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> diff --git a/trace2.c b/trace2.c
>> index 6dc74dff4c7..d4220af9ae1 100644
>> --- a/trace2.c
>> ...
> An in-flight topic seem to want to see git_env_bool() that is
> declared in parse.h that is pulled in via inclusion of config.h
> hence this hunk breaks 'seen'.
>
>> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
>> index d5ca0046c89..a0032ee3964 100644
>> --- a/t/helper/test-trace2.c
>> ...
> An in-flight topic starts using "struct key_value_info" that is
> available via the inclusion of "config.h", hence this hunk breaks
> the build of 'seen'.
It seems that we have gained another topic in flight that gets
broken by this change. I can keep piling merge-fixes on top, but it
does not look like a strategy that would scale well.
Can we get this series thoroughly reviewed quickly to merge it down
via 'next' to 'master' soonish, so that other topics can be rebased
on the result, or is that too much to ask during the Winter lull?
Thanks.There was a problem hiding this comment.
On the Git mailing list, Elijah Newren wrote (reply to this):
On Wed, Dec 20, 2023 at 11:34 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >> diff --git a/trace2.c b/trace2.c
> >> index 6dc74dff4c7..d4220af9ae1 100644
> >> --- a/trace2.c
> >> ...
> > An in-flight topic seem to want to see git_env_bool() that is
> > declared in parse.h that is pulled in via inclusion of config.h
> > hence this hunk breaks 'seen'.
> >
> >> diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
> >> index d5ca0046c89..a0032ee3964 100644
> >> --- a/t/helper/test-trace2.c
> >> ...
> > An in-flight topic starts using "struct key_value_info" that is
> > available via the inclusion of "config.h", hence this hunk breaks
> > the build of 'seen'.
>
> It seems that we have gained another topic in flight that gets
> broken by this change. I can keep piling merge-fixes on top, but it
> does not look like a strategy that would scale well.
>
> Can we get this series thoroughly reviewed quickly to merge it down
> via 'next' to 'master' soonish, so that other topics can be rebased
> on the result, or is that too much to ask during the Winter lull?
>
> Thanks.
The Winter lull is my winter surge, so I can certainly quickly make
whatever changes are required (well, assuming I can shake this
fever...). But that doesn't help much with reviewing, since that
should be done by someone other than the author. However, these
particular type of changes are pretty innocuous; there's really not
anything clever going on, it's just a lot of gruntwork, and
does-it-compile is most of the review.
Anyway, I'll reroll, dropping or holding back any changes that
conflict with next or seen, and see if that encourages anyone to chime
in.
|
This branch is now known as |
|
This patch series was integrated into seen via git@c921012. |
|
There was a status update in the "New Topics" section about the branch Remove unused header "#include". Has a few interactions with topics in flight. source: <pull.1617.git.1701585682.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@30fdd87. |
|
This patch series was integrated into seen via git@e805b6f. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Has a few interactions with topics in flight. source: <pull.1617.git.1701585682.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@e7dd90b. |
|
This patch series was integrated into seen via git@e850ab5. |
|
This patch series was integrated into seen via git@050f7d8. |
|
This patch series was integrated into seen via git@3bc315c. |
|
This patch series was integrated into seen via git@60801d6. |
|
This patch series was integrated into seen via git@0e79e20. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Has a few interactions with topics in flight. source: <pull.1617.git.1701585682.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@9d58804. |
|
This patch series was integrated into seen via git@e052b39. |
|
User |
|
This patch series was integrated into seen via git@6ada328. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Has a few interactions with topics in flight. source: <pull.1617.git.1701585682.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@0df8c6a. |
Each of these were checked with
gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE}
to ensure that removing the direct inclusion of the header actually
resulted in that header no longer being included at all (i.e. that
no other header pulled it in transitively).
...except for a few cases where we verified that although the header
was brought in transitively, nothing from it was directly used in
that source file. These cases were:
* builtin/credential-cache.c
* builtin/pull.c
* builtin/send-pack.c
Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The unnecessary include in the header transitively pulled in some other headers actually needed by source files, though. Have those source files explicitly include the headers they need. Signed-off-by: Elijah Newren <newren@gmail.com>
The next commit will remove a bunch of unnecessary includes, but to do so, we need some of the lower level direct includes that files rely on to be explicitly specified. Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
newren
force-pushed
the
header-cleanup-6
branch
from
57f9da7
to
45f893e
Compare
|
/submit |
|
Submitted as pull.1617.v2.git.1703351700.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@03af80b. |
|
This patch series was integrated into seen via git@9dc33c9. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Will merge to 'next'. source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@8d37d73. |
|
This patch series was integrated into next via git@1ccddc2. |
|
This patch series was integrated into seen via git@944e794. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Will merge to 'master'. source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@97a62a5. |
|
This patch series was integrated into seen via git@7473779. |
|
This patch series was integrated into seen via git@d2dc98b. |
|
There was a status update in the "Cooking" section about the branch Remove unused header "#include". Will merge to 'master'. source: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@492ee03. |
|
This patch series was integrated into master via git@492ee03. |
|
This patch series was integrated into next via git@492ee03. |
|
Closed via 492ee03. |
Here are a number of header cleanups, to remove unnecessary includes.
(Sorry for the multiple day delay from when I said I'd send in an update; COVID sucks and knocked me out for multiple days and has me running on near empty.)
Changes since v1:
cc: Elijah Newren newren@gmail.com