Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2] t1006: add tests for %(objectsize:disk)
From: René Scharfe @ 2023-12-24  9:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Ondrej Pohorelsky, brian m . carlson, Junio C Hamano
In-Reply-To: <20231223100905.GB2016274@coredump.intra.peff.net>

Am 23.12.23 um 11:09 schrieb Jeff King:
>
> ---
>  t/t1006-cat-file.sh | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
> index 271c5e4fd3..e0c6482797 100755
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -1100,6 +1100,42 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will wor
>  	cmp expect actual
>  '
>
> +test_expect_success 'cat-file %(objectsize:disk) with --batch-all-objects' '
> +	# our state has both loose and packed objects,
> +	# so find both for our expected output
> +	{
> +		find .git/objects/?? -type f |
> +		awk -F/ "{ print \$0, \$3\$4 }" |
> +		while read path oid
> +		do
> +			size=$(test_file_size "$path") &&
> +			echo "$oid $size" ||
> +			return 1
> +		done &&
> +		rawsz=$(test_oid rawsz) &&
> +		find .git/objects/pack -name "*.idx" |
> +		while read idx
> +		do
> +			git show-index <"$idx" >idx.raw &&
> +			sort -nr <idx.raw >idx.sorted &&
> +			packsz=$(test_file_size "${idx%.idx}.pack") &&
> +			end=$((packsz - rawsz)) &&
> +			while read start oid rest
> +			do
> +				size=$((end - start)) &&
> +				end=$start &&
> +				echo "$oid $size" ||
> +				return 1
> +			done <idx.sorted ||
> +			return 1
> +		done
> +	} >expect.raw &&
> +	sort <expect.raw >expect &&
> +	git cat-file --batch-all-objects \
> +		--batch-check="%(objectname) %(objectsize:disk)" >actual &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'set up replacement object' '
>  	orig=$(git rev-parse HEAD) &&
>  	git cat-file commit $orig >orig &&

Looks good to me.

René

^ permalink raw reply

* Re: [PATCH] mem-pool: fix big allocations
From: René Scharfe @ 2023-12-24  9:30 UTC (permalink / raw)
  To: Elijah Newren; +Cc: Git List, Jameson Miller
In-Reply-To: <CABPp-BFiK9A6426d7CFeMZvaBcvmShaY9O0krtCe7jeV9wW7nQ@mail.gmail.com>

Am 24.12.23 um 04:11 schrieb Elijah Newren:
> On Thu, Dec 21, 2023 at 3:13 PM René Scharfe <l.s.r@web.de> wrote:
>>
>> Add a basic unit test to demonstate the issue by using mem_pool_calloc()
>
> demonstrate (missing 'r')?

Yes.  I didn't intend to mention any demons or states, let alone a
demon state. *facepalm*

> Nice catch; looks good to me.  Out of curiosity, how'd you find the issue?

I was working on using a memory pool for name-rev.  I played with
different values for block_alloc, not fully sure why anymore -- either
for checking the performance impact or testing robustness.

René

^ permalink raw reply

* Re: [PATCH] sparse-checkout: be consistent with end of options markers
From: Jeff King @ 2023-12-24  8:32 UTC (permalink / raw)
  To: Elijah Newren via GitGitGadget; +Cc: git, Randall S. Becker, Elijah Newren
In-Reply-To: <pull.1625.git.git.1703379611749.gitgitgadget@gmail.com>

On Sun, Dec 24, 2023 at 01:00:11AM +0000, Elijah Newren via GitGitGadget wrote:

> Remove the erroneous PARSE_OPT_KEEP_UNKNOWN_OPT flag now to fix this
> bug.  Note that this does mean that anyone who might have been using
> [...]

Thanks for wrapping this up in patch form. It looks good to me,
including the reasoning. You didn't add any tests, but I find it rather
unlikely that we'd later regress here.

-Peff

^ permalink raw reply

* Re: [PATCH 1/3] sparse-checkout: take care of "--end-of-options" in set/add/check-rules
From: Elijah Newren @ 2023-12-24  7:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Josh Steadmon
In-Reply-To: <20231221065925.3234048-2-gitster@pobox.com>

On Wed, Dec 20, 2023 at 11:00 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> 93851746 (parse-options: decouple "--end-of-options" and "--",
> 2023-12-06) updated the world order to make callers of parse-options
> that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
> do with "--end-of-options" they may see after parse_options() returns.
>
> This unfortunately broke "sparse-checkout set/add", and from this
> invocation,
>
>   "git sparse-checkout [add|set] --[no-]cone --end-of-options pattern..."
>
> we now see "--end-of-options" listed in .git/info/sparse-checkout as if
> it is one of the path patterns.
>
> A breakage that results from the same cause exists in the check-rules
> subcommand, but check-rules has a few other problems that need to be
> corrected before it can fully work with --end-of-options safely,
> which will be addressed later.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/sparse-checkout.c          | 3 +++
>  parse-options.c                    | 8 ++++++++
>  parse-options.h                    | 2 ++
>  t/t1090-sparse-checkout-scope.sh   | 8 ++++++++
>  t/t1091-sparse-checkout-builtin.sh | 2 +-
>  5 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 5c8ffb1f75..8f55127202 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -779,6 +779,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
>                              builtin_sparse_checkout_add_options,
>                              builtin_sparse_checkout_add_usage,
>                              PARSE_OPT_KEEP_UNKNOWN_OPT);
> +       parse_opt_skip_end_of_options(&argc, &argv);
>
>         sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
>
> @@ -826,6 +827,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>                              builtin_sparse_checkout_set_options,
>                              builtin_sparse_checkout_set_usage,
>                              PARSE_OPT_KEEP_UNKNOWN_OPT);
> +       parse_opt_skip_end_of_options(&argc, &argv);
>
>         if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
>                 return 1;
> @@ -998,6 +1000,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
>                              builtin_sparse_checkout_check_rules_options,
>                              builtin_sparse_checkout_check_rules_usage,
>                              PARSE_OPT_KEEP_UNKNOWN_OPT);
> +       parse_opt_skip_end_of_options(&argc, &argv);
>
>         if (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)
>                 check_rules_opts.cone_mode = 1;
> diff --git a/parse-options.c b/parse-options.c
> index d50962062e..fe265bbf68 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1321,3 +1321,11 @@ void die_for_incompatible_opt4(int opt1, const char *opt1_name,
>                 break;
>         }
>  }
> +
> +void parse_opt_skip_end_of_options(int *argc, const char ***argv)
> +{
> +       if (*argc && !strcmp(**argv, "--end-of-options")) {
> +               (*argc)--;
> +               (*argv)++;
> +       }
> +}
> diff --git a/parse-options.h b/parse-options.h
> index bd62e20268..0d3354d4a8 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -498,6 +498,8 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
>  /* value is enum branch_track* */
>  int parse_opt_tracking_mode(const struct option *, const char *, int);
>
> +void parse_opt_skip_end_of_options(int *argc, const char ***argv);
> +
>  #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', "verbose", (var), (h))
>  #define OPT__QUIET(var, h)    OPT_COUNTUP('q', "quiet",   (var), (h))
>  #define OPT__VERBOSITY(var) { \
> diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh
> index 3a14218b24..5b96716235 100755
> --- a/t/t1090-sparse-checkout-scope.sh
> +++ b/t/t1090-sparse-checkout-scope.sh
> @@ -57,6 +57,14 @@ test_expect_success 'return to full checkout of main' '
>  test_expect_success 'skip-worktree on files outside sparse patterns' '
>         git sparse-checkout disable &&
>         git sparse-checkout set --no-cone "a*" &&
> +       cat .git/info/sparse-checkout >wo-eoo &&
> +
> +       git sparse-checkout disable &&
> +       git sparse-checkout set --no-cone --end-of-options "a*" &&
> +       cat .git/info/sparse-checkout >w-eoo &&
> +
> +       test_cmp wo-eoo w-eoo &&
> +
>         git checkout-index --all --ignore-skip-worktree-bits &&
>
>         git ls-files -t >output &&
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index f67611da28..e33a6ed1b4 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -334,7 +334,7 @@ test_expect_success 'cone mode: set with nested folders' '
>
>  test_expect_success 'cone mode: add independent path' '
>         git -C repo sparse-checkout set deep/deeper1 &&
> -       git -C repo sparse-checkout add folder1 &&
> +       git -C repo sparse-checkout add --end-of-options folder1 &&
>         cat >expect <<-\EOF &&
>         /*
>         !/*/
> --
> 2.43.0-174-g055bb6e996

I've got a counter-proposal for this patch at
https://lore.kernel.org/git/pull.1625.git.git.1703379611749.gitgitgadget@gmail.com/,
based on further thread discussion over at
https://lore.kernel.org/git/CABPp-BF9XZeESHdxdcZ91Vsn5tKqQ6_3tC11e7t9vTFp=uufbg@mail.gmail.com/.

^ permalink raw reply

* Re: [PATCH 2/3] sparse-checkout: use default patterns for 'set' only !stdin
From: Elijah Newren @ 2023-12-24  7:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <20231221065925.3234048-3-gitster@pobox.com>

On Wed, Dec 20, 2023 at 10:59 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "git sparse-checkout set ---no-cone" uses default patterns when none
> is given from the command line, but it should do so ONLY when
> --stdin is not being used.  Right now, add_patterns_from_input()
> called when reading from the standard input is sloppy and does not
> check if there are extra command line parameters that the command
> will silently ignore, but that will change soon and not setting this
> unnecessary and unused default patterns start to matter when it gets
> fixed.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  * This came from f2e3a218 (sparse-checkout: enable `set` to
>    initialize sparse-checkout mode, 2021-12-14) by Elijah.
>
>  builtin/sparse-checkout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 8f55127202..04ae81fce8 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -837,7 +837,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
>          * non-cone mode, if nothing is specified, manually select just the
>          * top-level directory (much as 'init' would do).
>          */
> -       if (!core_sparse_checkout_cone && argc == 0) {
> +       if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
>                 argv = default_patterns;
>                 argc = default_patterns_nr;
>         } else {
> --
> 2.43.0-174-g055bb6e996

Thanks for catching this; the fix looks good to me.

^ permalink raw reply

* Re: Git Rename Detection Bug
From: Elijah Newren @ 2023-12-24  7:46 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Jeremy Pridmore, git@vger.kernel.org, Paul Baumgartner
In-Reply-To: <990ab7d5-e29a-4766-b112-c8908a7ed196@iee.email>

Hi Philip,

Sorry for the late reply; I somehow missed this earlier.

On Wed, Nov 15, 2023 at 8:51 AM Philip Oakley <philipoakley@iee.email> wrote:
>
> Hi Elijah,
>
> On 11/11/2023 05:46, Elijah Newren wrote:
> > * filename similarity is extraordinarily expensive compared to exact
> > renames, and if not carefully handled, can sometimes rival the cost of
> > file content similarity computations given our spanhash
> > representations.
>
> I've not heard of spanhash representation before. Any references or
> further reading?

You can find more in diffcore-delta.c, especially the big comment near
the top of the file.  But here's a short explanation of spanhashes:
  * Split files into chunks delimited either by LF or 64 bytes,
whichever comes first.
  * Hash every chunk into an integer between 0 and 107926
  * Keep a character count for each of those integers as well (thus if
a line has N characters, but appears twice in the file, the associated
count for that integer will be 2N).
  * A "spanhash" is the combination of the integer that a chunk (or
span) hashes to, plus the count associated with it.
  * The list/array of spanhashes for a file (i.e. the list/array of
integers and character counts) is used to compare one file to another.

Now, why do I claim that comparison of filenames can rival cost of
file content similarity?  Well, in a monorepo of interest, the median
sized file is named something close to
"modules/client-resources/src/main/resources/buttons/smallTriangleBlackRight.png"
and is 2875 bytes.  As a png, all its chunks are probably the full 64
characters, which works out to about 45 chunks (assuming the 64-byte
chunks are different from each other).  The filename is 79 characters.
So, for this case, 45 pairs of integers vs 79 characters.  So, the
comparison cost is roughly the same order of magnitude.
(Yes, creating the spanhashes is a heavy overhead; however, we only
initialize it once and then do N comparisons of each spanhash to the
other spanhashes.  And we'd be doing N comparisons of each filename to
other filenames, so the overhead of creating the spanhashes can be
overlooked if your merge has enough files modified on both sides of
history.)

Yes, this particular repository is a case I randomly picked that you
can argue is special.  But rather than look at the particular example,
I think it's interesting to check how the spanhash size vs. filename
size scale with repository size.  From my experience: (1) I don't
think the median-sized file varies all that much between small and big
repositories; every time I check a repo the median size seems to be
order of a few thousand bytes, regardless of whether the repository
I'm looking at is tiny or huge, (2) while small repositories often
have much shorter filenames, big repositories often will have
filenames even longer than my example; length of filename tends to
grow with repository size from deep directory nestings.  So, between
these two facts, I'd expect the filename comparison costs to grow
relative to file content comparison costs, when considering only
median-sized files being modified.  And since it's common to have
merges or rebases or diffs where only approximately-median-sized files
are involved, I think this is relevant to look at.  Finally, since I
already had an example that showed the cost likely roughly comparable
for a random repository of interest, and it's not even all that big a
repository compared to many out there, I think the combination
motivates pretty well my claim that filename similarity costs _could_
rival file content similarity costs if one wasn't careful.

I don't have a rigorous proof here.  And, in fact, I ended up doing
this rough back-of-the-envelope analysis _after_ implementing some
filename similarity comparison ideas and seeing performance degrade
badly, and wondering why it made such a difference.  I don't know if I
ever got exact numbers, but I certainly didn't record them.  This
rough analysis, though, was what made me realize that I needed to be
careful with any such added filename comparisons, though, and is why
I'm leery of adding more.

^ permalink raw reply

* Re: [PATCH] mem-pool: fix big allocations
From: Elijah Newren @ 2023-12-24  3:11 UTC (permalink / raw)
  To: René Scharfe; +Cc: Git List, Jameson Miller
In-Reply-To: <fa89d269-1a23-4ed6-bebc-30c0b629f444@web.de>

On Thu, Dec 21, 2023 at 3:13 PM René Scharfe <l.s.r@web.de> wrote:
>
> Memory pool allocations that require a new block and would fill at
> least half of it are handled specially.  Before 158dfeff3d (mem-pool:
> add life cycle management functions, 2018-07-02) they used to be
> allocated outside of the pool.  This patch made mem_pool_alloc() create
> a bespoke block instead, to allow releasing it when the pool gets
> discarded.
>
> Unfortunately mem_pool_alloc() returns a pointer to the start of such a
> bespoke block, i.e. to the struct mp_block at its top.  When the caller
> writes to it, the management information gets corrupted.  This affects
> mem_pool_discard() and -- if there are no other blocks in the pool --
> also mem_pool_alloc().
>
> Return the payload pointer of bespoke blocks, just like for smaller
> allocations, to protect the management struct.
>
> Also update next_free to mark the block as full.  This is only strictly
> necessary for the first allocated block, because subsequent ones are
> inserted after the current block and never considered for further
> allocations, but it's easier to just do it in all cases.
>
> Add a basic unit test to demonstate the issue by using mem_pool_calloc()

demonstrate (missing 'r')?

> with a tiny block size, which forces the creation of a bespoke block.
> Without the mem_pool_alloc() fix it reports zeroed pointers:
>
>    ok 1 - mem_pool_calloc returns 100 zeroed bytes with big block
>    # check "((pool->mp_block->next_free) != (((void*)0))) == 1" failed at t/unit-tests/t-mem-pool.c:22
>    #    left: 0
>    #   right: 1
>    # check "((pool->mp_block->end) != (((void*)0))) == 1" failed at t/unit-tests/t-mem-pool.c:23
>    #    left: 0
>    #   right: 1
>    not ok 2 - mem_pool_calloc returns 100 zeroed bytes with tiny block
>    1..2
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  Makefile                  |  1 +
>  mem-pool.c                |  6 +++---
>  t/unit-tests/t-mem-pool.c | 34 ++++++++++++++++++++++++++++++++++
>  3 files changed, 38 insertions(+), 3 deletions(-)
>  create mode 100644 t/unit-tests/t-mem-pool.c
>
> diff --git a/Makefile b/Makefile
> index 88ba7a3c51..15990ff312 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1340,6 +1340,7 @@ THIRD_PARTY_SOURCES += sha1collisiondetection/%
>  THIRD_PARTY_SOURCES += sha1dc/%
>
>  UNIT_TEST_PROGRAMS += t-basic
> +UNIT_TEST_PROGRAMS += t-mem-pool
>  UNIT_TEST_PROGRAMS += t-strbuf
>  UNIT_TEST_PROGS = $(patsubst %,$(UNIT_TEST_BIN)/%$X,$(UNIT_TEST_PROGRAMS))
>  UNIT_TEST_OBJS = $(patsubst %,$(UNIT_TEST_DIR)/%.o,$(UNIT_TEST_PROGRAMS))
> diff --git a/mem-pool.c b/mem-pool.c
> index c34846d176..e8d976c3ee 100644
> --- a/mem-pool.c
> +++ b/mem-pool.c
> @@ -99,9 +99,9 @@ void *mem_pool_alloc(struct mem_pool *pool, size_t len)
>
>         if (!p) {
>                 if (len >= (pool->block_alloc / 2))
> -                       return mem_pool_alloc_block(pool, len, pool->mp_block);
> -
> -               p = mem_pool_alloc_block(pool, pool->block_alloc, NULL);
> +                       p = mem_pool_alloc_block(pool, len, pool->mp_block);
> +               else
> +                       p = mem_pool_alloc_block(pool, pool->block_alloc, NULL);
>         }
>
>         r = p->next_free;
> diff --git a/t/unit-tests/t-mem-pool.c b/t/unit-tests/t-mem-pool.c
> new file mode 100644
> index 0000000000..2295779b0b
> --- /dev/null
> +++ b/t/unit-tests/t-mem-pool.c
> @@ -0,0 +1,34 @@
> +#include "test-lib.h"
> +#include "mem-pool.h"
> +
> +#define check_ptr(a, op, b) check_int(((a) op (b)), ==, 1)
> +
> +static void setup_static(void (*f)(struct mem_pool *), size_t block_alloc)
> +{
> +       struct mem_pool pool = { .block_alloc = block_alloc };
> +       f(&pool);
> +       mem_pool_discard(&pool, 0);
> +}
> +
> +static void t_calloc_100(struct mem_pool *pool)
> +{
> +       size_t size = 100;
> +       char *buffer = mem_pool_calloc(pool, 1, size);
> +       for (size_t i = 0; i < size; i++)
> +               check_int(buffer[i], ==, 0);
> +       if (!check_ptr(pool->mp_block, !=, NULL))
> +               return;
> +       check_ptr(pool->mp_block->next_free, <=, pool->mp_block->end);
> +       check_ptr(pool->mp_block->next_free, !=, NULL);
> +       check_ptr(pool->mp_block->end, !=, NULL);
> +}
> +
> +int cmd_main(int argc, const char **argv)
> +{
> +       TEST(setup_static(t_calloc_100, 1024 * 1024),
> +            "mem_pool_calloc returns 100 zeroed bytes with big block");
> +       TEST(setup_static(t_calloc_100, 1),
> +            "mem_pool_calloc returns 100 zeroed bytes with tiny block");
> +
> +       return test_done();
> +}
> --
> 2.43.0

Nice catch; looks good to me.  Out of curiosity, how'd you find the issue?

^ permalink raw reply

* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Elijah Newren @ 2023-12-24  1:02 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Josh Steadmon, git
In-Reply-To: <CABPp-BF9XZeESHdxdcZ91Vsn5tKqQ6_3tC11e7t9vTFp=uufbg@mail.gmail.com>

On Sat, Dec 23, 2023 at 2:45 PM Elijah Newren <newren@gmail.com> wrote:
>
> Hi,
>
> On Sat, Dec 23, 2023 at 2:02 AM Jeff King <peff@peff.net> wrote:
> >
> > On Thu, Dec 21, 2023 at 02:04:56PM -0800, Junio C Hamano wrote:
> >
> > > Jeff King <peff@peff.net> writes:
> > >
> > > > Right, that is the "gotcha" I mentioned in my other email. Though that
> > > > is the way it has behaved historically, my argument is that users are
> > > > unreasonable to expect it to work:
> > > >
> > > >   1. It is not consistent with the rest of Git commands.
> > > >
> > > >   2. It is inconsistent with respect to existing options (and is an
> > > >      accident waiting to happen when new options are added).
> > > >
> > > > So I'd consider it a bug-fix.
> > >
> > > So the counter-proposal here is just to drop KEEP_UNKNOWN_OPT and
> > > deliberately break them^W^W^Wrealign their expectations?
> >
> > Yes. :) But keep in mind we are un-breaking other people, like those who
> > typo:
> >
> >   git sparse-checkout --sikp-checks
> >
> > and don't see an error (instead, we make a garbage entry in the sparse
> > checkout file).
>
> I think you mean
>      git sparse-checkout set --sikp-checks
> or
>      git sparse-checkout add --sikp-checks
> (i.e. with either 'set' or 'add' in there)
>
> Neither of these are currently an error, but I agree both definitely
> ought to be.  (Without the 'set' or 'add', you currently do get an
> error, as we'd expect.)
>
> > > I do not have much stake in sparse-checkout, so I am fine with that
> > > direction.  But I suspect other folks on the list would have users
> > > of their own who would rather loudly complain to them if we broke
> > > them ;-)
> >
> > Likewise, I have never used sparse-checkout myself, and I don't care
> > _that_ much. My interest is mostly in having various Git commands behave
> > consistently. This whole discussion started because the centralized
> > --end-of-options fix interacted badly with this unusual behavior.
>
> Well, I care quite a bit about sparse-checkout, and I would rather see
> Peff's proposed fix, even if some users might have to tweak their
> commands a little.

And I wrote it up as a patch over at
https://lore.kernel.org/git/pull.1625.git.git.1703379611749.gitgitgadget@gmail.com/

^ permalink raw reply

* [PATCH] sparse-checkout: be consistent with end of options markers
From: Elijah Newren via GitGitGadget @ 2023-12-24  1:00 UTC (permalink / raw)
  To: git; +Cc: Randall S. Becker, Jeff King, Elijah Newren, Elijah Newren

From: Elijah Newren <newren@gmail.com>

93851746 (parse-options: decouple "--end-of-options" and "--",
2023-12-06) updated the world order to make callers of parse-options
that set PARSE_OPT_KEEP_UNKNOWN_OPT responsible for deciding what to
do with "--end-of-options" they may see after parse_options() returns.

This made a previous bug in sparse-checkout more visible; namely,
that

  git sparse-checkout [add|set] --[no-]cone --end-of-options ...

would simply treat "--end-of-options" as one of the paths to include in
the sparse-checkout.  But this was already problematic before; namely,

  git sparse-checkout [add|set| --[no-]cone --sikp-checks ...

would not give an error on the mis-typed "--skip-checks" but instead
simply treat "--sikp-checks" as a path or pattern to include in the
sparse-checkout, which is highly unfriendly.

This behavior begain when the command was converted to parse-options in
7bffca95ea (sparse-checkout: add '--stdin' option to set subcommand,
2019-11-21).  Back then it was just called KEEP_UNKNOWN. Later it was
renamed to KEEP_UNKNOWN_OPT in 99d86d60e5 (parse-options:
PARSE_OPT_KEEP_UNKNOWN only applies to --options, 2022-08-19) to clarify
that it was only about dashed options; we always keep non-option
arguments.  Looking at that original patch, both Peff and I think that
the author was simply confused about the mis-named option, and really
just wanted to keep the non-option arguments.  We never should have used
the flag all along (and the other cases were cargo-culted within the
file).

Remove the erroneous PARSE_OPT_KEEP_UNKNOWN_OPT flag now to fix this
bug.  Note that this does mean that anyone who might have been using

  git sparse-checkout [add|set] [--[no-]cone] --foo --bar

to request paths or patterns '--foo' and '--bar' will now have to use

  git sparse-checkout [add|set] [--[no-]cone] -- --foo --bar

That makes sparse-checkout more consistent with other git commands,
provides users much friendlier error messages and behavior, and is
consistent with the all-capps warning in git-sparse-checkout.txt that
this command "is experimental...its behavior...will likely change".  :-)

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    [RFC] sparse-checkout: be consistent with end of options markers
    
    Follow-up to thread over at
    https://lore.kernel.org/git/CABPp-BF9XZeESHdxdcZ91Vsn5tKqQ6_3tC11e7t9vTFp=uufbg@mail.gmail.com/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1625%2Fgitgitgadget%2Fsparse-checkout-end-of-options-consistency-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1625/gitgitgadget/sparse-checkout-end-of-options-consistency-v1
Pull-Request: https://github.com/git/git/pull/1625

 builtin/sparse-checkout.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 5c8ffb1f759..0e68e9b0b0d 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -777,8 +777,7 @@ static int sparse_checkout_add(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_add_options,
-			     builtin_sparse_checkout_add_usage,
-			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+			     builtin_sparse_checkout_add_usage, 0);
 
 	sanitize_paths(argc, argv, prefix, add_opts.skip_checks);
 
@@ -824,8 +823,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_set_options,
-			     builtin_sparse_checkout_set_usage,
-			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+			     builtin_sparse_checkout_set_usage, 0);
 
 	if (update_modes(&set_opts.cone_mode, &set_opts.sparse_index))
 		return 1;
@@ -996,8 +994,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
 
 	argc = parse_options(argc, argv, prefix,
 			     builtin_sparse_checkout_check_rules_options,
-			     builtin_sparse_checkout_check_rules_usage,
-			     PARSE_OPT_KEEP_UNKNOWN_OPT);
+			     builtin_sparse_checkout_check_rules_usage, 0);
 
 	if (check_rules_opts.rules_file && check_rules_opts.cone_mode < 0)
 		check_rules_opts.cone_mode = 1;

base-commit: 055bb6e9969085777b7fab83e3fee0017654f134
-- 
gitgitgadget

^ permalink raw reply related

* Re: [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add
From: Elijah Newren @ 2023-12-23 22:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Josh Steadmon, git
In-Reply-To: <20231223100229.GA2016274@coredump.intra.peff.net>

Hi,

On Sat, Dec 23, 2023 at 2:02 AM Jeff King <peff@peff.net> wrote:
>
> On Thu, Dec 21, 2023 at 02:04:56PM -0800, Junio C Hamano wrote:
>
> > Jeff King <peff@peff.net> writes:
> >
> > > Right, that is the "gotcha" I mentioned in my other email. Though that
> > > is the way it has behaved historically, my argument is that users are
> > > unreasonable to expect it to work:
> > >
> > >   1. It is not consistent with the rest of Git commands.
> > >
> > >   2. It is inconsistent with respect to existing options (and is an
> > >      accident waiting to happen when new options are added).
> > >
> > > So I'd consider it a bug-fix.
> >
> > So the counter-proposal here is just to drop KEEP_UNKNOWN_OPT and
> > deliberately break them^W^W^Wrealign their expectations?
>
> Yes. :) But keep in mind we are un-breaking other people, like those who
> typo:
>
>   git sparse-checkout --sikp-checks
>
> and don't see an error (instead, we make a garbage entry in the sparse
> checkout file).

I think you mean
     git sparse-checkout set --sikp-checks
or
     git sparse-checkout add --sikp-checks
(i.e. with either 'set' or 'add' in there)

Neither of these are currently an error, but I agree both definitely
ought to be.  (Without the 'set' or 'add', you currently do get an
error, as we'd expect.)

> > I do not have much stake in sparse-checkout, so I am fine with that
> > direction.  But I suspect other folks on the list would have users
> > of their own who would rather loudly complain to them if we broke
> > them ;-)
>
> Likewise, I have never used sparse-checkout myself, and I don't care
> _that_ much. My interest is mostly in having various Git commands behave
> consistently. This whole discussion started because the centralized
> --end-of-options fix interacted badly with this unusual behavior.

Well, I care quite a bit about sparse-checkout, and I would rather see
Peff's proposed fix, even if some users might have to tweak their
commands a little.

Of course, I'm not the only sparse-checkout user representative, but
Randall commented elsewhere in this thread that he used
sparse-checkout quite a bit, and he feels that "without the --,
--sikp-checks should result in an error."  In other words, he is
basically agreeing that taking Peff's fix seems more important than
strict backward compatibility here.  (His wording may not sound like
that at first glance, probably because of the 'set'/'add' omission in
the example command, but I think what he said actually amounts to
agreeing with this change.)

Finally, git-sparse-checkout(1) does say "THIS COMMAND IS
EXPERIMENTAL.  ITS BEHAVIOR...WILL LIKELY CHANGE".  I apologize for
being pulled from my intent to work on [*], which I think would allow
us to finally drop this warning (I'll still get back to it, one way or
another...), but I think this is a good case where we should take
advantage of the existing warning and simply fix the command.

Anyway, just my $0.02,
Elijah

[*] https://lore.kernel.org/git/pull.1367.v4.git.1667714666810.gitgitgadget@gmail.com/

^ permalink raw reply

* Re: rebase invoking pre-commit
From: Elijah Newren @ 2023-12-23 21:37 UTC (permalink / raw)
  To: Sean Allred; +Cc: Git List
In-Reply-To: <m0sf3vi86g.fsf@epic96565.epic.com>

On Thu, Dec 21, 2023 at 12:59 PM Sean Allred <allred.sean@gmail.com> wrote:
>
> Is there a current reason why pre-commit shouldn't be invoked during
> rebase, or is this just waiting for a reviewable patch?
>
> This was brought up before at [1] in 2015, but that thread so old at
> this point that it seemed prudent to double-check before investing time
> in a developing and testing a patch.
>
> [1]: https://lore.kernel.org/git/1m55i3m.1fum4zo1fpnhncM%25lists@haller-berlin.de/

I'm very opinionated here.  I'm just one person, so definitely take
this with a grain of salt, but in my view...

Personally, I think implementing any per-commit hook in rebase by
default is a mistake.  It enforces a
must-be-in-a-worktree-and-the-worktree-must-be-updated-with-every-replayed-commit
mindset, which I find highly problematic[2], even if that's "what we
always used to do".  Because of that, I would prefer to see this at
most be a command line flag.  However, we've already got a command
line flag that while not identical, is essentially equivalent: "--exec
$MY_SCRIPT" (it's not the same because it's a post-commit check, but
you get notification of any problematic commits, and an immediate stop
of the rebase for you to fix up the problematic commit; fixing up the
commit shouldn't be problematic since you are, after all, already
rebasing).

I see Phillip already responded and suggested not running the
pre-commit hook with every commit, but only upon the first commit
after a "git rebase --continue".  That seems far more reasonable to me
than running on every commit...though even that worries me in regards
to what assumptions that entails about what is present in the working
tree.  (For example, what about folks with large repositories, so
large that a branch switch or full checkout is extremely costly, and
which could benefit from resolving conflicts in a separate
sparse-checkout worktree, potentially much more sparse than their main
checkout?  And what if people like that really fast rebase resolution
(namely, done in a separate very sparse checkout which also has the
advantage of not polluting your current working tree) so much that
they use it on smaller repositories as well?  Can I not even
experiment with this idea because of the historical
per-commit-at-least-as-full-as-main-worktree-checkout assumptions
we've baked into rebase?)

While at it, I should also mention that I'm not a fan of the broken
pre-rebase hook; its design ties us to doing a single branch at a
time.  Maybe that hook is not quite as bad, though, since we already
broke that hook and no one seemed to care (in particular, when
--update-refs was implemented).  But if no one seems to care about
broken hooks, I think the proper answer is to either get rid of the
hook or fix it.

Anyway, as I mentioned, I'm quite opinionated here.  To the point that
I deemed git-rebase in its current form to possibly be unfixable
(after first putting a lot of work into improving it over the past
years) and eventually introduced a new "git-replay" command which I
hope to make into a competent replacement for it.  Given that I do
have another command to do my experiments, others on the list may
think it's fine to send rebase further down this route, but I was
hoping to avoid further drift so that there might be some way of
re-implementing rebase on top of my "git-replay" ideas/design.

Just my $0.02,
Elijah

[2] https://lore.kernel.org/git/20231124111044.3426007-1-christian.couder@gmail.com/

^ permalink raw reply

* [PATCH v2 12/12] treewide: remove unnecessary includes in source files
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:15 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/archive.c                   | 1 -
 builtin/commit-graph.c              | 1 -
 builtin/fsck.c                      | 1 -
 builtin/fsmonitor--daemon.c         | 2 --
 builtin/grep.c                      | 1 -
 builtin/mktag.c                     | 1 -
 builtin/rev-list.c                  | 1 -
 builtin/send-pack.c                 | 1 -
 commit-graph.c                      | 1 -
 compat/simple-ipc/ipc-shared.c      | 3 ---
 compat/simple-ipc/ipc-unix-socket.c | 1 -
 fsmonitor-ipc.c                     | 1 -
 http.c                              | 1 -
 line-log.c                          | 1 -
 merge-ort.c                         | 1 -
 notes-utils.c                       | 1 -
 ref-filter.c                        | 1 -
 remote-curl.c                       | 1 -
 repo-settings.c                     | 1 -
 t/helper/test-repository.c          | 1 -
 trace2/tr2_ctr.c                    | 1 -
 trace2/tr2_tmr.c                    | 1 -
 22 files changed, 25 deletions(-)

diff --git a/builtin/archive.c b/builtin/archive.c
index 90761fdfee0..15ee1ec7bb7 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -9,7 +9,6 @@
 #include "parse-options.h"
 #include "pkt-line.h"
 #include "repository.h"
-#include "sideband.h"
 
 static void create_output_file(const char *output_file)
 {
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 81a28c6fcdd..666ad574a46 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -4,7 +4,6 @@
 #include "environment.h"
 #include "gettext.h"
 #include "hex.h"
-#include "lockfile.h"
 #include "parse-options.h"
 #include "repository.h"
 #include "commit-graph.h"
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 9317b7b841d..a7cf94f67ed 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -10,7 +10,6 @@
 #include "refs.h"
 #include "pack.h"
 #include "cache-tree.h"
-#include "tree-walk.h"
 #include "fsck.h"
 #include "parse-options.h"
 #include "progress.h"
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 9f80b9eaff5..1593713f4cb 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -7,7 +7,6 @@
 #include "parse-options.h"
 #include "fsmonitor-ll.h"
 #include "fsmonitor-ipc.h"
-#include "fsmonitor-path-utils.h"
 #include "fsmonitor-settings.h"
 #include "compat/fsmonitor/fsm-health.h"
 #include "compat/fsmonitor/fsm-listen.h"
@@ -15,7 +14,6 @@
 #include "repository.h"
 #include "simple-ipc.h"
 #include "khash.h"
-#include "pkt-line.h"
 #include "run-command.h"
 #include "trace.h"
 #include "trace2.h"
diff --git a/builtin/grep.c b/builtin/grep.c
index f076cc705b4..c8e33f97755 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -14,7 +14,6 @@
 #include "parse-options.h"
 #include "string-list.h"
 #include "run-command.h"
-#include "userdiff.h"
 #include "grep.h"
 #include "quote.h"
 #include "dir.h"
diff --git a/builtin/mktag.c b/builtin/mktag.c
index d8e0b5afc07..4767f1a97e6 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -3,7 +3,6 @@
 #include "hex.h"
 #include "parse-options.h"
 #include "strbuf.h"
-#include "tag.h"
 #include "replace-object.h"
 #include "object-file.h"
 #include "object-store-ll.h"
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 460ba7cbaa7..b3f47838580 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -12,7 +12,6 @@
 #include "object-name.h"
 #include "object-file.h"
 #include "object-store-ll.h"
-#include "pack.h"
 #include "pack-bitmap.h"
 #include "log-tree.h"
 #include "graph.h"
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 395f2e490d4..0b839f583a0 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -2,7 +2,6 @@
 #include "config.h"
 #include "hex.h"
 #include "pkt-line.h"
-#include "sideband.h"
 #include "run-command.h"
 #include "remote.h"
 #include "connect.h"
diff --git a/commit-graph.c b/commit-graph.c
index e7212400da3..15980cf9492 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -4,7 +4,6 @@
 #include "gettext.h"
 #include "hex.h"
 #include "lockfile.h"
-#include "pack.h"
 #include "packfile.h"
 #include "commit.h"
 #include "object.h"
diff --git a/compat/simple-ipc/ipc-shared.c b/compat/simple-ipc/ipc-shared.c
index e5e1dda8ccd..cb176d966f2 100644
--- a/compat/simple-ipc/ipc-shared.c
+++ b/compat/simple-ipc/ipc-shared.c
@@ -1,8 +1,5 @@
 #include "git-compat-util.h"
 #include "simple-ipc.h"
-#include "strbuf.h"
-#include "pkt-line.h"
-#include "thread-utils.h"
 
 #ifndef SUPPORTS_SIMPLE_IPC
 /*
diff --git a/compat/simple-ipc/ipc-unix-socket.c b/compat/simple-ipc/ipc-unix-socket.c
index b2f4f22ce44..9b3f2cdf8c9 100644
--- a/compat/simple-ipc/ipc-unix-socket.c
+++ b/compat/simple-ipc/ipc-unix-socket.c
@@ -2,7 +2,6 @@
 #include "gettext.h"
 #include "simple-ipc.h"
 #include "strbuf.h"
-#include "pkt-line.h"
 #include "thread-utils.h"
 #include "trace2.h"
 #include "unix-socket.h"
diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c
index 153918cf768..45471b5b741 100644
--- a/fsmonitor-ipc.c
+++ b/fsmonitor-ipc.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "fsmonitor-ll.h"
 #include "gettext.h"
 #include "simple-ipc.h"
 #include "fsmonitor-ipc.h"
diff --git a/http.c b/http.c
index a64005ceb80..3565c4ec611 100644
--- a/http.c
+++ b/http.c
@@ -4,7 +4,6 @@
 #include "http.h"
 #include "config.h"
 #include "pack.h"
-#include "sideband.h"
 #include "run-command.h"
 #include "url.h"
 #include "urlmatch.h"
diff --git a/line-log.c b/line-log.c
index c276ccec549..8ff6ccb7724 100644
--- a/line-log.c
+++ b/line-log.c
@@ -12,7 +12,6 @@
 #include "xdiff-interface.h"
 #include "strbuf.h"
 #include "log-tree.h"
-#include "userdiff.h"
 #include "line-log.h"
 #include "setup.h"
 #include "strvec.h"
diff --git a/merge-ort.c b/merge-ort.c
index 2a0be468505..77ba7f3020c 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -41,7 +41,6 @@
 #include "revision.h"
 #include "sparse-index.h"
 #include "strmap.h"
-#include "submodule.h"
 #include "trace2.h"
 #include "tree.h"
 #include "unpack-trees.h"
diff --git a/notes-utils.c b/notes-utils.c
index 97c031c26ec..08e5dbc6073 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -5,7 +5,6 @@
 #include "gettext.h"
 #include "refs.h"
 #include "notes-utils.h"
-#include "repository.h"
 #include "strbuf.h"
 
 void create_notes_commit(struct repository *r,
diff --git a/ref-filter.c b/ref-filter.c
index 96959a3762c..01b90e325c2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -29,7 +29,6 @@
 #include "commit-reach.h"
 #include "worktree.h"
 #include "hashmap.h"
-#include "strvec.h"
 
 static struct ref_msg {
 	const char *gone;
diff --git a/remote-curl.c b/remote-curl.c
index 55eefa70f97..7f81bf3fafc 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -11,7 +11,6 @@
 #include "run-command.h"
 #include "pkt-line.h"
 #include "string-list.h"
-#include "sideband.h"
 #include "strvec.h"
 #include "credential.h"
 #include "oid-array.h"
diff --git a/repo-settings.c b/repo-settings.c
index 525f69c0c77..30cd4787627 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -2,7 +2,6 @@
 #include "config.h"
 #include "repository.h"
 #include "midx.h"
-#include "compat/fsmonitor/fsm-listen.h"
 
 static void repo_cfg_bool(struct repository *r, const char *key, int *dest,
 			  int def)
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index c925655c648..0c7c5aa4dd7 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -3,7 +3,6 @@
 #include "commit.h"
 #include "environment.h"
 #include "hex.h"
-#include "object-store-ll.h"
 #include "object.h"
 #include "repository.h"
 #include "setup.h"
diff --git a/trace2/tr2_ctr.c b/trace2/tr2_ctr.c
index 87cf9034fba..d3a33715c14 100644
--- a/trace2/tr2_ctr.c
+++ b/trace2/tr2_ctr.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "thread-utils.h"
 #include "trace2/tr2_tgt.h"
 #include "trace2/tr2_tls.h"
 #include "trace2/tr2_ctr.h"
diff --git a/trace2/tr2_tmr.c b/trace2/tr2_tmr.c
index 31d0e4d1bd1..51f564b07a4 100644
--- a/trace2/tr2_tmr.c
+++ b/trace2/tr2_tmr.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "thread-utils.h"
 #include "trace2/tr2_tgt.h"
 #include "trace2/tr2_tls.h"
 #include "trace2/tr2_tmr.h"
-- 
gitgitgadget

^ permalink raw reply related

* [PATCH v2 11/12] treewide: add direct includes currently only pulled in transitively
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 builtin/commit-graph.c      | 1 +
 builtin/for-each-ref.c      | 1 +
 builtin/fsmonitor--daemon.c | 1 +
 commit-graph.c              | 1 +
 4 files changed, 4 insertions(+)

diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index c5684342ecf..81a28c6fcdd 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -11,6 +11,7 @@
 #include "object-store-ll.h"
 #include "progress.h"
 #include "replace-object.h"
+#include "strbuf.h"
 #include "tag.h"
 #include "trace2.h"
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6235d72f9d3..b5bc700d13c 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,4 +1,5 @@
 #include "builtin.h"
+#include "commit.h"
 #include "config.h"
 #include "gettext.h"
 #include "object.h"
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 7260604534f..9f80b9eaff5 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -12,6 +12,7 @@
 #include "compat/fsmonitor/fsm-health.h"
 #include "compat/fsmonitor/fsm-listen.h"
 #include "fsmonitor--daemon.h"
+#include "repository.h"
 #include "simple-ipc.h"
 #include "khash.h"
 #include "pkt-line.h"
diff --git a/commit-graph.c b/commit-graph.c
index 5bfee53e87b..e7212400da3 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "config.h"
+#include "csum-file.h"
 #include "gettext.h"
 #include "hex.h"
 #include "lockfile.h"
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 10/12] trace2/tr2_tls.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 trace2/tr2_tgt_normal.c | 1 +
 trace2/tr2_tls.c        | 1 +
 trace2/tr2_tls.h        | 1 -
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index 38d5ebddf65..baef48aa698 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -2,6 +2,7 @@
 #include "config.h"
 #include "repository.h"
 #include "run-command.h"
+#include "strbuf.h"
 #include "quote.h"
 #include "version.h"
 #include "trace2/tr2_dst.h"
diff --git a/trace2/tr2_tls.c b/trace2/tr2_tls.c
index 601c9e5036f..4f75392952b 100644
--- a/trace2/tr2_tls.c
+++ b/trace2/tr2_tls.c
@@ -1,4 +1,5 @@
 #include "git-compat-util.h"
+#include "strbuf.h"
 #include "thread-utils.h"
 #include "trace.h"
 #include "trace2/tr2_tls.h"
diff --git a/trace2/tr2_tls.h b/trace2/tr2_tls.h
index f9049805d4d..3dfe6557fc4 100644
--- a/trace2/tr2_tls.h
+++ b/trace2/tr2_tls.h
@@ -1,7 +1,6 @@
 #ifndef TR2_TLS_H
 #define TR2_TLS_H
 
-#include "strbuf.h"
 #include "trace2/tr2_ctr.h"
 #include "trace2/tr2_tmr.h"
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 09/12] submodule-config.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 submodule-config.h        | 1 -
 t/helper/test-submodule.c | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/submodule-config.h b/submodule-config.h
index e8164cca3e4..958f320ac6c 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -3,7 +3,6 @@
 
 #include "config.h"
 #include "submodule.h"
-#include "strbuf.h"
 #include "tree-walk.h"
 
 /**
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index 356e0a26c5a..50c154d0370 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -4,6 +4,7 @@
 #include "remote.h"
 #include "repository.h"
 #include "setup.h"
+#include "strbuf.h"
 #include "submodule-config.h"
 #include "submodule.h"
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 08/12] pkt-line.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 pkt-line.c               | 1 +
 pkt-line.h               | 1 -
 t/helper/test-pkt-line.c | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkt-line.c b/pkt-line.c
index af83a19f4df..236dd3a3ee1 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -4,6 +4,7 @@
 #include "gettext.h"
 #include "hex.h"
 #include "run-command.h"
+#include "sideband.h"
 #include "trace.h"
 #include "write-or-die.h"
 
diff --git a/pkt-line.h b/pkt-line.h
index 954eec87197..aedef56286f 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -2,7 +2,6 @@
 #define PKTLINE_H
 
 #include "strbuf.h"
-#include "sideband.h"
 
 /*
  * Write a packetized stream, where each line is preceded by
diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index f4d134a1452..77e99c37df0 100644
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -1,6 +1,7 @@
 #include "git-compat-util.h"
 #include "test-tool.h"
 #include "pkt-line.h"
+#include "sideband.h"
 #include "write-or-die.h"
 
 static void pack_line(const char *line)
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 07/12] line-log.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 line-log.c | 1 +
 line-log.h | 2 --
 log-tree.c | 1 +
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/line-log.c b/line-log.c
index a878cb7810a..c276ccec549 100644
--- a/line-log.c
+++ b/line-log.c
@@ -1,4 +1,5 @@
 #include "git-compat-util.h"
+#include "diffcore.h"
 #include "line-range.h"
 #include "hex.h"
 #include "tag.h"
diff --git a/line-log.h b/line-log.h
index 4291da8d792..e9dadbc1a58 100644
--- a/line-log.h
+++ b/line-log.h
@@ -1,8 +1,6 @@
 #ifndef LINE_LOG_H
 #define LINE_LOG_H
 
-#include "diffcore.h"
-
 struct rev_info;
 struct commit;
 struct string_list;
diff --git a/log-tree.c b/log-tree.c
index 504da6b519e..337b9334cdb 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -2,6 +2,7 @@
 #include "commit-reach.h"
 #include "config.h"
 #include "diff.h"
+#include "diffcore.h"
 #include "environment.h"
 #include "hex.h"
 #include "object-name.h"
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 06/12] http.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 http-fetch.c  | 1 +
 http-push.c   | 1 +
 http.h        | 1 -
 remote-curl.c | 1 +
 4 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/http-fetch.c b/http-fetch.c
index 93695a440ad..bec94988bbe 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -6,6 +6,7 @@
 #include "walker.h"
 #include "setup.h"
 #include "strvec.h"
+#include "url.h"
 #include "urlmatch.h"
 #include "trace2.h"
 
diff --git a/http-push.c b/http-push.c
index 329513270c8..b4d0b2a6aa3 100644
--- a/http-push.c
+++ b/http-push.c
@@ -15,6 +15,7 @@
 #include "strvec.h"
 #include "tree.h"
 #include "tree-walk.h"
+#include "url.h"
 #include "packfile.h"
 #include "object-store-ll.h"
 #include "commit-reach.h"
diff --git a/http.h b/http.h
index 3a409bccd4e..3af19a8bf53 100644
--- a/http.h
+++ b/http.h
@@ -10,7 +10,6 @@ struct packed_git;
 
 #include "strbuf.h"
 #include "remote.h"
-#include "url.h"
 
 #define DEFAULT_MAX_REQUESTS 5
 
diff --git a/remote-curl.c b/remote-curl.c
index 204feebabe4..55eefa70f97 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -21,6 +21,7 @@
 #include "quote.h"
 #include "trace2.h"
 #include "transport.h"
+#include "url.h"
 #include "write-or-die.h"
 
 static struct remote *remote;
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 05/12] fsmonitor--daemon.h: remove unnecessary includes
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 builtin/fsmonitor--daemon.c          | 2 ++
 compat/fsmonitor/fsm-health-win32.c  | 1 +
 compat/fsmonitor/fsm-listen-darwin.c | 1 +
 compat/fsmonitor/fsm-listen-win32.c  | 1 +
 fsmonitor--daemon.h                  | 4 +---
 5 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index 5d01db5c029..7260604534f 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "abspath.h"
 #include "config.h"
+#include "dir.h"
 #include "environment.h"
 #include "gettext.h"
 #include "parse-options.h"
@@ -14,6 +15,7 @@
 #include "simple-ipc.h"
 #include "khash.h"
 #include "pkt-line.h"
+#include "run-command.h"
 #include "trace.h"
 #include "trace2.h"
 
diff --git a/compat/fsmonitor/fsm-health-win32.c b/compat/fsmonitor/fsm-health-win32.c
index 2d4e245beb1..2aa8c219ace 100644
--- a/compat/fsmonitor/fsm-health-win32.c
+++ b/compat/fsmonitor/fsm-health-win32.c
@@ -4,6 +4,7 @@
 #include "fsm-health.h"
 #include "fsmonitor--daemon.h"
 #include "gettext.h"
+#include "simple-ipc.h"
 
 /*
  * Every minute wake up and test our health.
diff --git a/compat/fsmonitor/fsm-listen-darwin.c b/compat/fsmonitor/fsm-listen-darwin.c
index 11b56d3ef12..2fc67442eb5 100644
--- a/compat/fsmonitor/fsm-listen-darwin.c
+++ b/compat/fsmonitor/fsm-listen-darwin.c
@@ -29,6 +29,7 @@
 #include "fsmonitor--daemon.h"
 #include "fsmonitor-path-utils.h"
 #include "gettext.h"
+#include "simple-ipc.h"
 #include "string-list.h"
 #include "trace.h"
 
diff --git a/compat/fsmonitor/fsm-listen-win32.c b/compat/fsmonitor/fsm-listen-win32.c
index 90a24122844..5a21dade7b8 100644
--- a/compat/fsmonitor/fsm-listen-win32.c
+++ b/compat/fsmonitor/fsm-listen-win32.c
@@ -4,6 +4,7 @@
 #include "fsm-listen.h"
 #include "fsmonitor--daemon.h"
 #include "gettext.h"
+#include "simple-ipc.h"
 #include "trace2.h"
 
 /*
diff --git a/fsmonitor--daemon.h b/fsmonitor--daemon.h
index 673f80d2aad..5cbbec8d940 100644
--- a/fsmonitor--daemon.h
+++ b/fsmonitor--daemon.h
@@ -3,9 +3,7 @@
 
 #ifdef HAVE_FSMONITOR_DAEMON_BACKEND
 
-#include "dir.h"
-#include "run-command.h"
-#include "simple-ipc.h"
+#include "hashmap.h"
 #include "thread-utils.h"
 #include "fsmonitor-path-utils.h"
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 04/12] blame.h: remove unnecessary includes
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 blame.c | 2 ++
 blame.h | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/blame.c b/blame.c
index 141756975bf..1a16d4eb6a5 100644
--- a/blame.c
+++ b/blame.c
@@ -3,6 +3,7 @@
 #include "object-store-ll.h"
 #include "cache-tree.h"
 #include "mergesort.h"
+#include "commit.h"
 #include "convert.h"
 #include "diff.h"
 #include "diffcore.h"
@@ -10,6 +11,7 @@
 #include "hex.h"
 #include "path.h"
 #include "read-cache.h"
+#include "revision.h"
 #include "setup.h"
 #include "tag.h"
 #include "trace2.h"
diff --git a/blame.h b/blame.h
index 31ddc85f19e..5b4e47d44c6 100644
--- a/blame.h
+++ b/blame.h
@@ -1,12 +1,9 @@
 #ifndef BLAME_H
 #define BLAME_H
 
-#include "commit.h"
 #include "oidset.h"
 #include "xdiff-interface.h"
-#include "revision.h"
 #include "prio-queue.h"
-#include "diff.h"
 
 #define PICKAXE_BLAME_MOVE		01
 #define PICKAXE_BLAME_COPY		02
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 02/12] treewide: remove unnecessary includes in source files
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

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>
---
 add-patch.c                   | 1 -
 apply.c                       | 1 -
 archive.c                     | 1 -
 bisect.c                      | 1 -
 blob.c                        | 1 -
 bloom.c                       | 1 -
 builtin/add.c                 | 3 ---
 builtin/am.c                  | 4 ----
 builtin/apply.c               | 1 -
 builtin/bisect.c              | 1 -
 builtin/blame.c               | 1 -
 builtin/branch.c              | 3 ---
 builtin/cat-file.c            | 1 -
 builtin/checkout-index.c      | 1 -
 builtin/checkout.c            | 3 ---
 builtin/clone.c               | 1 -
 builtin/commit-graph.c        | 1 -
 builtin/commit-tree.c         | 3 ---
 builtin/commit.c              | 8 --------
 builtin/credential-cache.c    | 2 --
 builtin/describe.c            | 2 --
 builtin/diff-files.c          | 1 -
 builtin/diff-index.c          | 2 --
 builtin/diff-tree.c           | 1 -
 builtin/diff.c                | 2 --
 builtin/difftool.c            | 1 -
 builtin/fast-export.c         | 1 -
 builtin/fetch.c               | 2 --
 builtin/for-each-ref.c        | 2 --
 builtin/fsck.c                | 2 --
 builtin/get-tar-commit-id.c   | 1 -
 builtin/grep.c                | 3 ---
 builtin/hash-object.c         | 1 -
 builtin/hook.c                | 1 -
 builtin/index-pack.c          | 2 --
 builtin/init-db.c             | 1 -
 builtin/log.c                 | 2 --
 builtin/ls-files.c            | 4 ----
 builtin/ls-remote.c           | 1 -
 builtin/ls-tree.c             | 2 --
 builtin/mailinfo.c            | 1 -
 builtin/merge-base.c          | 3 ---
 builtin/merge-recursive.c     | 3 ---
 builtin/merge-tree.c          | 1 -
 builtin/merge.c               | 4 ----
 builtin/mv.c                  | 1 -
 builtin/notes.c               | 2 --
 builtin/pack-objects.c        | 3 ---
 builtin/pull.c                | 5 -----
 builtin/push.c                | 1 -
 builtin/range-diff.c          | 1 -
 builtin/read-tree.c           | 2 --
 builtin/rebase.c              | 4 ----
 builtin/receive-pack.c        | 1 -
 builtin/repack.c              | 1 -
 builtin/rerere.c              | 1 -
 builtin/reset.c               | 3 ---
 builtin/rev-list.c            | 1 -
 builtin/revert.c              | 2 --
 builtin/rm.c                  | 1 -
 builtin/send-pack.c           | 4 ----
 builtin/show-ref.c            | 1 -
 builtin/sparse-checkout.c     | 4 ----
 builtin/stash.c               | 1 -
 builtin/submodule--helper.c   | 1 -
 builtin/tag.c                 | 1 -
 builtin/unpack-objects.c      | 4 ----
 builtin/update-ref.c          | 1 -
 builtin/verify-commit.c       | 2 --
 builtin/verify-tag.c          | 1 -
 bulk-checkin.c                | 1 -
 bundle-uri.c                  | 1 -
 cache-tree.c                  | 1 -
 combine-diff.c                | 1 -
 commit-graph.c                | 1 -
 commit-reach.c                | 1 -
 commit.c                      | 2 --
 config.c                      | 3 ---
 delta-islands.c               | 5 -----
 diff-lib.c                    | 1 -
 diff-no-index.c               | 3 ---
 diff.c                        | 2 --
 diffcore-break.c              | 1 -
 diffcore-delta.c              | 1 -
 dir.c                         | 1 -
 entry.c                       | 1 -
 exec-cmd.c                    | 1 -
 fetch-pack.c                  | 2 --
 fsck.c                        | 1 -
 gettext.c                     | 2 --
 gpg-interface.c               | 1 -
 grep.c                        | 1 -
 http-fetch.c                  | 1 -
 http-push.c                   | 2 --
 http-walker.c                 | 1 -
 http.c                        | 1 -
 imap-send.c                   | 2 --
 line-log.c                    | 2 --
 line-range.c                  | 1 -
 list-objects-filter-options.c | 5 -----
 list-objects-filter.c         | 5 -----
 ls-refs.c                     | 1 -
 merge-blobs.c                 | 2 --
 merge-ort.c                   | 2 --
 merge-recursive.c             | 5 -----
 merge.c                       | 3 ---
 negotiator/noop.c             | 1 -
 notes.c                       | 2 --
 object-file.c                 | 8 --------
 object-name.c                 | 2 --
 pack-bitmap-write.c           | 3 ---
 pack-check.c                  | 1 -
 pack-write.c                  | 1 -
 packfile.c                    | 1 -
 parse-options.c               | 2 --
 patch-ids.c                   | 1 -
 protocol-caps.c               | 1 -
 reachable.c                   | 1 -
 read-cache.c                  | 2 --
 ref-filter.c                  | 2 --
 reflog.c                      | 1 -
 refs/files-backend.c          | 2 --
 refs/packed-backend.c         | 1 -
 refs/ref-cache.c              | 1 -
 reftable/dump.c               | 2 --
 reftable/generic.c            | 1 -
 reftable/merged.c             | 1 -
 reftable/merged_test.c        | 1 -
 reftable/reader.c             | 1 -
 reftable/readwrite_test.c     | 1 -
 reftable/refname_test.c       | 1 -
 reftable/stack_test.c         | 1 -
 reftable/test_framework.c     | 1 -
 reftable/tree_test.c          | 2 --
 remote-curl.c                 | 1 -
 remote.c                      | 1 -
 rerere.c                      | 2 --
 reset.c                       | 1 -
 revision.c                    | 2 --
 run-command.c                 | 2 --
 send-pack.c                   | 2 --
 sequencer.c                   | 3 ---
 setup.c                       | 1 -
 shallow.c                     | 1 -
 shell.c                       | 1 -
 submodule.c                   | 3 ---
 t/helper/test-bundle-uri.c    | 2 --
 t/helper/test-reach.c         | 2 --
 t/helper/test-repository.c    | 1 -
 t/helper/test-simple-ipc.c    | 1 -
 tmp-objdir.c                  | 1 -
 trace2.c                      | 3 ---
 transport-helper.c            | 2 --
 transport.c                   | 3 ---
 tree.c                        | 3 ---
 upload-pack.c                 | 6 ------
 wrapper.c                     | 1 -
 xdiff-interface.c             | 2 --
 158 files changed, 293 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index bfe19876cd5..79eda168ebb 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -12,7 +12,6 @@
 #include "strvec.h"
 #include "pathspec.h"
 #include "color.h"
-#include "diff.h"
 #include "compat/terminal.h"
 #include "prompt.h"
 
diff --git a/apply.c b/apply.c
index 3d69fec836d..7608e3301ca 100644
--- a/apply.c
+++ b/apply.c
@@ -12,7 +12,6 @@
 #include "base85.h"
 #include "config.h"
 #include "object-store-ll.h"
-#include "blob.h"
 #include "delta.h"
 #include "diff.h"
 #include "dir.h"
diff --git a/archive.c b/archive.c
index ca11db185b1..4562a69a0cc 100644
--- a/archive.c
+++ b/archive.c
@@ -17,7 +17,6 @@
 #include "archive.h"
 #include "parse-options.h"
 #include "unpack-trees.h"
-#include "dir.h"
 #include "quote.h"
 
 static char const * const archive_usage[] = {
diff --git a/bisect.c b/bisect.c
index 1be8e0a2711..8487f8cd1bd 100644
--- a/bisect.c
+++ b/bisect.c
@@ -9,7 +9,6 @@
 #include "refs.h"
 #include "list-objects.h"
 #include "quote.h"
-#include "hash-lookup.h"
 #include "run-command.h"
 #include "log-tree.h"
 #include "bisect.h"
diff --git a/blob.c b/blob.c
index 888e28a5594..3fb2922b1ae 100644
--- a/blob.c
+++ b/blob.c
@@ -1,6 +1,5 @@
 #include "git-compat-util.h"
 #include "blob.h"
-#include "repository.h"
 #include "alloc.h"
 
 const char *blob_type = "blob";
diff --git a/bloom.c b/bloom.c
index 1474aa19fa5..e529f7605ca 100644
--- a/bloom.c
+++ b/bloom.c
@@ -2,7 +2,6 @@
 #include "bloom.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "revision.h"
 #include "hashmap.h"
 #include "commit-graph.h"
 #include "commit.h"
diff --git a/builtin/add.c b/builtin/add.c
index 5126d2ede3d..2151c45fbf0 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -12,14 +12,11 @@
 #include "dir.h"
 #include "gettext.h"
 #include "pathspec.h"
-#include "exec-cmd.h"
-#include "cache-tree.h"
 #include "run-command.h"
 #include "parse-options.h"
 #include "path.h"
 #include "preload-index.h"
 #include "diff.h"
-#include "diffcore.h"
 #include "read-cache.h"
 #include "repository.h"
 #include "revision.h"
diff --git a/builtin/am.c b/builtin/am.c
index 9f084d58bc7..d1990d7edcb 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -10,7 +10,6 @@
 #include "config.h"
 #include "editor.h"
 #include "environment.h"
-#include "exec-cmd.h"
 #include "gettext.h"
 #include "hex.h"
 #include "parse-options.h"
@@ -24,7 +23,6 @@
 #include "refs.h"
 #include "commit.h"
 #include "diff.h"
-#include "diffcore.h"
 #include "unpack-trees.h"
 #include "branch.h"
 #include "object-name.h"
@@ -35,11 +33,9 @@
 #include "log-tree.h"
 #include "notes-utils.h"
 #include "rerere.h"
-#include "prompt.h"
 #include "mailinfo.h"
 #include "apply.h"
 #include "string-list.h"
-#include "packfile.h"
 #include "pager.h"
 #include "path.h"
 #include "repository.h"
diff --git a/builtin/apply.c b/builtin/apply.c
index c18b7ea5d3d..861a01910ca 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -1,6 +1,5 @@
 #include "builtin.h"
 #include "gettext.h"
-#include "parse-options.h"
 #include "repository.h"
 #include "apply.h"
 
diff --git a/builtin/bisect.c b/builtin/bisect.c
index 35938b05fd1..7d5faedfabf 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -7,7 +7,6 @@
 #include "parse-options.h"
 #include "bisect.h"
 #include "refs.h"
-#include "dir.h"
 #include "strvec.h"
 #include "run-command.h"
 #include "oid-array.h"
diff --git a/builtin/blame.c b/builtin/blame.c
index 9c987d65675..e9ea190baf6 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -25,7 +25,6 @@
 #include "userdiff.h"
 #include "line-range.h"
 #include "line-log.h"
-#include "dir.h"
 #include "progress.h"
 #include "object-name.h"
 #include "object-store-ll.h"
diff --git a/builtin/branch.c b/builtin/branch.c
index e7ee9bd0f15..6e30d5eac53 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -17,13 +17,10 @@
 #include "remote.h"
 #include "parse-options.h"
 #include "branch.h"
-#include "diff.h"
 #include "path.h"
-#include "revision.h"
 #include "string-list.h"
 #include "column.h"
 #include "utf8.h"
-#include "wt-status.h"
 #include "ref-filter.h"
 #include "worktree.h"
 #include "help.h"
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index ea8ad601ecc..7d4899348a3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -15,7 +15,6 @@
 #include "parse-options.h"
 #include "userdiff.h"
 #include "streaming.h"
-#include "tree-walk.h"
 #include "oid-array.h"
 #include "packfile.h"
 #include "object-file.h"
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 3b68b476153..2e086a204dc 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -7,7 +7,6 @@
 #define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "config.h"
-#include "dir.h"
 #include "gettext.h"
 #include "lockfile.h"
 #include "quote.h"
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f02434bc155..12a46da89a8 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,7 +1,6 @@
 #define USE_THE_INDEX_VARIABLE
 #include "builtin.h"
 #include "advice.h"
-#include "blob.h"
 #include "branch.h"
 #include "cache-tree.h"
 #include "checkout.h"
@@ -27,10 +26,8 @@
 #include "remote.h"
 #include "resolve-undo.h"
 #include "revision.h"
-#include "run-command.h"
 #include "setup.h"
 #include "submodule.h"
-#include "submodule-config.h"
 #include "symlinks.h"
 #include "trace2.h"
 #include "tree.h"
diff --git a/builtin/clone.c b/builtin/clone.c
index c6357af9498..8e9c055533b 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -19,7 +19,6 @@
 #include "hex.h"
 #include "lockfile.h"
 #include "parse-options.h"
-#include "fetch-pack.h"
 #include "refs.h"
 #include "refspec.h"
 #include "object-file.h"
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 45d035af600..c5684342ecf 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -1,7 +1,6 @@
 #include "builtin.h"
 #include "commit.h"
 #include "config.h"
-#include "dir.h"
 #include "environment.h"
 #include "gettext.h"
 #include "hex.h"
diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 02625e71761..1bb78198392 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -11,9 +11,6 @@
 #include "object-store-ll.h"
 #include "repository.h"
 #include "commit.h"
-#include "tree.h"
-#include "utf8.h"
-#include "gpg-interface.h"
 #include "parse-options.h"
 
 static const char * const commit_tree_usage[] = {
diff --git a/builtin/commit.c b/builtin/commit.c
index 781af2e206c..ca2d18532ad 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -16,17 +16,12 @@
 #include "editor.h"
 #include "environment.h"
 #include "diff.h"
-#include "diffcore.h"
 #include "commit.h"
 #include "gettext.h"
 #include "revision.h"
 #include "wt-status.h"
 #include "run-command.h"
-#include "hook.h"
-#include "refs.h"
-#include "log-tree.h"
 #include "strbuf.h"
-#include "utf8.h"
 #include "object-name.h"
 #include "parse-options.h"
 #include "path.h"
@@ -35,9 +30,6 @@
 #include "string-list.h"
 #include "rerere.h"
 #include "unpack-trees.h"
-#include "quote.h"
-#include "submodule.h"
-#include "gpg-interface.h"
 #include "column.h"
 #include "sequencer.h"
 #include "sparse-index.h"
diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index 43b9d0e5b16..bba96d4ffd6 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c
@@ -7,8 +7,6 @@
 
 #ifndef NO_UNIX_SOCKETS
 
-#include "credential.h"
-#include "string-list.h"
 #include "unix-socket.h"
 #include "run-command.h"
 
diff --git a/builtin/describe.c b/builtin/describe.c
index fb6b0508f32..d6c77a714f4 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -7,9 +7,7 @@
 #include "lockfile.h"
 #include "commit.h"
 #include "tag.h"
-#include "blob.h"
 #include "refs.h"
-#include "exec-cmd.h"
 #include "object-name.h"
 #include "parse-options.h"
 #include "read-cache-ll.h"
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index f38912cd407..018011f29ea 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -11,7 +11,6 @@
 #include "preload-index.h"
 #include "repository.h"
 #include "revision.h"
-#include "submodule.h"
 
 static const char diff_files_usage[] =
 "git diff-files [-q] [-0 | -1 | -2 | -3 | -c | --cc] [<common-diff-options>] [<path>...]"
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 220f341ffa2..3e05260ac0e 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -7,8 +7,6 @@
 #include "repository.h"
 #include "revision.h"
 #include "setup.h"
-#include "sparse-index.h"
-#include "submodule.h"
 
 static const char diff_cache_usage[] =
 "git diff-index [-m] [--cached] [--merge-base] "
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 86be6342861..a8e68ce8ef6 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -6,7 +6,6 @@
 #include "gettext.h"
 #include "hex.h"
 #include "log-tree.h"
-#include "submodule.h"
 #include "read-cache-ll.h"
 #include "repository.h"
 #include "revision.h"
diff --git a/builtin/diff.c b/builtin/diff.c
index 55e7d21755a..6e196e0c7d2 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -10,7 +10,6 @@
 #include "lockfile.h"
 #include "color.h"
 #include "commit.h"
-#include "blob.h"
 #include "gettext.h"
 #include "tag.h"
 #include "diff.h"
@@ -21,7 +20,6 @@
 #include "revision.h"
 #include "log-tree.h"
 #include "setup.h"
-#include "submodule.h"
 #include "oid-array.h"
 #include "tree.h"
 
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 0f5eae9cd41..a3c72b8258e 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -18,7 +18,6 @@
 #include "copy.h"
 #include "run-command.h"
 #include "environment.h"
-#include "exec-cmd.h"
 #include "gettext.h"
 #include "hex.h"
 #include "parse-options.h"
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 70aff515acb..f18f0809f9c 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -25,7 +25,6 @@
 #include "quote.h"
 #include "remote.h"
 #include "blob.h"
-#include "commit-slab.h"
 
 static const char *fast_export_usage[] = {
 	N_("git fast-export [<rev-list-opts>]"),
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fd134ba74d9..72d735589a3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -26,7 +26,6 @@
 #include "connected.h"
 #include "strvec.h"
 #include "utf8.h"
-#include "packfile.h"
 #include "pager.h"
 #include "path.h"
 #include "pkt-line.h"
@@ -38,7 +37,6 @@
 #include "shallow.h"
 #include "trace.h"
 #include "trace2.h"
-#include "worktree.h"
 #include "bundle-uri.h"
 
 #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 350bfa6e811..6235d72f9d3 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -1,13 +1,11 @@
 #include "builtin.h"
 #include "config.h"
 #include "gettext.h"
-#include "refs.h"
 #include "object.h"
 #include "parse-options.h"
 #include "ref-filter.h"
 #include "strbuf.h"
 #include "strvec.h"
-#include "commit-reach.h"
 
 static char const * const for_each_ref_usage[] = {
 	N_("git for-each-ref [<options>] [<pattern>]"),
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 611925905e4..9317b7b841d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -13,10 +13,8 @@
 #include "tree-walk.h"
 #include "fsck.h"
 #include "parse-options.h"
-#include "dir.h"
 #include "progress.h"
 #include "streaming.h"
-#include "decorate.h"
 #include "packfile.h"
 #include "object-file.h"
 #include "object-name.h"
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 20d0dfe9cf1..66a7389f9f4 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -4,7 +4,6 @@
 #include "builtin.h"
 #include "commit.h"
 #include "tar.h"
-#include "quote.h"
 
 static const char builtin_get_tar_commit_id_usage[] =
 "git get-tar-commit-id";
diff --git a/builtin/grep.c b/builtin/grep.c
index fe78d4c98b1..f076cc705b4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -9,9 +9,6 @@
 #include "hex.h"
 #include "repository.h"
 #include "config.h"
-#include "blob.h"
-#include "tree.h"
-#include "commit.h"
 #include "tag.h"
 #include "tree-walk.h"
 #include "parse-options.h"
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index 5ffec99dcea..82ca6d2bfdc 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -14,7 +14,6 @@
 #include "blob.h"
 #include "quote.h"
 #include "parse-options.h"
-#include "exec-cmd.h"
 #include "setup.h"
 #include "strbuf.h"
 #include "write-or-die.h"
diff --git a/builtin/hook.c b/builtin/hook.c
index 09b51a6487c..5234693a94b 100644
--- a/builtin/hook.c
+++ b/builtin/hook.c
@@ -3,7 +3,6 @@
 #include "gettext.h"
 #include "hook.h"
 #include "parse-options.h"
-#include "strbuf.h"
 #include "strvec.h"
 
 #define BUILTIN_HOOK_RUN_USAGE \
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index dda94a9f46d..0841b6940a3 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -8,11 +8,9 @@
 #include "csum-file.h"
 #include "blob.h"
 #include "commit.h"
-#include "tag.h"
 #include "tree.h"
 #include "progress.h"
 #include "fsck.h"
-#include "exec-cmd.h"
 #include "strbuf.h"
 #include "streaming.h"
 #include "thread-utils.h"
diff --git a/builtin/init-db.c b/builtin/init-db.c
index cb727c826f5..b89814a6f87 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -5,7 +5,6 @@
  */
 #include "builtin.h"
 #include "abspath.h"
-#include "config.h"
 #include "environment.h"
 #include "gettext.h"
 #include "object-file.h"
diff --git a/builtin/log.c b/builtin/log.c
index ba775d7b5cf..cec7ce46f1c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -26,7 +26,6 @@
 #include "tag.h"
 #include "reflog-walk.h"
 #include "patch-ids.h"
-#include "run-command.h"
 #include "shortlog.h"
 #include "remote.h"
 #include "string-list.h"
@@ -36,7 +35,6 @@
 #include "streaming.h"
 #include "version.h"
 #include "mailmap.h"
-#include "gpg-interface.h"
 #include "progress.h"
 #include "commit-slab.h"
 #include "repository.h"
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index a0229c32778..92f94e65bf0 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,19 +14,15 @@
 #include "gettext.h"
 #include "object-name.h"
 #include "strbuf.h"
-#include "tree.h"
-#include "cache-tree.h"
 #include "parse-options.h"
 #include "resolve-undo.h"
 #include "string-list.h"
 #include "path.h"
 #include "pathspec.h"
 #include "read-cache.h"
-#include "run-command.h"
 #include "setup.h"
 #include "sparse-index.h"
 #include "submodule.h"
-#include "submodule-config.h"
 #include "object-store.h"
 #include "hex.h"
 
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index fc765754305..2975ea4082f 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -5,7 +5,6 @@
 #include "pkt-line.h"
 #include "ref-filter.h"
 #include "remote.h"
-#include "refs.h"
 #include "parse-options.h"
 #include "wildmatch.h"
 
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index 209d2dc0d59..e4a891337c3 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -9,9 +9,7 @@
 #include "hex.h"
 #include "object-name.h"
 #include "object-store-ll.h"
-#include "blob.h"
 #include "tree.h"
-#include "commit.h"
 #include "path.h"
 #include "quote.h"
 #include "parse-options.h"
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 53b55dd71c0..53a22645da5 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -6,7 +6,6 @@
 #include "abspath.h"
 #include "environment.h"
 #include "gettext.h"
-#include "utf8.h"
 #include "strbuf.h"
 #include "mailinfo.h"
 #include "parse-options.h"
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e68b7fe45d7..d26e8fbf6f7 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -3,9 +3,6 @@
 #include "commit.h"
 #include "gettext.h"
 #include "hex.h"
-#include "refs.h"
-#include "diff.h"
-#include "revision.h"
 #include "object-name.h"
 #include "parse-options.h"
 #include "repository.h"
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index 3366699657c..c2ce044a201 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -1,13 +1,10 @@
 #include "builtin.h"
 #include "advice.h"
-#include "commit.h"
 #include "gettext.h"
 #include "hash.h"
-#include "tag.h"
 #include "merge-recursive.h"
 #include "object-name.h"
 #include "repository.h"
-#include "xdiff-interface.h"
 
 static const char builtin_merge_recursive_usage[] =
 	"git %s <base>... -- <head> <remote> ...";
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index a35e0452d66..f3c46691010 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -13,7 +13,6 @@
 #include "parse-options.h"
 #include "repository.h"
 #include "blob.h"
-#include "exec-cmd.h"
 #include "merge-blobs.h"
 #include "quote.h"
 #include "tree.h"
diff --git a/builtin/merge.c b/builtin/merge.c
index d748d46e135..5b788546637 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -31,8 +31,6 @@
 #include "unpack-trees.h"
 #include "cache-tree.h"
 #include "dir.h"
-#include "utf8.h"
-#include "log-tree.h"
 #include "color.h"
 #include "rerere.h"
 #include "help.h"
@@ -42,10 +40,8 @@
 #include "resolve-undo.h"
 #include "remote.h"
 #include "fmt-merge-msg.h"
-#include "gpg-interface.h"
 #include "sequencer.h"
 #include "string-list.h"
-#include "packfile.h"
 #include "tag.h"
 #include "alias.h"
 #include "branch.h"
diff --git a/builtin/mv.c b/builtin/mv.c
index c596515ad05..22e64fc2900 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -15,7 +15,6 @@
 #include "pathspec.h"
 #include "lockfile.h"
 #include "dir.h"
-#include "cache-tree.h"
 #include "string-list.h"
 #include "parse-options.h"
 #include "read-cache-ll.h"
diff --git a/builtin/notes.c b/builtin/notes.c
index 9f38863dd50..e65cae0bcf7 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -9,7 +9,6 @@
 
 #include "builtin.h"
 #include "config.h"
-#include "alloc.h"
 #include "editor.h"
 #include "environment.h"
 #include "gettext.h"
@@ -19,7 +18,6 @@
 #include "object-store-ll.h"
 #include "path.h"
 #include "repository.h"
-#include "blob.h"
 #include "pretty.h"
 #include "refs.h"
 #include "exec-cmd.h"
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 89a8b5a9768..ab1c9de9815 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -6,10 +6,8 @@
 #include "config.h"
 #include "attr.h"
 #include "object.h"
-#include "blob.h"
 #include "commit.h"
 #include "tag.h"
-#include "tree.h"
 #include "delta.h"
 #include "pack.h"
 #include "pack-revindex.h"
@@ -18,7 +16,6 @@
 #include "diff.h"
 #include "revision.h"
 #include "list-objects.h"
-#include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
 #include "pack-objects.h"
 #include "progress.h"
diff --git a/builtin/pull.c b/builtin/pull.c
index be2b2c9ebc9..73a68b75b06 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -14,7 +14,6 @@
 #include "merge.h"
 #include "object-name.h"
 #include "parse-options.h"
-#include "exec-cmd.h"
 #include "run-command.h"
 #include "oid-array.h"
 #include "remote.h"
@@ -24,15 +23,11 @@
 #include "rebase.h"
 #include "refs.h"
 #include "refspec.h"
-#include "revision.h"
 #include "submodule.h"
 #include "submodule-config.h"
-#include "tempfile.h"
-#include "lockfile.h"
 #include "wt-status.h"
 #include "commit-reach.h"
 #include "sequencer.h"
-#include "packfile.h"
 
 /**
  * Parses the value of --rebase. If value is a false value, returns
diff --git a/builtin/push.c b/builtin/push.c
index 2e708383c24..23e841d5b54 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -7,7 +7,6 @@
 #include "config.h"
 #include "environment.h"
 #include "gettext.h"
-#include "refs.h"
 #include "refspec.h"
 #include "run-command.h"
 #include "remote.h"
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e455a4795cc..f02cbac087d 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -5,7 +5,6 @@
 #include "range-diff.h"
 #include "config.h"
 #include "repository.h"
-#include "revision.h"
 
 static const char * const builtin_range_diff_usage[] = {
 N_("git range-diff [<options>] <old-base>..<old-tip> <new-base>..<new-tip>"),
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 8196ca9dd85..20e7db19737 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -16,14 +16,12 @@
 #include "tree-walk.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
-#include "dir.h"
 #include "parse-options.h"
 #include "repository.h"
 #include "resolve-undo.h"
 #include "setup.h"
 #include "sparse-index.h"
 #include "submodule.h"
-#include "submodule-config.h"
 
 static int nr_trees;
 static int read_empty;
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 043c65dccd9..d338b4053da 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -11,14 +11,10 @@
 #include "gettext.h"
 #include "hex.h"
 #include "run-command.h"
-#include "exec-cmd.h"
 #include "strvec.h"
 #include "dir.h"
-#include "packfile.h"
 #include "refs.h"
-#include "quote.h"
 #include "config.h"
-#include "cache-tree.h"
 #include "unpack-trees.h"
 #include "lockfile.h"
 #include "object-file.h"
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 8c4f0cb90a9..401d93b675f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -22,7 +22,6 @@
 #include "connected.h"
 #include "strvec.h"
 #include "version.h"
-#include "tag.h"
 #include "gpg-interface.h"
 #include "sigchain.h"
 #include "fsck.h"
diff --git a/builtin/repack.c b/builtin/repack.c
index edaee4dbec7..fc10570d06b 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -8,7 +8,6 @@
 #include "path.h"
 #include "run-command.h"
 #include "server-info.h"
-#include "sigchain.h"
 #include "strbuf.h"
 #include "string-list.h"
 #include "strvec.h"
diff --git a/builtin/rerere.c b/builtin/rerere.c
index 07a9d37275c..b2efc6f640e 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -1,6 +1,5 @@
 #include "builtin.h"
 #include "config.h"
-#include "dir.h"
 #include "gettext.h"
 #include "parse-options.h"
 #include "repository.h"
diff --git a/builtin/reset.c b/builtin/reset.c
index 4b018d20e3b..8390bfe4c48 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -16,10 +16,8 @@
 #include "hash.h"
 #include "hex.h"
 #include "lockfile.h"
-#include "tag.h"
 #include "object.h"
 #include "pretty.h"
-#include "run-command.h"
 #include "refs.h"
 #include "diff.h"
 #include "diffcore.h"
@@ -33,7 +31,6 @@
 #include "setup.h"
 #include "sparse-index.h"
 #include "submodule.h"
-#include "submodule-config.h"
 #include "trace.h"
 #include "trace2.h"
 #include "dir.h"
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 181353dcf51..460ba7cbaa7 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -7,7 +7,6 @@
 #include "hex.h"
 #include "revision.h"
 #include "list-objects.h"
-#include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
 #include "object.h"
 #include "object-name.h"
diff --git a/builtin/revert.c b/builtin/revert.c
index e6f9a1ad267..89821bab957 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "config.h"
 #include "builtin.h"
 #include "parse-options.h"
 #include "diff.h"
@@ -7,7 +6,6 @@
 #include "repository.h"
 #include "revision.h"
 #include "rerere.h"
-#include "dir.h"
 #include "sequencer.h"
 #include "branch.h"
 
diff --git a/builtin/rm.c b/builtin/rm.c
index dff819ae509..fd130cea2d2 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -9,7 +9,6 @@
 #include "config.h"
 #include "lockfile.h"
 #include "dir.h"
-#include "cache-tree.h"
 #include "gettext.h"
 #include "hash.h"
 #include "tree-walk.h"
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index cd6d9e41129..395f2e490d4 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -1,8 +1,6 @@
 #include "builtin.h"
 #include "config.h"
-#include "commit.h"
 #include "hex.h"
-#include "refs.h"
 #include "pkt-line.h"
 #include "sideband.h"
 #include "run-command.h"
@@ -11,9 +9,7 @@
 #include "send-pack.h"
 #include "quote.h"
 #include "transport.h"
-#include "version.h"
 #include "oid-array.h"
-#include "gpg-interface.h"
 #include "gettext.h"
 #include "protocol.h"
 #include "parse-options.h"
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 7aac525a878..f102f6f6131 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -6,7 +6,6 @@
 #include "object-name.h"
 #include "object-store-ll.h"
 #include "object.h"
-#include "tag.h"
 #include "string-list.h"
 #include "parse-options.h"
 
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 5c8ffb1f759..80227f3df17 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -8,14 +8,10 @@
 #include "parse-options.h"
 #include "pathspec.h"
 #include "repository.h"
-#include "run-command.h"
 #include "strbuf.h"
 #include "string-list.h"
-#include "cache-tree.h"
 #include "lockfile.h"
-#include "resolve-undo.h"
 #include "unpack-trees.h"
-#include "wt-status.h"
 #include "quote.h"
 #include "setup.h"
 #include "sparse-index.h"
diff --git a/builtin/stash.c b/builtin/stash.c
index 4a6771c9f4c..b2813c614cb 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -26,7 +26,6 @@
 #include "sparse-index.h"
 #include "log-tree.h"
 #include "diffcore.h"
-#include "exec-cmd.h"
 #include "reflog.h"
 #include "add-interactive.h"
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index cce46450abe..fda50f2af1e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -22,7 +22,6 @@
 #include "remote.h"
 #include "refs.h"
 #include "refspec.h"
-#include "connect.h"
 #include "revision.h"
 #include "diffcore.h"
 #include "diff.h"
diff --git a/builtin/tag.c b/builtin/tag.c
index 3918eacbb57..358b3086161 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -18,7 +18,6 @@
 #include "object-store-ll.h"
 #include "path.h"
 #include "tag.h"
-#include "run-command.h"
 #include "parse-options.h"
 #include "diff.h"
 #include "revision.h"
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index fef74234488..e0a701f2b38 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -10,12 +10,8 @@
 #include "delta.h"
 #include "pack.h"
 #include "blob.h"
-#include "commit.h"
 #include "replace-object.h"
 #include "strbuf.h"
-#include "tag.h"
-#include "tree.h"
-#include "tree-walk.h"
 #include "progress.h"
 #include "decorate.h"
 #include "fsck.h"
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index c0c4e65e6fb..61338a01ecf 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -7,7 +7,6 @@
 #include "parse-options.h"
 #include "quote.h"
 #include "repository.h"
-#include "strvec.h"
 
 static const char * const git_update_ref_usage[] = {
 	N_("git update-ref [<options>] -d <refname> [<old-val>]"),
diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c
index 9680b587013..0d2b9aea2ae 100644
--- a/builtin/verify-commit.c
+++ b/builtin/verify-commit.c
@@ -9,10 +9,8 @@
 #include "config.h"
 #include "gettext.h"
 #include "object-name.h"
-#include "object-store-ll.h"
 #include "repository.h"
 #include "commit.h"
-#include "run-command.h"
 #include "parse-options.h"
 #include "gpg-interface.h"
 
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index d8753270ebe..c731e2f87b4 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -9,7 +9,6 @@
 #include "config.h"
 #include "gettext.h"
 #include "tag.h"
-#include "run-command.h"
 #include "object-name.h"
 #include "parse-options.h"
 #include "gpg-interface.h"
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 6ce62999e58..eb46b886379 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -11,7 +11,6 @@
 #include "csum-file.h"
 #include "pack.h"
 #include "strbuf.h"
-#include "string-list.h"
 #include "tmp-objdir.h"
 #include "packfile.h"
 #include "object-file.h"
diff --git a/bundle-uri.c b/bundle-uri.c
index 8492fffd2f7..ca32050a78f 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -4,7 +4,6 @@
 #include "copy.h"
 #include "environment.h"
 #include "gettext.h"
-#include "object-store-ll.h"
 #include "refs.h"
 #include "run-command.h"
 #include "hashmap.h"
diff --git a/cache-tree.c b/cache-tree.c
index 641427ed410..64678fe1993 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -11,7 +11,6 @@
 #include "read-cache-ll.h"
 #include "replace-object.h"
 #include "promisor-remote.h"
-#include "sparse-index.h"
 #include "trace.h"
 #include "trace2.h"
 
diff --git a/combine-diff.c b/combine-diff.c
index f90f4424829..db94581f724 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -2,7 +2,6 @@
 #include "object-store-ll.h"
 #include "commit.h"
 #include "convert.h"
-#include "blob.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "environment.h"
diff --git a/commit-graph.c b/commit-graph.c
index ee66098e077..5bfee53e87b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -8,7 +8,6 @@
 #include "commit.h"
 #include "object.h"
 #include "refs.h"
-#include "revision.h"
 #include "hash-lookup.h"
 #include "commit-graph.h"
 #include "object-file.h"
diff --git a/commit-reach.c b/commit-reach.c
index a868a575ea1..ecc913fc99b 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -4,7 +4,6 @@
 #include "decorate.h"
 #include "hex.h"
 #include "prio-queue.h"
-#include "tree.h"
 #include "ref-filter.h"
 #include "revision.h"
 #include "tag.h"
diff --git a/commit.c b/commit.c
index 8405d7c3fce..f6342d7deb5 100644
--- a/commit.c
+++ b/commit.c
@@ -8,7 +8,6 @@
 #include "repository.h"
 #include "object-name.h"
 #include "object-store-ll.h"
-#include "pkt-line.h"
 #include "utf8.h"
 #include "diff.h"
 #include "revision.h"
@@ -23,7 +22,6 @@
 #include "advice.h"
 #include "refs.h"
 #include "commit-reach.h"
-#include "run-command.h"
 #include "setup.h"
 #include "shallow.h"
 #include "tree.h"
diff --git a/config.c b/config.c
index b330c7adb4a..cdc39cf3693 100644
--- a/config.c
+++ b/config.c
@@ -30,15 +30,12 @@
 #include "pager.h"
 #include "path.h"
 #include "utf8.h"
-#include "dir.h"
 #include "color.h"
-#include "replace-object.h"
 #include "refs.h"
 #include "setup.h"
 #include "strvec.h"
 #include "trace2.h"
 #include "wildmatch.h"
-#include "worktree.h"
 #include "ws.h"
 #include "write-or-die.h"
 
diff --git a/delta-islands.c b/delta-islands.c
index 5de5759f3f1..ee2318d45a1 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -1,18 +1,13 @@
 #include "git-compat-util.h"
-#include "attr.h"
 #include "object.h"
-#include "blob.h"
 #include "commit.h"
 #include "gettext.h"
 #include "hex.h"
 #include "tag.h"
 #include "tree.h"
-#include "delta.h"
 #include "pack.h"
 #include "tree-walk.h"
 #include "diff.h"
-#include "revision.h"
-#include "list-objects.h"
 #include "progress.h"
 #include "refs.h"
 #include "khash.h"
diff --git a/diff-lib.c b/diff-lib.c
index 0e9ec4f68af..8fde93d7cac 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -2,7 +2,6 @@
  * Copyright (C) 2005 Junio C Hamano
  */
 #include "git-compat-util.h"
-#include "quote.h"
 #include "commit.h"
 #include "diff.h"
 #include "diffcore.h"
diff --git a/diff-no-index.c b/diff-no-index.c
index e7041b89e38..3a8965672c5 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -8,13 +8,10 @@
 #include "abspath.h"
 #include "color.h"
 #include "commit.h"
-#include "blob.h"
-#include "tag.h"
 #include "diff.h"
 #include "diffcore.h"
 #include "gettext.h"
 #include "revision.h"
-#include "log-tree.h"
 #include "parse-options.h"
 #include "string-list.h"
 #include "dir.h"
diff --git a/diff.c b/diff.c
index 2c602df10a3..587341805c2 100644
--- a/diff.c
+++ b/diff.c
@@ -16,12 +16,10 @@
 #include "hex.h"
 #include "xdiff-interface.h"
 #include "color.h"
-#include "attr.h"
 #include "run-command.h"
 #include "utf8.h"
 #include "object-store-ll.h"
 #include "userdiff.h"
-#include "submodule-config.h"
 #include "submodule.h"
 #include "hashmap.h"
 #include "mem-pool.h"
diff --git a/diffcore-break.c b/diffcore-break.c
index f57ece2757d..49ba38aa7c0 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -2,7 +2,6 @@
  * Copyright (C) 2005 Junio C Hamano
  */
 #include "git-compat-util.h"
-#include "diff.h"
 #include "diffcore.h"
 #include "hash.h"
 #include "object.h"
diff --git a/diffcore-delta.c b/diffcore-delta.c
index c30b56e983b..4927ab8fb0c 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "diff.h"
 #include "diffcore.h"
 
 /*
diff --git a/dir.c b/dir.c
index 16fdb03f2a5..3e75c126edf 100644
--- a/dir.c
+++ b/dir.c
@@ -16,7 +16,6 @@
 #include "object-file.h"
 #include "object-store-ll.h"
 #include "path.h"
-#include "attr.h"
 #include "refs.h"
 #include "wildmatch.h"
 #include "pathspec.h"
diff --git a/entry.c b/entry.c
index 076e97eb89c..f918a3a78e8 100644
--- a/entry.c
+++ b/entry.c
@@ -1,5 +1,4 @@
 #include "git-compat-util.h"
-#include "blob.h"
 #include "object-store-ll.h"
 #include "dir.h"
 #include "environment.h"
diff --git a/exec-cmd.c b/exec-cmd.c
index 1d597e84ea7..909777f61f4 100644
--- a/exec-cmd.c
+++ b/exec-cmd.c
@@ -4,7 +4,6 @@
 #include "exec-cmd.h"
 #include "gettext.h"
 #include "path.h"
-#include "quote.h"
 #include "run-command.h"
 #include "strvec.h"
 #include "trace.h"
diff --git a/fetch-pack.c b/fetch-pack.c
index 26999e3b659..b0373bd87c4 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -10,7 +10,6 @@
 #include "pkt-line.h"
 #include "commit.h"
 #include "tag.h"
-#include "exec-cmd.h"
 #include "pack.h"
 #include "sideband.h"
 #include "fetch-pack.h"
@@ -18,7 +17,6 @@
 #include "run-command.h"
 #include "connect.h"
 #include "trace2.h"
-#include "transport.h"
 #include "version.h"
 #include "oid-array.h"
 #include "oidset.h"
diff --git a/fsck.c b/fsck.c
index 6a0bbc50877..9fce4c9628f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -16,7 +16,6 @@
 #include "refs.h"
 #include "url.h"
 #include "utf8.h"
-#include "decorate.h"
 #include "oidset.h"
 #include "packfile.h"
 #include "submodule-config.h"
diff --git a/gettext.c b/gettext.c
index f27e94407b4..57facbc21ec 100644
--- a/gettext.c
+++ b/gettext.c
@@ -7,9 +7,7 @@
 #include "environment.h"
 #include "exec-cmd.h"
 #include "gettext.h"
-#include "strbuf.h"
 #include "utf8.h"
-#include "config.h"
 
 #ifndef NO_GETTEXT
 #	include <libintl.h>
diff --git a/gpg-interface.c b/gpg-interface.c
index 48f43c5a21d..636475f598f 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -12,7 +12,6 @@
 #include "sigchain.h"
 #include "tempfile.h"
 #include "alias.h"
-#include "environment.h"
 
 static int git_gpg_config(const char *, const char *,
 			  const struct config_context *, void *);
diff --git a/grep.c b/grep.c
index fc2d0c837a3..5f23d1a50ca 100644
--- a/grep.c
+++ b/grep.c
@@ -9,7 +9,6 @@
 #include "xdiff-interface.h"
 #include "diff.h"
 #include "diffcore.h"
-#include "commit.h"
 #include "quote.h"
 #include "help.h"
 
diff --git a/http-fetch.c b/http-fetch.c
index fffda592670..93695a440ad 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1,6 +1,5 @@
 #include "git-compat-util.h"
 #include "config.h"
-#include "exec-cmd.h"
 #include "gettext.h"
 #include "hex.h"
 #include "http.h"
diff --git a/http-push.c b/http-push.c
index a704f490fdb..329513270c8 100644
--- a/http-push.c
+++ b/http-push.c
@@ -6,10 +6,8 @@
 #include "tag.h"
 #include "blob.h"
 #include "http.h"
-#include "refs.h"
 #include "diff.h"
 #include "revision.h"
-#include "exec-cmd.h"
 #include "remote.h"
 #include "list-objects.h"
 #include "setup.h"
diff --git a/http-walker.c b/http-walker.c
index 78d99f7c4b0..b395ef13279 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -1,6 +1,5 @@
 #include "git-compat-util.h"
 #include "repository.h"
-#include "commit.h"
 #include "hex.h"
 #include "walker.h"
 #include "http.h"
diff --git a/http.c b/http.c
index 8f71bf00d89..a64005ceb80 100644
--- a/http.c
+++ b/http.c
@@ -15,7 +15,6 @@
 #include "trace.h"
 #include "transport.h"
 #include "packfile.h"
-#include "protocol.h"
 #include "string-list.h"
 #include "object-file.h"
 #include "object-store-ll.h"
diff --git a/imap-send.c b/imap-send.c
index 996651e4f80..904cb23cf57 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -24,12 +24,10 @@
 #include "git-compat-util.h"
 #include "config.h"
 #include "credential.h"
-#include "exec-cmd.h"
 #include "gettext.h"
 #include "run-command.h"
 #include "parse-options.h"
 #include "setup.h"
-#include "strbuf.h"
 #if defined(NO_OPENSSL) && !defined(HAVE_OPENSSL_CSPRNG)
 typedef void *SSL;
 #endif
diff --git a/line-log.c b/line-log.c
index 24a1ecb6779..a878cb7810a 100644
--- a/line-log.c
+++ b/line-log.c
@@ -2,7 +2,6 @@
 #include "line-range.h"
 #include "hex.h"
 #include "tag.h"
-#include "blob.h"
 #include "tree.h"
 #include "diff.h"
 #include "commit.h"
@@ -12,7 +11,6 @@
 #include "xdiff-interface.h"
 #include "strbuf.h"
 #include "log-tree.h"
-#include "graph.h"
 #include "userdiff.h"
 #include "line-log.h"
 #include "setup.h"
diff --git a/line-range.c b/line-range.c
index 47bf0d6f1a2..60f0e5ada81 100644
--- a/line-range.c
+++ b/line-range.c
@@ -1,7 +1,6 @@
 #include "git-compat-util.h"
 #include "line-range.h"
 #include "xdiff-interface.h"
-#include "strbuf.h"
 #include "userdiff.h"
 
 /*
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 8a08b7af49c..c5f363ca6f7 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -1,11 +1,6 @@
 #include "git-compat-util.h"
-#include "commit.h"
 #include "config.h"
 #include "gettext.h"
-#include "revision.h"
-#include "strvec.h"
-#include "list-objects.h"
-#include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
 #include "promisor-remote.h"
 #include "trace.h"
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 9327ccd5057..da287cc8e0d 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -2,14 +2,9 @@
 #include "dir.h"
 #include "gettext.h"
 #include "hex.h"
-#include "tag.h"
 #include "commit.h"
-#include "tree.h"
-#include "blob.h"
 #include "diff.h"
-#include "tree-walk.h"
 #include "revision.h"
-#include "list-objects.h"
 #include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
 #include "oidmap.h"
diff --git a/ls-refs.c b/ls-refs.c
index 0e49b932c30..819cbefee3d 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -5,7 +5,6 @@
 #include "hex.h"
 #include "repository.h"
 #include "refs.h"
-#include "remote.h"
 #include "strvec.h"
 #include "ls-refs.h"
 #include "pkt-line.h"
diff --git a/merge-blobs.c b/merge-blobs.c
index 9293cbf75c8..2f659fd0143 100644
--- a/merge-blobs.c
+++ b/merge-blobs.c
@@ -1,6 +1,4 @@
 #include "git-compat-util.h"
-#include "run-command.h"
-#include "xdiff-interface.h"
 #include "merge-ll.h"
 #include "blob.h"
 #include "merge-blobs.h"
diff --git a/merge-ort.c b/merge-ort.c
index 6491070d965..2a0be468505 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -19,7 +19,6 @@
 
 #include "alloc.h"
 #include "attr.h"
-#include "blob.h"
 #include "cache-tree.h"
 #include "commit.h"
 #include "commit-reach.h"
@@ -42,7 +41,6 @@
 #include "revision.h"
 #include "sparse-index.h"
 #include "strmap.h"
-#include "submodule-config.h"
 #include "submodule.h"
 #include "trace2.h"
 #include "tree.h"
diff --git a/merge-recursive.c b/merge-recursive.c
index e3beb0801b1..a0c3e7a2d91 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -6,10 +6,7 @@
 #include "git-compat-util.h"
 #include "merge-recursive.h"
 
-#include "advice.h"
 #include "alloc.h"
-#include "attr.h"
-#include "blob.h"
 #include "cache-tree.h"
 #include "commit.h"
 #include "commit-reach.h"
@@ -32,8 +29,6 @@
 #include "revision.h"
 #include "sparse-index.h"
 #include "string-list.h"
-#include "submodule-config.h"
-#include "submodule.h"
 #include "symlinks.h"
 #include "tag.h"
 #include "tree-walk.h"
diff --git a/merge.c b/merge.c
index b60925459c2..ca89b312d17 100644
--- a/merge.c
+++ b/merge.c
@@ -1,6 +1,4 @@
 #include "git-compat-util.h"
-#include "diff.h"
-#include "diffcore.h"
 #include "gettext.h"
 #include "hash.h"
 #include "hex.h"
@@ -13,7 +11,6 @@
 #include "tree.h"
 #include "tree-walk.h"
 #include "unpack-trees.h"
-#include "dir.h"
 
 static const char *merge_argument(struct commit *commit)
 {
diff --git a/negotiator/noop.c b/negotiator/noop.c
index de39028ab7f..65e3c200084 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -1,6 +1,5 @@
 #include "git-compat-util.h"
 #include "noop.h"
-#include "../commit.h"
 #include "../fetch-negotiator.h"
 
 static void known_common(struct fetch_negotiator *n UNUSED,
diff --git a/notes.c b/notes.c
index 1ef2a331ce9..fed1eda80cd 100644
--- a/notes.c
+++ b/notes.c
@@ -5,8 +5,6 @@
 #include "notes.h"
 #include "object-name.h"
 #include "object-store-ll.h"
-#include "blob.h"
-#include "tree.h"
 #include "utf8.h"
 #include "strbuf.h"
 #include "tree-walk.h"
diff --git a/object-file.c b/object-file.c
index 7c7afe57936..619f039ebc7 100644
--- a/object-file.c
+++ b/object-file.c
@@ -15,24 +15,16 @@
 #include "hex.h"
 #include "string-list.h"
 #include "lockfile.h"
-#include "delta.h"
 #include "pack.h"
-#include "blob.h"
 #include "commit.h"
 #include "run-command.h"
-#include "tag.h"
-#include "tree.h"
-#include "tree-walk.h"
 #include "refs.h"
-#include "pack-revindex.h"
-#include "hash-lookup.h"
 #include "bulk-checkin.h"
 #include "repository.h"
 #include "replace-object.h"
 #include "streaming.h"
 #include "dir.h"
 #include "list.h"
-#include "mergesort.h"
 #include "quote.h"
 #include "packfile.h"
 #include "object-file.h"
diff --git a/object-name.c b/object-name.c
index 0bfa29dbbfe..3a2ef5d6800 100644
--- a/object-name.c
+++ b/object-name.c
@@ -8,7 +8,6 @@
 #include "tag.h"
 #include "commit.h"
 #include "tree.h"
-#include "blob.h"
 #include "tree-walk.h"
 #include "refs.h"
 #include "remote.h"
@@ -21,7 +20,6 @@
 #include "read-cache-ll.h"
 #include "repository.h"
 #include "setup.h"
-#include "submodule.h"
 #include "midx.h"
 #include "commit-reach.h"
 #include "date.h"
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index f4ecdf8b0e3..be4733e3bdc 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -4,12 +4,9 @@
 #include "hex.h"
 #include "object-store-ll.h"
 #include "commit.h"
-#include "tag.h"
 #include "diff.h"
 #include "revision.h"
-#include "list-objects.h"
 #include "progress.h"
-#include "pack-revindex.h"
 #include "pack.h"
 #include "pack-bitmap.h"
 #include "hash-lookup.h"
diff --git a/pack-check.c b/pack-check.c
index 977f619618e..25104d5b14c 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -3,7 +3,6 @@
 #include "hex.h"
 #include "repository.h"
 #include "pack.h"
-#include "pack-revindex.h"
 #include "progress.h"
 #include "packfile.h"
 #include "object-file.h"
diff --git a/pack-write.c b/pack-write.c
index b19ddf15b28..80ecfa544c5 100644
--- a/pack-write.c
+++ b/pack-write.c
@@ -7,7 +7,6 @@
 #include "remote.h"
 #include "chunk-format.h"
 #include "pack-mtimes.h"
-#include "oidmap.h"
 #include "pack-objects.h"
 #include "pack-revindex.h"
 #include "path.h"
diff --git a/packfile.c b/packfile.c
index 9cc0a2e37a8..84a005674d8 100644
--- a/packfile.c
+++ b/packfile.c
@@ -9,7 +9,6 @@
 #include "mergesort.h"
 #include "packfile.h"
 #include "delta.h"
-#include "streaming.h"
 #include "hash-lookup.h"
 #include "commit.h"
 #include "object.h"
diff --git a/parse-options.c b/parse-options.c
index e0c94b0546b..6054a3ca5ae 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -2,8 +2,6 @@
 #include "parse-options.h"
 #include "abspath.h"
 #include "parse.h"
-#include "commit.h"
-#include "color.h"
 #include "gettext.h"
 #include "strbuf.h"
 #include "string-list.h"
diff --git a/patch-ids.c b/patch-ids.c
index c3e1a0dd216..a5683b462c6 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -2,7 +2,6 @@
 #include "diff.h"
 #include "commit.h"
 #include "hash.h"
-#include "hash-lookup.h"
 #include "hex.h"
 #include "patch-ids.h"
 
diff --git a/protocol-caps.c b/protocol-caps.c
index 808a68c974a..75f4cbb0a70 100644
--- a/protocol-caps.c
+++ b/protocol-caps.c
@@ -3,7 +3,6 @@
 #include "gettext.h"
 #include "hex.h"
 #include "pkt-line.h"
-#include "strvec.h"
 #include "hash-ll.h"
 #include "hex.h"
 #include "object.h"
diff --git a/reachable.c b/reachable.c
index 0ce8f83e56a..f29b06a5d05 100644
--- a/reachable.c
+++ b/reachable.c
@@ -2,7 +2,6 @@
 #include "gettext.h"
 #include "hex.h"
 #include "refs.h"
-#include "tag.h"
 #include "commit.h"
 #include "blob.h"
 #include "diff.h"
diff --git a/read-cache.c b/read-cache.c
index 080bd39713b..08970caff7c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -20,7 +20,6 @@
 #include "oid-array.h"
 #include "tree.h"
 #include "commit.h"
-#include "blob.h"
 #include "environment.h"
 #include "gettext.h"
 #include "mem-pool.h"
@@ -31,7 +30,6 @@
 #include "read-cache.h"
 #include "resolve-undo.h"
 #include "revision.h"
-#include "run-command.h"
 #include "strbuf.h"
 #include "trace2.h"
 #include "varint.h"
diff --git a/ref-filter.c b/ref-filter.c
index e4d3510e28e..96959a3762c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -22,12 +22,10 @@
 #include "ref-filter.h"
 #include "revision.h"
 #include "utf8.h"
-#include "version.h"
 #include "versioncmp.h"
 #include "trailer.h"
 #include "wt-status.h"
 #include "commit-slab.h"
-#include "commit-graph.h"
 #include "commit-reach.h"
 #include "worktree.h"
 #include "hashmap.h"
diff --git a/reflog.c b/reflog.c
index 9ad50e7d93e..0a1bc35e8cd 100644
--- a/reflog.c
+++ b/reflog.c
@@ -6,7 +6,6 @@
 #include "revision.h"
 #include "tree.h"
 #include "tree-walk.h"
-#include "worktree.h"
 
 /* Remember to update object flag allocation in object.h */
 #define INCOMPLETE	(1u<<10)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index db5c0c7a724..922e65e0d9c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1,5 +1,4 @@
 #include "../git-compat-util.h"
-#include "../config.h"
 #include "../copy.h"
 #include "../environment.h"
 #include "../gettext.h"
@@ -19,7 +18,6 @@
 #include "../dir.h"
 #include "../chdir-notify.h"
 #include "../setup.h"
-#include "../worktree.h"
 #include "../wrapper.h"
 #include "../write-or-die.h"
 #include "../revision.h"
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 59c78d7941f..5963e67c14c 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1,5 +1,4 @@
 #include "../git-compat-util.h"
-#include "../alloc.h"
 #include "../config.h"
 #include "../gettext.h"
 #include "../hash.h"
diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 6e3b725245c..a372a00941f 100644
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -1,5 +1,4 @@
 #include "../git-compat-util.h"
-#include "../alloc.h"
 #include "../hash.h"
 #include "../refs.h"
 #include "../repository.h"
diff --git a/reftable/dump.c b/reftable/dump.c
index ce936b4e188..26e0393c7db 100644
--- a/reftable/dump.c
+++ b/reftable/dump.c
@@ -11,14 +11,12 @@ https://developers.google.com/open-source/licenses/bsd
 
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
-#include "reftable-merged.h"
 #include "reftable-record.h"
 #include "reftable-tests.h"
 #include "reftable-writer.h"
 #include "reftable-iterator.h"
 #include "reftable-reader.h"
 #include "reftable-stack.h"
-#include "reftable-generic.h"
 
 #include <stddef.h>
 #include <stdio.h>
diff --git a/reftable/generic.c b/reftable/generic.c
index 57f8032db94..b9f1c7c18a2 100644
--- a/reftable/generic.c
+++ b/reftable/generic.c
@@ -6,7 +6,6 @@ license that can be found in the LICENSE file or at
 https://developers.google.com/open-source/licenses/bsd
 */
 
-#include "basics.h"
 #include "constants.h"
 #include "record.h"
 #include "generic.h"
diff --git a/reftable/merged.c b/reftable/merged.c
index 5ded470c086..9191f3addd1 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -11,7 +11,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "constants.h"
 #include "iter.h"
 #include "pq.h"
-#include "reader.h"
 #include "record.h"
 #include "generic.h"
 #include "reftable-merged.h"
diff --git a/reftable/merged_test.c b/reftable/merged_test.c
index d08c16abefb..0d6e0d4bf57 100644
--- a/reftable/merged_test.c
+++ b/reftable/merged_test.c
@@ -12,7 +12,6 @@ https://developers.google.com/open-source/licenses/bsd
 
 #include "basics.h"
 #include "blocksource.h"
-#include "constants.h"
 #include "reader.h"
 #include "record.h"
 #include "test_framework.h"
diff --git a/reftable/reader.c b/reftable/reader.c
index b4db23ce188..8b7a27781c3 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -16,7 +16,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "record.h"
 #include "reftable-error.h"
 #include "reftable-generic.h"
-#include "tree.h"
 
 uint64_t block_source_size(struct reftable_block_source *source)
 {
diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index 469ab79a5ad..f0d468be394 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -11,7 +11,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "basics.h"
 #include "block.h"
 #include "blocksource.h"
-#include "constants.h"
 #include "reader.h"
 #include "record.h"
 #include "test_framework.h"
diff --git a/reftable/refname_test.c b/reftable/refname_test.c
index 8645cd93bbd..699e1aea412 100644
--- a/reftable/refname_test.c
+++ b/reftable/refname_test.c
@@ -9,7 +9,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "basics.h"
 #include "block.h"
 #include "blocksource.h"
-#include "constants.h"
 #include "reader.h"
 #include "record.h"
 #include "refname.h"
diff --git a/reftable/stack_test.c b/reftable/stack_test.c
index d0b717510fa..d1b2908fa36 100644
--- a/reftable/stack_test.c
+++ b/reftable/stack_test.c
@@ -13,7 +13,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "reftable-reader.h"
 #include "merged.h"
 #include "basics.h"
-#include "constants.h"
 #include "record.h"
 #include "test_framework.h"
 #include "reftable-tests.h"
diff --git a/reftable/test_framework.c b/reftable/test_framework.c
index 84ac972cad0..04044fc1a0f 100644
--- a/reftable/test_framework.c
+++ b/reftable/test_framework.c
@@ -9,7 +9,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "system.h"
 #include "test_framework.h"
 
-#include "basics.h"
 
 void set_test_hash(uint8_t *p, int i)
 {
diff --git a/reftable/tree_test.c b/reftable/tree_test.c
index ac3a045ad4a..6961a657adb 100644
--- a/reftable/tree_test.c
+++ b/reftable/tree_test.c
@@ -9,8 +9,6 @@ https://developers.google.com/open-source/licenses/bsd
 #include "system.h"
 #include "tree.h"
 
-#include "basics.h"
-#include "record.h"
 #include "test_framework.h"
 #include "reftable-tests.h"
 
diff --git a/remote-curl.c b/remote-curl.c
index ef05752ca57..204feebabe4 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -8,7 +8,6 @@
 #include "strbuf.h"
 #include "walker.h"
 #include "http.h"
-#include "exec-cmd.h"
 #include "run-command.h"
 #include "pkt-line.h"
 #include "string-list.h"
diff --git a/remote.c b/remote.c
index abb24822beb..6bef95316af 100644
--- a/remote.c
+++ b/remote.c
@@ -15,7 +15,6 @@
 #include "diff.h"
 #include "revision.h"
 #include "dir.h"
-#include "tag.h"
 #include "setup.h"
 #include "string-list.h"
 #include "strvec.h"
diff --git a/rerere.c b/rerere.c
index 09e19412859..ca7e77ba68c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -12,12 +12,10 @@
 #include "dir.h"
 #include "resolve-undo.h"
 #include "merge-ll.h"
-#include "attr.h"
 #include "path.h"
 #include "pathspec.h"
 #include "object-file.h"
 #include "object-store-ll.h"
-#include "hash-lookup.h"
 #include "strmap.h"
 
 #define RESOLVED 0
diff --git a/reset.c b/reset.c
index 48da0adf851..0f2ff0fe315 100644
--- a/reset.c
+++ b/reset.c
@@ -6,7 +6,6 @@
 #include "object-name.h"
 #include "refs.h"
 #include "reset.h"
-#include "run-command.h"
 #include "tree-walk.h"
 #include "tree.h"
 #include "unpack-trees.h"
diff --git a/revision.c b/revision.c
index 00d5c29bfce..51c056adbe1 100644
--- a/revision.c
+++ b/revision.c
@@ -21,12 +21,10 @@
 #include "reflog-walk.h"
 #include "patch-ids.h"
 #include "decorate.h"
-#include "log-tree.h"
 #include "string-list.h"
 #include "line-log.h"
 #include "mailmap.h"
 #include "commit-slab.h"
-#include "dir.h"
 #include "cache-tree.h"
 #include "bisect.h"
 #include "packfile.h"
diff --git a/run-command.c b/run-command.c
index a558042c876..0e7435718a5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -14,9 +14,7 @@
 #include "quote.h"
 #include "config.h"
 #include "packfile.h"
-#include "hook.h"
 #include "compat/nonblock.h"
-#include "alloc.h"
 
 void child_process_init(struct child_process *child)
 {
diff --git a/send-pack.c b/send-pack.c
index 89aca9d829e..37f59d4f66b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -4,7 +4,6 @@
 #include "date.h"
 #include "gettext.h"
 #include "hex.h"
-#include "refs.h"
 #include "object-store-ll.h"
 #include "pkt-line.h"
 #include "sideband.h"
@@ -12,7 +11,6 @@
 #include "remote.h"
 #include "connect.h"
 #include "send-pack.h"
-#include "quote.h"
 #include "transport.h"
 #include "version.h"
 #include "oid-array.h"
diff --git a/sequencer.c b/sequencer.c
index d584cac8ed9..fab88e4efcb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -15,10 +15,8 @@
 #include "pager.h"
 #include "commit.h"
 #include "sequencer.h"
-#include "tag.h"
 #include "run-command.h"
 #include "hook.h"
-#include "exec-cmd.h"
 #include "utf8.h"
 #include "cache-tree.h"
 #include "diff.h"
@@ -39,7 +37,6 @@
 #include "notes-utils.h"
 #include "sigchain.h"
 #include "unpack-trees.h"
-#include "worktree.h"
 #include "oidmap.h"
 #include "oidset.h"
 #include "commit-slab.h"
diff --git a/setup.c b/setup.c
index fc592dc6dd5..c5d3efe1964 100644
--- a/setup.c
+++ b/setup.c
@@ -13,7 +13,6 @@
 #include "string-list.h"
 #include "chdir-notify.h"
 #include "path.h"
-#include "promisor-remote.h"
 #include "quote.h"
 #include "trace2.h"
 #include "worktree.h"
diff --git a/shallow.c b/shallow.c
index ac728cdd778..7711798127e 100644
--- a/shallow.c
+++ b/shallow.c
@@ -7,7 +7,6 @@
 #include "commit.h"
 #include "tag.h"
 #include "pkt-line.h"
-#include "remote.h"
 #include "refs.h"
 #include "oid-array.h"
 #include "path.h"
diff --git a/shell.c b/shell.c
index 5c67e7bd97e..2ece8b16e2e 100644
--- a/shell.c
+++ b/shell.c
@@ -4,7 +4,6 @@
 #include "strbuf.h"
 #include "run-command.h"
 #include "alias.h"
-#include "prompt.h"
 
 #define COMMAND_DIR "git-shell-commands"
 #define HELP_COMMAND COMMAND_DIR "/help"
diff --git a/submodule.c b/submodule.c
index e603a19a876..213da79f661 100644
--- a/submodule.c
+++ b/submodule.c
@@ -17,10 +17,8 @@
 #include "string-list.h"
 #include "oid-array.h"
 #include "strvec.h"
-#include "blob.h"
 #include "thread-utils.h"
 #include "path.h"
-#include "quote.h"
 #include "remote.h"
 #include "worktree.h"
 #include "parse-options.h"
@@ -30,7 +28,6 @@
 #include "commit-reach.h"
 #include "read-cache-ll.h"
 #include "setup.h"
-#include "shallow.h"
 #include "trace2.h"
 
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
diff --git a/t/helper/test-bundle-uri.c b/t/helper/test-bundle-uri.c
index 475058592d1..09dc78733c0 100644
--- a/t/helper/test-bundle-uri.c
+++ b/t/helper/test-bundle-uri.c
@@ -5,9 +5,7 @@
 #include "strbuf.h"
 #include "string-list.h"
 #include "transport.h"
-#include "ref-filter.h"
 #include "remote.h"
-#include "refs.h"
 
 enum input_mode {
 	KEY_VALUE_PAIRS,
diff --git a/t/helper/test-reach.c b/t/helper/test-reach.c
index 3e173399a00..1e159a754db 100644
--- a/t/helper/test-reach.c
+++ b/t/helper/test-reach.c
@@ -1,11 +1,9 @@
 #include "test-tool.h"
 #include "commit.h"
 #include "commit-reach.h"
-#include "config.h"
 #include "gettext.h"
 #include "hex.h"
 #include "object-name.h"
-#include "parse-options.h"
 #include "ref-filter.h"
 #include "setup.h"
 #include "string-list.h"
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index 4cd8a952e5c..c925655c648 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -1,7 +1,6 @@
 #include "test-tool.h"
 #include "commit-graph.h"
 #include "commit.h"
-#include "config.h"
 #include "environment.h"
 #include "hex.h"
 #include "object-store-ll.h"
diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c
index 941ae7e3bcf..fb5927775da 100644
--- a/t/helper/test-simple-ipc.c
+++ b/t/helper/test-simple-ipc.c
@@ -4,7 +4,6 @@
 
 #include "test-tool.h"
 #include "gettext.h"
-#include "strbuf.h"
 #include "simple-ipc.h"
 #include "parse-options.h"
 #include "thread-utils.h"
diff --git a/tmp-objdir.c b/tmp-objdir.c
index 5f9074ad1c0..3509258be53 100644
--- a/tmp-objdir.c
+++ b/tmp-objdir.c
@@ -6,7 +6,6 @@
 #include "environment.h"
 #include "object-file.h"
 #include "path.h"
-#include "sigchain.h"
 #include "string-list.h"
 #include "strbuf.h"
 #include "strvec.h"
diff --git a/trace2.c b/trace2.c
index 6dc74dff4c7..d961f0e3a84 100644
--- a/trace2.c
+++ b/trace2.c
@@ -1,12 +1,9 @@
 #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"
diff --git a/transport-helper.c b/transport-helper.c
index 49811ef176d..e34a8f47cfb 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -3,13 +3,11 @@
 #include "quote.h"
 #include "run-command.h"
 #include "commit.h"
-#include "diff.h"
 #include "environment.h"
 #include "gettext.h"
 #include "hex.h"
 #include "object-name.h"
 #include "repository.h"
-#include "revision.h"
 #include "remote.h"
 #include "string-list.h"
 #include "thread-utils.h"
diff --git a/transport.c b/transport.c
index 219af8fd50e..bd7899e9bf5 100644
--- a/transport.c
+++ b/transport.c
@@ -10,9 +10,7 @@
 #include "remote.h"
 #include "connect.h"
 #include "send-pack.h"
-#include "walker.h"
 #include "bundle.h"
-#include "dir.h"
 #include "gettext.h"
 #include "refs.h"
 #include "refspec.h"
@@ -26,7 +24,6 @@
 #include "transport-internal.h"
 #include "protocol.h"
 #include "object-name.h"
-#include "object-store-ll.h"
 #include "color.h"
 #include "bundle-uri.h"
 
diff --git a/tree.c b/tree.c
index 990f9c9854e..508e5fd76fd 100644
--- a/tree.c
+++ b/tree.c
@@ -1,12 +1,9 @@
 #include "git-compat-util.h"
-#include "cache-tree.h"
 #include "hex.h"
 #include "tree.h"
 #include "object-name.h"
 #include "object-store-ll.h"
-#include "blob.h"
 #include "commit.h"
-#include "tag.h"
 #include "alloc.h"
 #include "tree-walk.h"
 #include "repository.h"
diff --git a/upload-pack.c b/upload-pack.c
index ea234ab6a45..2537affa907 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -9,13 +9,10 @@
 #include "repository.h"
 #include "object-store-ll.h"
 #include "oid-array.h"
-#include "tag.h"
 #include "object.h"
 #include "commit.h"
 #include "diff.h"
 #include "revision.h"
-#include "list-objects.h"
-#include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
 #include "run-command.h"
 #include "connect.h"
@@ -24,11 +21,8 @@
 #include "string-list.h"
 #include "strvec.h"
 #include "trace2.h"
-#include "prio-queue.h"
 #include "protocol.h"
-#include "quote.h"
 #include "upload-pack.h"
-#include "serve.h"
 #include "commit-graph.h"
 #include "commit-reach.h"
 #include "shallow.h"
diff --git a/wrapper.c b/wrapper.c
index 7da15a56da6..eeac3741cf1 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -5,7 +5,6 @@
 #include "abspath.h"
 #include "parse.h"
 #include "gettext.h"
-#include "repository.h"
 #include "strbuf.h"
 #include "trace2.h"
 
diff --git a/xdiff-interface.c b/xdiff-interface.c
index adcea109fa9..b39ffb1f718 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -6,8 +6,6 @@
 #include "xdiff-interface.h"
 #include "xdiff/xtypes.h"
 #include "xdiff/xdiffi.h"
-#include "xdiff/xemit.h"
-#include "xdiff/xmacros.h"
 #include "xdiff/xutils.h"
 
 struct xdiff_emit_state {
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 03/12] archive.h: remove unnecessary include
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: 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>
---
 archive-tar.c | 1 +
 archive-zip.c | 1 +
 archive.c     | 1 +
 archive.h     | 1 -
 4 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/archive-tar.c b/archive-tar.c
index 07269968399..f2a0ed77523 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -9,6 +9,7 @@
 #include "tar.h"
 #include "archive.h"
 #include "object-store-ll.h"
+#include "strbuf.h"
 #include "streaming.h"
 #include "run-command.h"
 #include "write-or-die.h"
diff --git a/archive-zip.c b/archive-zip.c
index 7229e3e454f..fd1d3f816d3 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -10,6 +10,7 @@
 #include "streaming.h"
 #include "utf8.h"
 #include "object-store-ll.h"
+#include "strbuf.h"
 #include "userdiff.h"
 #include "write-or-die.h"
 #include "xdiff-interface.h"
diff --git a/archive.c b/archive.c
index 4562a69a0cc..50fd35bd27b 100644
--- a/archive.c
+++ b/archive.c
@@ -5,6 +5,7 @@
 #include "environment.h"
 #include "gettext.h"
 #include "hex.h"
+#include "object-name.h"
 #include "path.h"
 #include "pretty.h"
 #include "setup.h"
diff --git a/archive.h b/archive.h
index 3a4bdfbd078..bbe65ba0f90 100644
--- a/archive.h
+++ b/archive.h
@@ -1,7 +1,6 @@
 #ifndef ARCHIVE_H
 #define ARCHIVE_H
 
-#include "object-name.h"
 #include "pathspec.h"
 #include "string-list.h"
 
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 01/12] treewide: remove unnecessary includes from header files
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.v2.git.1703351700.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

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>
---
 fetch-pack.h       | 1 -
 midx.h             | 1 -
 ref-filter.h       | 1 -
 submodule-config.h | 1 -
 4 files changed, 4 deletions(-)

diff --git a/fetch-pack.h b/fetch-pack.h
index 8c7752fc821..6775d265175 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -2,7 +2,6 @@
 #define FETCH_PACK_H
 
 #include "string-list.h"
-#include "run-command.h"
 #include "protocol.h"
 #include "list-objects-filter-options.h"
 #include "oidset.h"
diff --git a/midx.h b/midx.h
index a5d98919c85..eb57a37519c 100644
--- a/midx.h
+++ b/midx.h
@@ -1,7 +1,6 @@
 #ifndef MIDX_H
 #define MIDX_H
 
-#include "repository.h"
 #include "string-list.h"
 
 struct object_id;
diff --git a/ref-filter.h b/ref-filter.h
index 1524bc463a5..4ecb6ab1c60 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -3,7 +3,6 @@
 
 #include "gettext.h"
 #include "oid-array.h"
-#include "refs.h"
 #include "commit.h"
 #include "string-list.h"
 #include "strvec.h"
diff --git a/submodule-config.h b/submodule-config.h
index 2a37689cc27..e8164cca3e4 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -2,7 +2,6 @@
 #define SUBMODULE_CONFIG_CACHE_H
 
 #include "config.h"
-#include "hashmap.h"
 #include "submodule.h"
 #include "strbuf.h"
 #include "tree-walk.h"
-- 
gitgitgadget


^ permalink raw reply related

* [PATCH v2 00/12] Additional header cleanups (removing unnecessary includes)
From: Elijah Newren via GitGitGadget @ 2023-12-23 17:14 UTC (permalink / raw)
  To: git; +Cc: Elijah Newren, Elijah Newren
In-Reply-To: <pull.1617.git.1701585682.gitgitgadget@gmail.com>

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:

 * Dropped three changes that conflicted with changes in next & seen (in
   attr.c, trace2.c, and test-trace2.c). After topics in next & seen merge
   down, I can submit a future patch that still does the relevant part of
   the cleanups that I'm dropping for now.

Elijah Newren (12):
  treewide: remove unnecessary includes from header files
  treewide: remove unnecessary includes in source files
  archive.h: remove unnecessary include
  blame.h: remove unnecessary includes
  fsmonitor--daemon.h: remove unnecessary includes
  http.h: remove unnecessary include
  line-log.h: remove unnecessary include
  pkt-line.h: remove unnecessary include
  submodule-config.h: remove unnecessary include
  trace2/tr2_tls.h: remove unnecessary include
  treewide: add direct includes currently only pulled in transitively
  treewide: remove unnecessary includes in source files

 add-patch.c                          | 1 -
 apply.c                              | 1 -
 archive-tar.c                        | 1 +
 archive-zip.c                        | 1 +
 archive.c                            | 2 +-
 archive.h                            | 1 -
 bisect.c                             | 1 -
 blame.c                              | 2 ++
 blame.h                              | 3 ---
 blob.c                               | 1 -
 bloom.c                              | 1 -
 builtin/add.c                        | 3 ---
 builtin/am.c                         | 4 ----
 builtin/apply.c                      | 1 -
 builtin/archive.c                    | 1 -
 builtin/bisect.c                     | 1 -
 builtin/blame.c                      | 1 -
 builtin/branch.c                     | 3 ---
 builtin/cat-file.c                   | 1 -
 builtin/checkout-index.c             | 1 -
 builtin/checkout.c                   | 3 ---
 builtin/clone.c                      | 1 -
 builtin/commit-graph.c               | 3 +--
 builtin/commit-tree.c                | 3 ---
 builtin/commit.c                     | 8 --------
 builtin/credential-cache.c           | 2 --
 builtin/describe.c                   | 2 --
 builtin/diff-files.c                 | 1 -
 builtin/diff-index.c                 | 2 --
 builtin/diff-tree.c                  | 1 -
 builtin/diff.c                       | 2 --
 builtin/difftool.c                   | 1 -
 builtin/fast-export.c                | 1 -
 builtin/fetch.c                      | 2 --
 builtin/for-each-ref.c               | 3 +--
 builtin/fsck.c                       | 3 ---
 builtin/fsmonitor--daemon.c          | 5 +++--
 builtin/get-tar-commit-id.c          | 1 -
 builtin/grep.c                       | 4 ----
 builtin/hash-object.c                | 1 -
 builtin/hook.c                       | 1 -
 builtin/index-pack.c                 | 2 --
 builtin/init-db.c                    | 1 -
 builtin/log.c                        | 2 --
 builtin/ls-files.c                   | 4 ----
 builtin/ls-remote.c                  | 1 -
 builtin/ls-tree.c                    | 2 --
 builtin/mailinfo.c                   | 1 -
 builtin/merge-base.c                 | 3 ---
 builtin/merge-recursive.c            | 3 ---
 builtin/merge-tree.c                 | 1 -
 builtin/merge.c                      | 4 ----
 builtin/mktag.c                      | 1 -
 builtin/mv.c                         | 1 -
 builtin/notes.c                      | 2 --
 builtin/pack-objects.c               | 3 ---
 builtin/pull.c                       | 5 -----
 builtin/push.c                       | 1 -
 builtin/range-diff.c                 | 1 -
 builtin/read-tree.c                  | 2 --
 builtin/rebase.c                     | 4 ----
 builtin/receive-pack.c               | 1 -
 builtin/repack.c                     | 1 -
 builtin/rerere.c                     | 1 -
 builtin/reset.c                      | 3 ---
 builtin/rev-list.c                   | 2 --
 builtin/revert.c                     | 2 --
 builtin/rm.c                         | 1 -
 builtin/send-pack.c                  | 5 -----
 builtin/show-ref.c                   | 1 -
 builtin/sparse-checkout.c            | 4 ----
 builtin/stash.c                      | 1 -
 builtin/submodule--helper.c          | 1 -
 builtin/tag.c                        | 1 -
 builtin/unpack-objects.c             | 4 ----
 builtin/update-ref.c                 | 1 -
 builtin/verify-commit.c              | 2 --
 builtin/verify-tag.c                 | 1 -
 bulk-checkin.c                       | 1 -
 bundle-uri.c                         | 1 -
 cache-tree.c                         | 1 -
 combine-diff.c                       | 1 -
 commit-graph.c                       | 3 +--
 commit-reach.c                       | 1 -
 commit.c                             | 2 --
 compat/fsmonitor/fsm-health-win32.c  | 1 +
 compat/fsmonitor/fsm-listen-darwin.c | 1 +
 compat/fsmonitor/fsm-listen-win32.c  | 1 +
 compat/simple-ipc/ipc-shared.c       | 3 ---
 compat/simple-ipc/ipc-unix-socket.c  | 1 -
 config.c                             | 3 ---
 delta-islands.c                      | 5 -----
 diff-lib.c                           | 1 -
 diff-no-index.c                      | 3 ---
 diff.c                               | 2 --
 diffcore-break.c                     | 1 -
 diffcore-delta.c                     | 1 -
 dir.c                                | 1 -
 entry.c                              | 1 -
 exec-cmd.c                           | 1 -
 fetch-pack.c                         | 2 --
 fetch-pack.h                         | 1 -
 fsck.c                               | 1 -
 fsmonitor--daemon.h                  | 4 +---
 fsmonitor-ipc.c                      | 1 -
 gettext.c                            | 2 --
 gpg-interface.c                      | 1 -
 grep.c                               | 1 -
 http-fetch.c                         | 2 +-
 http-push.c                          | 3 +--
 http-walker.c                        | 1 -
 http.c                               | 2 --
 http.h                               | 1 -
 imap-send.c                          | 2 --
 line-log.c                           | 4 +---
 line-log.h                           | 2 --
 line-range.c                         | 1 -
 list-objects-filter-options.c        | 5 -----
 list-objects-filter.c                | 5 -----
 log-tree.c                           | 1 +
 ls-refs.c                            | 1 -
 merge-blobs.c                        | 2 --
 merge-ort.c                          | 3 ---
 merge-recursive.c                    | 5 -----
 merge.c                              | 3 ---
 midx.h                               | 1 -
 negotiator/noop.c                    | 1 -
 notes-utils.c                        | 1 -
 notes.c                              | 2 --
 object-file.c                        | 8 --------
 object-name.c                        | 2 --
 pack-bitmap-write.c                  | 3 ---
 pack-check.c                         | 1 -
 pack-write.c                         | 1 -
 packfile.c                           | 1 -
 parse-options.c                      | 2 --
 patch-ids.c                          | 1 -
 pkt-line.c                           | 1 +
 pkt-line.h                           | 1 -
 protocol-caps.c                      | 1 -
 reachable.c                          | 1 -
 read-cache.c                         | 2 --
 ref-filter.c                         | 3 ---
 ref-filter.h                         | 1 -
 reflog.c                             | 1 -
 refs/files-backend.c                 | 2 --
 refs/packed-backend.c                | 1 -
 refs/ref-cache.c                     | 1 -
 reftable/dump.c                      | 2 --
 reftable/generic.c                   | 1 -
 reftable/merged.c                    | 1 -
 reftable/merged_test.c               | 1 -
 reftable/reader.c                    | 1 -
 reftable/readwrite_test.c            | 1 -
 reftable/refname_test.c              | 1 -
 reftable/stack_test.c                | 1 -
 reftable/test_framework.c            | 1 -
 reftable/tree_test.c                 | 2 --
 remote-curl.c                        | 3 +--
 remote.c                             | 1 -
 repo-settings.c                      | 1 -
 rerere.c                             | 2 --
 reset.c                              | 1 -
 revision.c                           | 2 --
 run-command.c                        | 2 --
 send-pack.c                          | 2 --
 sequencer.c                          | 3 ---
 setup.c                              | 1 -
 shallow.c                            | 1 -
 shell.c                              | 1 -
 submodule-config.h                   | 2 --
 submodule.c                          | 3 ---
 t/helper/test-bundle-uri.c           | 2 --
 t/helper/test-pkt-line.c             | 1 +
 t/helper/test-reach.c                | 2 --
 t/helper/test-repository.c           | 2 --
 t/helper/test-simple-ipc.c           | 1 -
 t/helper/test-submodule.c            | 1 +
 tmp-objdir.c                         | 1 -
 trace2.c                             | 3 ---
 trace2/tr2_ctr.c                     | 1 -
 trace2/tr2_tgt_normal.c              | 1 +
 trace2/tr2_tls.c                     | 1 +
 trace2/tr2_tls.h                     | 1 -
 trace2/tr2_tmr.c                     | 1 -
 transport-helper.c                   | 2 --
 transport.c                          | 3 ---
 tree.c                               | 3 ---
 upload-pack.c                        | 6 ------
 wrapper.c                            | 1 -
 xdiff-interface.c                    | 2 --
 191 files changed, 25 insertions(+), 335 deletions(-)


base-commit: 564d0252ca632e0264ed670534a51d18a689ef5d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1617%2Fnewren%2Fheader-cleanup-6-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1617/newren/header-cleanup-6-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1617

Range-diff vs v1:

  1:  2f8ff69314e =  1:  2f8ff69314e treewide: remove unnecessary includes from header files
  2:  dbfb108214d !  2:  4dcd52b117e treewide: remove unnecessary includes in source files
     @@ archive.c
       
       static char const * const archive_usage[] = {
      
     - ## attr.c ##
     -@@
     - #include "utf8.h"
     - #include "quote.h"
     - #include "read-cache-ll.h"
     --#include "revision.h"
     - #include "object-store-ll.h"
     - #include "setup.h"
     - #include "thread-utils.h"
     -
       ## bisect.c ##
      @@
       #include "refs.h"
     @@ t/helper/test-bundle-uri.c
       enum input_mode {
       	KEY_VALUE_PAIRS,
      
     - ## t/helper/test-fast-rebase.c ##
     -@@
     - #include "read-cache-ll.h"
     - #include "refs.h"
     - #include "revision.h"
     --#include "sequencer.h"
     - #include "setup.h"
     - #include "strvec.h"
     - #include "tree.h"
     -
       ## t/helper/test-reach.c ##
      @@
       #include "test-tool.h"
     @@ t/helper/test-simple-ipc.c
       #include "parse-options.h"
       #include "thread-utils.h"
      
     - ## t/helper/test-trace2.c ##
     -@@
     - #include "strvec.h"
     - #include "run-command.h"
     - #include "exec-cmd.h"
     --#include "config.h"
     - #include "repository.h"
     - #include "trace2.h"
     - 
     -
       ## tmp-objdir.c ##
      @@
       #include "environment.h"
     @@ tmp-objdir.c
       ## trace2.c ##
      @@
       #include "git-compat-util.h"
     --#include "config.h"
     + #include "config.h"
      -#include "json-writer.h"
      -#include "quote.h"
       #include "repository.h"
  3:  43222a4dac4 =  3:  6211270d678 archive.h: remove unnecessary include
  4:  bd69a954e9a =  4:  e5ba799753d blame.h: remove unnecessary includes
  5:  a2e4bcc56fb =  5:  8ae3696197b fsmonitor--daemon.h: remove unnecessary includes
  6:  393c5ca3a1d =  6:  29b7d47718c http.h: remove unnecessary include
  7:  5f8014882e0 =  7:  7270441cd64 line-log.h: remove unnecessary include
  8:  bc1fe09e996 =  8:  ab91f88a560 pkt-line.h: remove unnecessary include
  9:  6d25811965c =  9:  95688443246 submodule-config.h: remove unnecessary include
 10:  72fd5e2941f = 10:  d9062fb11be trace2/tr2_tls.h: remove unnecessary include
 11:  c11b94bfc7c = 11:  0639ba03d50 treewide: add direct includes currently only pulled in transitively
 12:  57f9da7bba0 = 12:  45f893e8e12 treewide: remove unnecessary includes in source files

-- 
gitgitgadget

^ permalink raw reply

* [PATCH v2] sideband.c: replace int with size_t for clarity
From: Chandra Pratap via GitGitGadget @ 2023-12-23 17:03 UTC (permalink / raw)
  To: git; +Cc: Torsten Bögershausen, Chandra Pratap, Chandra Pratap
In-Reply-To: <pull.1625.git.1703264469238.gitgitgadget@gmail.com>

From: Chandra Pratap <chandrapratap3519@gmail.com>

Replace int with size_t for non-negative values to improve
clarity and remove the 'NEEDSWORK' tag associated with it.wq

Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
---
    sideband.c: replace int with size_t for clarity

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1625%2FChand-ra%2Fdusra-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1625/Chand-ra/dusra-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1625

Range-diff vs v1:

 1:  be67e1bca38 ! 1:  fdacc69ae3b sideband.c: replace int with size_t for clarity
     @@ Metadata
       ## Commit message ##
          sideband.c: replace int with size_t for clarity
      
     -    Replace int with size_t for clarity and remove the
     -    'NEEDSWORK' tag associated with it.
     +    Replace int with size_t for non-negative values to improve
     +    clarity and remove the 'NEEDSWORK' tag associated with it.wq
      
          Signed-off-by: Chandra Pratap <chandrapratap3519@gmail.com>
      
     @@ sideband.c: void list_config_color_sideband_slots(struct string_list *list, cons
       {
       	int i;
       
     +@@ sideband.c: static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
     + 
     + 	for (i = 0; i < ARRAY_SIZE(keywords); i++) {
     + 		struct keyword_entry *p = keywords + i;
     +-		int len = strlen(p->keyword);
     ++		size_t len = strlen(p->keyword);
     + 
     + 		if (n < len)
     + 			continue;


 sideband.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sideband.c b/sideband.c
index 6cbfd391c47..7c3ec0ece29 100644
--- a/sideband.c
+++ b/sideband.c
@@ -69,9 +69,8 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
  * of the line. This should be called for a single line only, which is
  * passed as the first N characters of the SRC array.
  *
- * NEEDSWORK: use "size_t n" instead for clarity.
  */
-static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
+static void maybe_colorize_sideband(struct strbuf *dest, const char *src, size_t n)
 {
 	int i;
 
@@ -88,7 +87,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 
 	for (i = 0; i < ARRAY_SIZE(keywords); i++) {
 		struct keyword_entry *p = keywords + i;
-		int len = strlen(p->keyword);
+		size_t len = strlen(p->keyword);
 
 		if (n < len)
 			continue;

base-commit: 1a87c842ece327d03d08096395969aca5e0a6996
-- 
gitgitgadget

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox