diff mbox series

Makefile: replace most hardcoded object lists with $(wildcard)

Message ID patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series Makefile: replace most hardcoded object lists with $(wildcard) | expand

Commit Message

Ævar Arnfjörð Bjarmason Oct. 30, 2021, 10:32 p.m. UTC
Remove the hardcoded lists of objects in favor of using
$(wildcard). This means that every time a built-in, test tool etc. is
added we won't need to patch the top-level Makefile, except for the
few remaining cases where the asset in question would make it onto one
of our list of exceptions.

Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'
subdirectory, 2010-02-22) this has been relatively easy to do (and
even before that we could glob builtin-*.c). This pattern of
exhaustively enumerating files was then carried forward for
e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty
test-tool program, 2018-03-24).

One reason not to do this is that now a new *.c file at the top-level
will be immediately picked up, so if a new *.c file is being worked on
"make" will error if it doesn't compile, whereas before that file
would need to be explicitly listed in the Makefile. I think than small
trade-off is worth it.

There's a few small "while we're at it" changes here, since I'm
touching the code in question:

 - Start splitting up the the "Guard against the environment" section
   at the top, but don't move anything that exists there out to avoid
   merge conflicts

 - The $(TEST_BUILTINS_OBJS) variable was needlessly complex, because
   it didn't have the full paths we'd pathsubst it back & forth.

 - Introduce *_SRC in addition to *_OBJ for the variable I'm
   touching. Eventually we'll want to do this for all the *.o files,
   i.e. make the *.c list a source of truth for *.o, which means we can
   e.g. use that exhaustive list for "make TAGS".

 - Add a missing "curl-objs" target. See 029bac01a87 (Makefile: add
   {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)
   for the commit that added the rest.

 - De-indent an "ifndef" block, we don't usually indent their
   contents.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

There's probably never a good time to submit a change like this,
i.e. it'll likely always conflict with something, perhaps the
around-release period is paradoxically better than most.

This conflicts with some existing topics (including one of my own to
add the "hook" built in), but those merge conflicts are resolvable by
keeping this side of the conflict. I.e. we'll no longer need to
manually maintain these lists in the Makefile for the common cases.

 Makefile | 484 +++++++------------------------------------------------
 1 file changed, 54 insertions(+), 430 deletions(-)

Comments

Paul Smith Oct. 30, 2021, 11:15 p.m. UTC | #1
On Sun, 2021-10-31 at 00:32 +0200, Ævar Arnfjörð Bjarmason wrote:
> +LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))

Another way to write this would be:

   LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS)))

I don't know that there's any reason to choose one over the other.  I
don't think there's any real performance difference although one could
imagine this version to be VERY SLIGHTLY faster.  Also this one is a
little more "Lisp-ish"... that might be a pro or a con depending :).

Just kibitzing while waiting for dinner to arrive...
Jeff King Oct. 31, 2021, 8:29 a.m. UTC | #2
On Sun, Oct 31, 2021 at 12:32:26AM +0200, Ævar Arnfjörð Bjarmason wrote:

> Remove the hardcoded lists of objects in favor of using
> $(wildcard). This means that every time a built-in, test tool etc. is
> added we won't need to patch the top-level Makefile, except for the
> few remaining cases where the asset in question would make it onto one
> of our list of exceptions.
> 
> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'
> subdirectory, 2010-02-22) this has been relatively easy to do (and
> even before that we could glob builtin-*.c). This pattern of
> exhaustively enumerating files was then carried forward for
> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty
> test-tool program, 2018-03-24).
> 
> One reason not to do this is that now a new *.c file at the top-level
> will be immediately picked up, so if a new *.c file is being worked on
> "make" will error if it doesn't compile, whereas before that file
> would need to be explicitly listed in the Makefile. I think than small
> trade-off is worth it.

A more general way of thinking about this is that we are switching from
"ignore source files by default" to "stick source files into LIB_OBJS by
default". So it's helpful if you were going to stick that file into
LIB_OBJS, but harmful otherwise.

Your "new *.c file" example is one case, because it wouldn't have been
added _yet_. And I agree it's probably not that big a deal in practice.

The other cases are ones similar to what you had to exclude from
LIB_OBJS manually here:

> +LIB_OBJS += $(filter-out \
> +	$(ALL_COMPAT_OBJS) \
> +	git.o common-main.o $(PROGRAM_OBJS) \
> +	$(FUZZ_OBJS) $(CURL_OBJS),\
> +	$(patsubst %.c,%.o,$(wildcard *.c)))

So if I wanted to add a new external program source but forgot to put it
into PROGRAM_OBJS, the default would now be to pick it up in LIB_OBJS.
That's weird and definitely not what you'd want, but presumably you'd
figure it out pretty quickly because we wouldn't have built the command
you expected to exist.

Likewise, there's an interesting tradeoff here for non-program object
files. The current Makefile does not need to mention unix-socket.o
outside of the NO_UNIX_SOCKETS ifdef block, because that's where we
stick it in LIB_OBJS. After your patch, it gets mentioned twice: in that
same spot, but also as an exception to the LIB_OBJS rule (via the
ALL_COMPAT_OBJS variable above).

So we're trading off having to remember to do one thing (add stuff to
LIB_OBJS) for another (add stuff to the exception list). Now one of
those happens a lot more than the other, which is why you get such a
nice diffstat. So it might be worth the tradeoff.

I don't have a very strong opinion either way on this. I felt like we'd
discussed this direction before, and came up with this thread from the
archive:

  https://lore.kernel.org/git/20110222155637.GC27178@sigill.intra.peff.net/

There it was coupled with suggestions to actually change the file
layout. That could make some of those exceptions go away (say, if all of
LIB_OBJS was in "lib/"), but it's a bigger change overall. So I offer it
here mostly for historical context / interest.

I didn't see anything obviously wrong in the patch, but two comments:

>  - De-indent an "ifndef" block, we don't usually indent their
>    contents.

Quite a lot of existing conditional blocks are indented, but I think for
conditional inclusions of one entry in a larger list (where the rest of
the list isn't indented), this makes sense. And that's what you changed
here.

> +# LIB_OBJS: compat/* objects that live at the top-level
> +ALL_COMPAT_OBJS += unix-socket.o
> +ALL_COMPAT_OBJS += unix-stream-server.o
> +ALL_COMPAT_OBJS += sha1dc_git.o

I think "compat" is a misnomer here. For one thing, they're by
definition not "compat/*" objects, because they're not in that
directory. ;) But more importantly, the interesting thing about them is
not that they're compatibility layers, but that they're part of a
conditional compilation. I.e., we might or might not want them, which
will be determined elsewhere in the Makefile, so they must not be part
of the base LIB_OBJS set.

Probably CONDITIONAL_OBJS or something might be more descriptive. That
_could_ be used to include things like CURL_OBJS, but there's probably
value in keeping those in their own list anyway.

Likewise, they could go into a conditional-src/ directory (or some
less-horrible name) to keep them distinct without needing an explicit
list in the Makefile. That's sort of the flip-side of putting all the
other LIB_OBJS ones into lib/.

-Peff
Ævar Arnfjörð Bjarmason Oct. 31, 2021, 1 p.m. UTC | #3
On Sun, Oct 31 2021, Jeff King wrote:

> On Sun, Oct 31, 2021 at 12:32:26AM +0200, Ævar Arnfjörð Bjarmason wrote:
> [...]
>> +# LIB_OBJS: compat/* objects that live at the top-level
>> +ALL_COMPAT_OBJS += unix-socket.o
>> +ALL_COMPAT_OBJS += unix-stream-server.o
>> +ALL_COMPAT_OBJS += sha1dc_git.o
>
> I think "compat" is a misnomer here. For one thing, they're by
> definition not "compat/*" objects, because they're not in that
> directory. ;) But more importantly, the interesting thing about them is
> not that they're compatibility layers, but that they're part of a
> conditional compilation. I.e., we might or might not want them, which
> will be determined elsewhere in the Makefile, so they must not be part
> of the base LIB_OBJS set.
>
> Probably CONDITIONAL_OBJS or something might be more descriptive. That
> _could_ be used to include things like CURL_OBJS, but there's probably
> value in keeping those in their own list anyway.

Good point, will rename them.

> Likewise, they could go into a conditional-src/ directory (or some
> less-horrible name) to keep them distinct without needing an explicit
> list in the Makefile. That's sort of the flip-side of putting all the
> other LIB_OBJS ones into lib/.

The goal here was just to get us rid of tiresome merge conflicts when
two things are added to adjacent part of these lists going forward,
rather than some source-tree reorganization. I didn't search around and
didn't find that 2011-era thread.

I think overall just maintaining the list of the few exceptions is
better than any sort of general mass-move of these files.

Even if we carefully trickle those in at a rate that doesn't conflict
with anything in-flight, the end result will be that e.g.:

    git log -- lib/grep.c

Will stop at that rename commit, similar to builtin/log.c, unless you
specify --follow etc. Just that doesn't make it worth it to me. Likewise
sha1_file.c to sha1-file.c to object-file.c, which is a case I run into
every time I get a "git log" pathspec glob wrong.

Also.

I didn't notice before submitting this but this patch breaks the
vs-build job, because the cmake build in "contrib" is screen-scraping
the Makefile[1].

What's the status of that code? It's rather tiresome to need to patch
two independent and incompatible build systems every time there's some
structural change in the Makefile.

I hadn't looked in any detail at that recipe before, but it the vs-build
job has a hard dependency on GNU make anyway, since we use it for "make
artifacts-tar".

So whatever cmake special-sauce is happening there I don't see why
vs-build couldn't call out "make" for most of the work it's doing, isn't
it just some replacement for what the "vcxproj" target in
config.mak.uname used to do?

1. https://github.com/avar/git/runs/4057171803?check_suite_focus=true
Ævar Arnfjörð Bjarmason Nov. 1, 2021, 8:06 p.m. UTC | #4
On Sat, Oct 30 2021, Paul Smith wrote:

> On Sun, 2021-10-31 at 00:32 +0200, Ævar Arnfjörð Bjarmason wrote:
>> +LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))
>
> Another way to write this would be:
>
>    LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS)))
>
> I don't know that there's any reason to choose one over the other.  I
> don't think there's any real performance difference although one could
> imagine this version to be VERY SLIGHTLY faster.  Also this one is a
> little more "Lisp-ish"... that might be a pro or a con depending :).
>
> Just kibitzing while waiting for dinner to arrive...

Thanks. I changed it to use that in the v2.
Jeff King Nov. 3, 2021, 11:30 a.m. UTC | #5
On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:

> > Likewise, they could go into a conditional-src/ directory (or some
> > less-horrible name) to keep them distinct without needing an explicit
> > list in the Makefile. That's sort of the flip-side of putting all the
> > other LIB_OBJS ones into lib/.
> 
> The goal here was just to get us rid of tiresome merge conflicts when
> two things are added to adjacent part of these lists going forward,
> rather than some source-tree reorganization. I didn't search around and
> didn't find that 2011-era thread.

Right, sorry, I didn't mean to sidetrack. That was just the only
discussion I could find on the topic. I agree that it's worth
considering the two topics (moving files around vs Makefile changes)
separately.

> Even if we carefully trickle those in at a rate that doesn't conflict
> with anything in-flight, the end result will be that e.g.:
> 
>     git log -- lib/grep.c
> 
> Will stop at that rename commit, similar to builtin/log.c, unless you
> specify --follow etc. Just that doesn't make it worth it to me. Likewise
> sha1_file.c to sha1-file.c to object-file.c, which is a case I run into
> every time I get a "git log" pathspec glob wrong.

Agreed. One of the arguments back when we started to move around a few
files is that this dog-fooding would encourage us to make --follow mode
better. That hasn't really happened, though.

> I didn't notice before submitting this but this patch breaks the
> vs-build job, because the cmake build in "contrib" is screen-scraping
> the Makefile[1].
> 
> What's the status of that code? It's rather tiresome to need to patch
> two independent and incompatible build systems every time there's some
> structural change in the Makefile.

My opinion when we took in the cmake topic was that it would be OK for
people working on the main Makefile to break cmake. It's an add-on and
the people who care about cmake are the ones who will do the work to
track the Makefile.

But since there's a CI job that will nag you if it fails, that kind of
makes it everybody's problem in practice. That doesn't change my opinion
on how things _should_ work, but I have done small fixups as necessary
to stop the nagging.

> I hadn't looked in any detail at that recipe before, but it the vs-build
> job has a hard dependency on GNU make anyway, since we use it for "make
> artifacts-tar".
> 
> So whatever cmake special-sauce is happening there I don't see why
> vs-build couldn't call out "make" for most of the work it's doing, isn't
> it just some replacement for what the "vcxproj" target in
> config.mak.uname used to do?

The big question for me is whether that really is a hard dependency.
Obviously "make artifacts-tar" is for the CI job, but is the cmake stuff
supposed to work for regular users without relying on having GNU make at
all? I have no clue.

-Peff
Ævar Arnfjörð Bjarmason Nov. 3, 2021, 2:57 p.m. UTC | #6
On Wed, Nov 03 2021, Jeff King wrote:

> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:
> [...]
>> I didn't notice before submitting this but this patch breaks the
>> vs-build job, because the cmake build in "contrib" is screen-scraping
>> the Makefile[1].
>> 
>> What's the status of that code? It's rather tiresome to need to patch
>> two independent and incompatible build systems every time there's some
>> structural change in the Makefile.
>
> My opinion when we took in the cmake topic was that it would be OK for
> people working on the main Makefile to break cmake. It's an add-on and
> the people who care about cmake are the ones who will do the work to
> track the Makefile.
>
> But since there's a CI job that will nag you if it fails, that kind of
> makes it everybody's problem in practice. That doesn't change my opinion
> on how things _should_ work, but I have done small fixups as necessary
> to stop the nagging.

Yes, that was clearly the intent from reading the original discussion,
but we've crept towards it being an actual hard dependency. I'd also be
fine with some direction that just removed that vs-build/vs-test job to
something optional...

>> I hadn't looked in any detail at that recipe before, but it the vs-build
>> job has a hard dependency on GNU make anyway, since we use it for "make
>> artifacts-tar".
>> 
>> So whatever cmake special-sauce is happening there I don't see why
>> vs-build couldn't call out "make" for most of the work it's doing, isn't
>> it just some replacement for what the "vcxproj" target in
>> config.mak.uname used to do?
>
> The big question for me is whether that really is a hard dependency.
> Obviously "make artifacts-tar" is for the CI job, but is the cmake stuff
> supposed to work for regular users without relying on having GNU make at
> all? I have no clue.

It's a hard dependency for the job, since it tars up its built assets in
the first step, and those are then unpacked and used in subsequent
steps. It's being used to ferry the built binaries between CI phases.

But yes, the intent was clearly to not have a dependency on GNU make,
but as I argue in
<patch-v2-3.3-cd62d8f92d1-20211101T191231Z-avarab@gmail.com> I think
having those developers simply install it is better than forcing us to
maintain two distinct and incompatible build systems.

The selling point was that it was going to be really easy to maintain
them in parallel, i.e. you'd just add a thing to a list here and a list
there, but that assumes that nothing will ever structurally change in
the Makefile.

I think the other X-Y problem being solved there was that cmake has some
better integration for Visual Studio somehow. I.e. it does what the
"vcxproj" target in config.mak.uname does/did.

I think that would be a fine use for cmake, and we can clearly
accomplish that by having our cmake file effectively be a mostly thin
wrapper for logic that lives in the Makefile.

I.e. it would ask the Makefile what's in this list or other, what the
CFLAGS are etc. etc., and feed that into relevant cmake variables.

My patch starts us moving in that direction (but doesn't get anywhere
close to that end-goal). I think if we did that the ~1k line
CMakeLists.txt would be maybe 100-300 lines.
Johannes Schindelin Nov. 4, 2021, 12:31 a.m. UTC | #7
Hi Peff,

On Wed, 3 Nov 2021, Jeff King wrote:

> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > I didn't notice before submitting this but this patch breaks the
> > vs-build job, because the cmake build in "contrib" is screen-scraping
> > the Makefile[1].
> >
> > What's the status of that code? It's rather tiresome to need to patch
> > two independent and incompatible build systems every time there's some
> > structural change in the Makefile.
>
> My opinion when we took in the cmake topic was that it would be OK for
> people working on the main Makefile to break cmake. It's an add-on and
> the people who care about cmake are the ones who will do the work to
> track the Makefile.

I do try to have a look at breakages in `seen` when I have the time, but
lately I didn't. That's why you may have felt more of these CMake
headaches.

> But since there's a CI job that will nag you if it fails, that kind of
> makes it everybody's problem in practice. That doesn't change my opinion
> on how things _should_ work, but I have done small fixups as necessary
> to stop the nagging.

One very simple solution is to leave the Makefile alone unless it really,
really needs to be changed. There are costs to refactoring, and quite
honestly, it might be a good thing that something like a failing vs-build
job discourages refactoring for refactoring's sake.

> > I hadn't looked in any detail at that recipe before, but it the
> > vs-build job has a hard dependency on GNU make anyway, since we use it
> > for "make artifacts-tar".
> >
> > So whatever cmake special-sauce is happening there I don't see why
> > vs-build couldn't call out "make" for most of the work it's doing,
> > isn't it just some replacement for what the "vcxproj" target in
> > config.mak.uname used to do?
>
> The big question for me is whether that really is a hard dependency.
> Obviously "make artifacts-tar" is for the CI job, but is the cmake stuff
> supposed to work for regular users without relying on having GNU make at
> all? I have no clue.

The entire point of the CMake configuration is to allow developers on
Windows to use the tools they are used to, to build Git. And believe it or
not, GNU make is not one of those tools! I know. Very hard to believe. :-)

So yeah, the vs-build/vs-test combo tries to pay attention to this
intended scenario, and avoids using GNU make or any other of those Unix
tools that much less ubiquitous than some might want to believe.

Ciao,
Dscho
Ævar Arnfjörð Bjarmason Nov. 4, 2021, 9:46 a.m. UTC | #8
On Thu, Nov 04 2021, Johannes Schindelin wrote:

> Hi Peff,
>
> On Wed, 3 Nov 2021, Jeff King wrote:
>
>> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> > I didn't notice before submitting this but this patch breaks the
>> > vs-build job, because the cmake build in "contrib" is screen-scraping
>> > the Makefile[1].
>> >
>> > What's the status of that code? It's rather tiresome to need to patch
>> > two independent and incompatible build systems every time there's some
>> > structural change in the Makefile.
>>
>> My opinion when we took in the cmake topic was that it would be OK for
>> people working on the main Makefile to break cmake. It's an add-on and
>> the people who care about cmake are the ones who will do the work to
>> track the Makefile.
>
> I do try to have a look at breakages in `seen` when I have the time, but
> lately I didn't. That's why you may have felt more of these CMake
> headaches.

It's not only things that make it into "seen", as most will test their
topic in GitHub CI before submission in their own repos.

>> But since there's a CI job that will nag you if it fails, that kind of
>> makes it everybody's problem in practice. That doesn't change my opinion
>> on how things _should_ work, but I have done small fixups as necessary
>> to stop the nagging.
>
> One very simple solution is to leave the Makefile alone unless it really,
> really needs to be changed. There are costs to refactoring, and quite
> honestly, it might be a good thing that something like a failing vs-build
> job discourages refactoring for refactoring's sake.

Sure, but that's the case with any critical component we're using. A
question of "is it worth leaving it alone" is distinct from "is it
painful to touch it because you need to implement a fix twice in two
incompatible languages?".

In this case I do think the change is justified. I've personally got a
few local topics that I keep having to (even with rerere) solve
conflicts for due to this list of files, and Junio deals with the same.

Ditto for some of the changes I've made recently to make things
non-.PHONY. That's resulted in major workflow improvements for me,

But in any case, the selling point of the original cmake integration was
not something to the effect of:

    "nobody should have to change this in anything but ever so this
    re-implementation is a one-off"

But rather something like:

    "This re-implementation is a one-off, but any updates to both should
    be trivial."

As someone who's had a couple of recent run-ins with cmake I can tell
you it's really not trivial at all.

So given that the selling point of the original change didn't turn out
as was expected I think it's fair to re-visit whether we'd like to take
this path going forward, or to choose another trade-off.

>> > I hadn't looked in any detail at that recipe before, but it the
>> > vs-build job has a hard dependency on GNU make anyway, since we use it
>> > for "make artifacts-tar".
>> >
>> > So whatever cmake special-sauce is happening there I don't see why
>> > vs-build couldn't call out "make" for most of the work it's doing,
>> > isn't it just some replacement for what the "vcxproj" target in
>> > config.mak.uname used to do?
>>
>> The big question for me is whether that really is a hard dependency.
>> Obviously "make artifacts-tar" is for the CI job, but is the cmake stuff
>> supposed to work for regular users without relying on having GNU make at
>> all? I have no clue.
>
> The entire point of the CMake configuration is to allow developers on
> Windows to use the tools they are used to, to build Git. And believe it or
> not, GNU make is not one of those tools! I know. Very hard to believe. :-)

I believe that, the question is why it isn't a better trade-off to just
ask those users to install that software. Our Windows CI is doing it
on-the-fly, so clearly it's not that hard to do it.

Note that I'm not saying that whatever integration those users get in VS
from the special-cause CMake integration should change. We're only
talking about it invoking "make" under the hood in a way that'll be
invisible to the user.

POSIX "sh" isn't native to Windows either, and that CMake file invokes
shellscripts we ship to e.g. build the generated headers, so this
workflow is clearly something that's OK for an end-user once the one-off
hassle of installing a package is over with.
Philip Oakley Nov. 4, 2021, 2:29 p.m. UTC | #9
On 04/11/2021 09:46, Ævar Arnfjörð Bjarmason wrote:
>> he entire point of the CMake configuration is to allow developers on
>> Windows to use the tools they are used to, to build Git. And believe it or
>> not, GNU make is not one of those tools! I know. Very hard to believe. :-)
> I believe that, the question is why it isn't a better trade-off to just
> ask those users to install that software. Our Windows CI is doing it
> on-the-fly, so clearly it's not that hard to do it.
Just to say that, while it is real easy to download and install the
Git-for-Windows SDK (https://gitforwindows.org/#download-sdk), for most
(Windows) users it's a foreign land, with few friends who understand
what things like `gdb` are all about. It's all doable, but the learning
curve can be hard. The CI doesn't need a learning curve ;-)

Being able to fire up a well 'trusted' tool like Visual Studio to
investigate the code does help contributors understand the code.

--
Philip
Junio C Hamano Nov. 4, 2021, 5:07 p.m. UTC | #10
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> In this case I do think the change is justified. I've personally got a
> few local topics that I keep having to (even with rerere) solve
> conflicts for due to this list of files, and Junio deals with the same.
>
> Ditto for some of the changes I've made recently to make things
> non-.PHONY. That's resulted in major workflow improvements for me,

To me, I haven't noticed any workflow improvements for me, so I do
not see how my name got into the sentence.

> But in any case, the selling point of the original cmake integration was
> not something to the effect of:
>
>     "nobody should have to change this in anything but ever so this
>     re-implementation is a one-off"

I agree that this wasn't how it was sold, but ...

> But rather something like:
>
>     "This re-implementation is a one-off, but any updates to both should
>     be trivial."

... I do not think this was how it was sold, either.  As far as I
recall, it was rather: this may double the maintenance burden, but
the reward to help casual Windows builders is large enough that
those who want to add the CMake support are willing to bear their
share of the burden.

> As someone who's had a couple of recent run-ins with cmake I can tell
> you it's really not trivial at all.

Surely.

> So given that the selling point of the original change didn't turn out
> as was expected I think it's fair to re-visit whether we'd like to take
> this path going forward, or to choose another trade-off.

OK.  The rest of the message I am responding to is your revisiting,
I guess.

>> The entire point of the CMake configuration is to allow developers on
>> Windows to use the tools they are used to, to build Git. And believe it or
>> not, GNU make is not one of those tools! I know. Very hard to believe. :-)
>
> I believe that, the question is why it isn't a better trade-off to just
> ask those users to install that software. Our Windows CI is doing it
> on-the-fly, so clearly it's not that hard to do it.
>
> Note that I'm not saying that whatever integration those users get in VS
> from the special-cause CMake integration should change. We're only
> talking about it invoking "make" under the hood in a way that'll be
> invisible to the user.
>
> POSIX "sh" isn't native to Windows either, and that CMake file invokes
> shellscripts we ship to e.g. build the generated headers, so this
> workflow is clearly something that's OK for an end-user once the one-off
> hassle of installing a package is over with.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 12be39ac497..2f20fa54940 100644
--- a/Makefile
+++ b/Makefile
@@ -590,6 +590,19 @@  TEST_OBJS =
 TEST_PROGRAMS_NEED_X =
 THIRD_PARTY_SOURCES =
 
+## Guard against env: programs
+TEST_PROGRAMS =
+
+## Guard against env: sources
+CURL_SRC =
+TEST_PROGRAMS_NEED_X_SRC =
+XDIFF_SRC =
+
+## Guard against env: objects
+ALL_COMPAT_OBJS =
+CURL_OBJS =
+LIB_OBJS_DIRS =
+
 # Having this variable in your environment would break pipelines because
 # you cause "cd" to echo its destination to stdout.  It can also take
 # scripts to unexpected places.  If you like CDPATH, define it for your
@@ -688,87 +701,23 @@  X =
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
-TEST_BUILTINS_OBJS += test-advise.o
-TEST_BUILTINS_OBJS += test-bitmap.o
-TEST_BUILTINS_OBJS += test-bloom.o
-TEST_BUILTINS_OBJS += test-chmtime.o
-TEST_BUILTINS_OBJS += test-config.o
-TEST_BUILTINS_OBJS += test-crontab.o
-TEST_BUILTINS_OBJS += test-ctype.o
-TEST_BUILTINS_OBJS += test-date.o
-TEST_BUILTINS_OBJS += test-delta.o
-TEST_BUILTINS_OBJS += test-dir-iterator.o
-TEST_BUILTINS_OBJS += test-drop-caches.o
-TEST_BUILTINS_OBJS += test-dump-cache-tree.o
-TEST_BUILTINS_OBJS += test-dump-fsmonitor.o
-TEST_BUILTINS_OBJS += test-dump-split-index.o
-TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
-TEST_BUILTINS_OBJS += test-example-decorate.o
-TEST_BUILTINS_OBJS += test-fast-rebase.o
-TEST_BUILTINS_OBJS += test-genrandom.o
-TEST_BUILTINS_OBJS += test-genzeros.o
-TEST_BUILTINS_OBJS += test-getcwd.o
-TEST_BUILTINS_OBJS += test-hash-speed.o
-TEST_BUILTINS_OBJS += test-hash.o
-TEST_BUILTINS_OBJS += test-hashmap.o
-TEST_BUILTINS_OBJS += test-index-version.o
-TEST_BUILTINS_OBJS += test-json-writer.o
-TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
-TEST_BUILTINS_OBJS += test-match-trees.o
-TEST_BUILTINS_OBJS += test-mergesort.o
-TEST_BUILTINS_OBJS += test-mktemp.o
-TEST_BUILTINS_OBJS += test-oid-array.o
-TEST_BUILTINS_OBJS += test-oidmap.o
-TEST_BUILTINS_OBJS += test-oidtree.o
-TEST_BUILTINS_OBJS += test-online-cpus.o
-TEST_BUILTINS_OBJS += test-parse-options.o
-TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
-TEST_BUILTINS_OBJS += test-partial-clone.o
-TEST_BUILTINS_OBJS += test-path-utils.o
-TEST_BUILTINS_OBJS += test-pcre2-config.o
-TEST_BUILTINS_OBJS += test-pkt-line.o
-TEST_BUILTINS_OBJS += test-prio-queue.o
-TEST_BUILTINS_OBJS += test-proc-receive.o
-TEST_BUILTINS_OBJS += test-progress.o
-TEST_BUILTINS_OBJS += test-reach.o
-TEST_BUILTINS_OBJS += test-read-cache.o
-TEST_BUILTINS_OBJS += test-read-graph.o
-TEST_BUILTINS_OBJS += test-read-midx.o
-TEST_BUILTINS_OBJS += test-ref-store.o
-TEST_BUILTINS_OBJS += test-regex.o
-TEST_BUILTINS_OBJS += test-repository.o
-TEST_BUILTINS_OBJS += test-revision-walking.o
-TEST_BUILTINS_OBJS += test-run-command.o
-TEST_BUILTINS_OBJS += test-scrap-cache-tree.o
-TEST_BUILTINS_OBJS += test-serve-v2.o
-TEST_BUILTINS_OBJS += test-sha1.o
-TEST_BUILTINS_OBJS += test-sha256.o
-TEST_BUILTINS_OBJS += test-sigchain.o
-TEST_BUILTINS_OBJS += test-simple-ipc.o
-TEST_BUILTINS_OBJS += test-strcmp-offset.o
-TEST_BUILTINS_OBJS += test-string-list.o
-TEST_BUILTINS_OBJS += test-submodule-config.o
-TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
-TEST_BUILTINS_OBJS += test-subprocess.o
-TEST_BUILTINS_OBJS += test-trace2.o
-TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
-TEST_BUILTINS_OBJS += test-userdiff.o
-TEST_BUILTINS_OBJS += test-wildmatch.o
-TEST_BUILTINS_OBJS += test-windows-named-pipe.o
-TEST_BUILTINS_OBJS += test-write-cache.o
-TEST_BUILTINS_OBJS += test-xml-encode.o
-
 # Do not add more tests here unless they have extra dependencies. Add
 # them in TEST_BUILTINS_OBJS above.
 TEST_PROGRAMS_NEED_X += test-fake-ssh
 TEST_PROGRAMS_NEED_X += test-tool
 
-TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))
+TEST_PROGRAMS_NEED_X_SRC += $(TEST_PROGRAMS_NEED_X:%=t/helper/%.c)
+TEST_PROGRAMS += $(TEST_PROGRAMS_NEED_X_SRC:%.c=%$X)
+TEST_BUILTINS_SRC += $(filter-out $(TEST_PROGRAMS_NEED_X_SRC),$(wildcard t/helper/*.c))
+TEST_BUILTINS_OBJS += $(TEST_BUILTINS_SRC:%.c=%.o)
 
-# List built-in command $C whose implementation cmd_$C() is not in
-# builtin/$C.o but is linked in as part of some other command.
+# List built-in command $C whose implementation cmd_$C() is in
+# builtin/$C.o
+BUILTIN_OBJS = $(patsubst %.c,%.o,$(wildcard builtin/*.c))
 BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))
 
+# List built-in command $C whose implementation cmd_$C() is not in
+# builtin/$C.o but is linked in as part of some other command.
 BUILT_INS += git-cherry$X
 BUILT_INS += git-cherry-pick$X
 BUILT_INS += git-format-patch$X
@@ -828,355 +777,28 @@  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat
 	-name Documentation -prune -o \
 	-name '*.h' -print)))
 
-LIB_OBJS += abspath.o
-LIB_OBJS += add-interactive.o
-LIB_OBJS += add-patch.o
-LIB_OBJS += advice.o
-LIB_OBJS += alias.o
-LIB_OBJS += alloc.o
-LIB_OBJS += apply.o
-LIB_OBJS += archive-tar.o
-LIB_OBJS += archive-zip.o
-LIB_OBJS += archive.o
-LIB_OBJS += attr.o
-LIB_OBJS += base85.o
-LIB_OBJS += bisect.o
-LIB_OBJS += blame.o
-LIB_OBJS += blob.o
-LIB_OBJS += bloom.o
-LIB_OBJS += branch.o
-LIB_OBJS += bulk-checkin.o
-LIB_OBJS += bundle.o
-LIB_OBJS += cache-tree.o
-LIB_OBJS += cbtree.o
-LIB_OBJS += chdir-notify.o
-LIB_OBJS += checkout.o
-LIB_OBJS += chunk-format.o
-LIB_OBJS += color.o
-LIB_OBJS += column.o
-LIB_OBJS += combine-diff.o
-LIB_OBJS += commit-graph.o
-LIB_OBJS += commit-reach.o
-LIB_OBJS += commit.o
+# LIB_OBJS: compat/* objects that live at the top-level
+ALL_COMPAT_OBJS += unix-socket.o
+ALL_COMPAT_OBJS += unix-stream-server.o
+ALL_COMPAT_OBJS += sha1dc_git.o
+
+# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions
+LIB_OBJS += $(filter-out \
+	$(ALL_COMPAT_OBJS) \
+	git.o common-main.o $(PROGRAM_OBJS) \
+	$(FUZZ_OBJS) $(CURL_OBJS),\
+	$(patsubst %.c,%.o,$(wildcard *.c)))
+
+# LIB_OBJS: Directories that contain only LIB_OBJS
+LIB_OBJS_DIRS += ewah
+LIB_OBJS_DIRS += negotiator
+LIB_OBJS_DIRS += refs
+LIB_OBJS_DIRS += trace2
+LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))
+
+# LIB_OBJS: unconditional compat/* objects
 LIB_OBJS += compat/obstack.o
 LIB_OBJS += compat/terminal.o
-LIB_OBJS += config.o
-LIB_OBJS += connect.o
-LIB_OBJS += connected.o
-LIB_OBJS += convert.o
-LIB_OBJS += copy.o
-LIB_OBJS += credential.o
-LIB_OBJS += csum-file.o
-LIB_OBJS += ctype.o
-LIB_OBJS += date.o
-LIB_OBJS += decorate.o
-LIB_OBJS += delta-islands.o
-LIB_OBJS += diff-delta.o
-LIB_OBJS += diff-merges.o
-LIB_OBJS += diff-lib.o
-LIB_OBJS += diff-no-index.o
-LIB_OBJS += diff.o
-LIB_OBJS += diffcore-break.o
-LIB_OBJS += diffcore-delta.o
-LIB_OBJS += diffcore-order.o
-LIB_OBJS += diffcore-pickaxe.o
-LIB_OBJS += diffcore-rename.o
-LIB_OBJS += diffcore-rotate.o
-LIB_OBJS += dir-iterator.o
-LIB_OBJS += dir.o
-LIB_OBJS += editor.o
-LIB_OBJS += entry.o
-LIB_OBJS += environment.o
-LIB_OBJS += ewah/bitmap.o
-LIB_OBJS += ewah/ewah_bitmap.o
-LIB_OBJS += ewah/ewah_io.o
-LIB_OBJS += ewah/ewah_rlw.o
-LIB_OBJS += exec-cmd.o
-LIB_OBJS += fetch-negotiator.o
-LIB_OBJS += fetch-pack.o
-LIB_OBJS += fmt-merge-msg.o
-LIB_OBJS += fsck.o
-LIB_OBJS += fsmonitor.o
-LIB_OBJS += gettext.o
-LIB_OBJS += gpg-interface.o
-LIB_OBJS += graph.o
-LIB_OBJS += grep.o
-LIB_OBJS += hash-lookup.o
-LIB_OBJS += hashmap.o
-LIB_OBJS += help.o
-LIB_OBJS += hex.o
-LIB_OBJS += hook.o
-LIB_OBJS += ident.o
-LIB_OBJS += json-writer.o
-LIB_OBJS += kwset.o
-LIB_OBJS += levenshtein.o
-LIB_OBJS += line-log.o
-LIB_OBJS += line-range.o
-LIB_OBJS += linear-assignment.o
-LIB_OBJS += list-objects-filter-options.o
-LIB_OBJS += list-objects-filter.o
-LIB_OBJS += list-objects.o
-LIB_OBJS += ll-merge.o
-LIB_OBJS += lockfile.o
-LIB_OBJS += log-tree.o
-LIB_OBJS += ls-refs.o
-LIB_OBJS += mailinfo.o
-LIB_OBJS += mailmap.o
-LIB_OBJS += match-trees.o
-LIB_OBJS += mem-pool.o
-LIB_OBJS += merge-blobs.o
-LIB_OBJS += merge-ort.o
-LIB_OBJS += merge-ort-wrappers.o
-LIB_OBJS += merge-recursive.o
-LIB_OBJS += merge.o
-LIB_OBJS += mergesort.o
-LIB_OBJS += midx.o
-LIB_OBJS += name-hash.o
-LIB_OBJS += negotiator/default.o
-LIB_OBJS += negotiator/noop.o
-LIB_OBJS += negotiator/skipping.o
-LIB_OBJS += notes-cache.o
-LIB_OBJS += notes-merge.o
-LIB_OBJS += notes-utils.o
-LIB_OBJS += notes.o
-LIB_OBJS += object-file.o
-LIB_OBJS += object-name.o
-LIB_OBJS += object.o
-LIB_OBJS += oid-array.o
-LIB_OBJS += oidmap.o
-LIB_OBJS += oidset.o
-LIB_OBJS += oidtree.o
-LIB_OBJS += pack-bitmap-write.o
-LIB_OBJS += pack-bitmap.o
-LIB_OBJS += pack-check.o
-LIB_OBJS += pack-objects.o
-LIB_OBJS += pack-revindex.o
-LIB_OBJS += pack-write.o
-LIB_OBJS += packfile.o
-LIB_OBJS += pager.o
-LIB_OBJS += parallel-checkout.o
-LIB_OBJS += parse-options-cb.o
-LIB_OBJS += parse-options.o
-LIB_OBJS += patch-delta.o
-LIB_OBJS += patch-ids.o
-LIB_OBJS += path.o
-LIB_OBJS += pathspec.o
-LIB_OBJS += pkt-line.o
-LIB_OBJS += preload-index.o
-LIB_OBJS += pretty.o
-LIB_OBJS += prio-queue.o
-LIB_OBJS += progress.o
-LIB_OBJS += promisor-remote.o
-LIB_OBJS += prompt.o
-LIB_OBJS += protocol.o
-LIB_OBJS += protocol-caps.o
-LIB_OBJS += prune-packed.o
-LIB_OBJS += quote.o
-LIB_OBJS += range-diff.o
-LIB_OBJS += reachable.o
-LIB_OBJS += read-cache.o
-LIB_OBJS += rebase-interactive.o
-LIB_OBJS += rebase.o
-LIB_OBJS += ref-filter.o
-LIB_OBJS += reflog-walk.o
-LIB_OBJS += refs.o
-LIB_OBJS += refs/debug.o
-LIB_OBJS += refs/files-backend.o
-LIB_OBJS += refs/iterator.o
-LIB_OBJS += refs/packed-backend.o
-LIB_OBJS += refs/ref-cache.o
-LIB_OBJS += refspec.o
-LIB_OBJS += remote.o
-LIB_OBJS += replace-object.o
-LIB_OBJS += repo-settings.o
-LIB_OBJS += repository.o
-LIB_OBJS += rerere.o
-LIB_OBJS += reset.o
-LIB_OBJS += resolve-undo.o
-LIB_OBJS += revision.o
-LIB_OBJS += run-command.o
-LIB_OBJS += send-pack.o
-LIB_OBJS += sequencer.o
-LIB_OBJS += serve.o
-LIB_OBJS += server-info.o
-LIB_OBJS += setup.o
-LIB_OBJS += shallow.o
-LIB_OBJS += sideband.o
-LIB_OBJS += sigchain.o
-LIB_OBJS += sparse-index.o
-LIB_OBJS += split-index.o
-LIB_OBJS += stable-qsort.o
-LIB_OBJS += strbuf.o
-LIB_OBJS += streaming.o
-LIB_OBJS += string-list.o
-LIB_OBJS += strmap.o
-LIB_OBJS += strvec.o
-LIB_OBJS += sub-process.o
-LIB_OBJS += submodule-config.o
-LIB_OBJS += submodule.o
-LIB_OBJS += symlinks.o
-LIB_OBJS += tag.o
-LIB_OBJS += tempfile.o
-LIB_OBJS += thread-utils.o
-LIB_OBJS += tmp-objdir.o
-LIB_OBJS += trace.o
-LIB_OBJS += trace2.o
-LIB_OBJS += trace2/tr2_cfg.o
-LIB_OBJS += trace2/tr2_cmd_name.o
-LIB_OBJS += trace2/tr2_dst.o
-LIB_OBJS += trace2/tr2_sid.o
-LIB_OBJS += trace2/tr2_sysenv.o
-LIB_OBJS += trace2/tr2_tbuf.o
-LIB_OBJS += trace2/tr2_tgt_event.o
-LIB_OBJS += trace2/tr2_tgt_normal.o
-LIB_OBJS += trace2/tr2_tgt_perf.o
-LIB_OBJS += trace2/tr2_tls.o
-LIB_OBJS += trailer.o
-LIB_OBJS += transport-helper.o
-LIB_OBJS += transport.o
-LIB_OBJS += tree-diff.o
-LIB_OBJS += tree-walk.o
-LIB_OBJS += tree.o
-LIB_OBJS += unpack-trees.o
-LIB_OBJS += upload-pack.o
-LIB_OBJS += url.o
-LIB_OBJS += urlmatch.o
-LIB_OBJS += usage.o
-LIB_OBJS += userdiff.o
-LIB_OBJS += utf8.o
-LIB_OBJS += varint.o
-LIB_OBJS += version.o
-LIB_OBJS += versioncmp.o
-LIB_OBJS += walker.o
-LIB_OBJS += wildmatch.o
-LIB_OBJS += worktree.o
-LIB_OBJS += wrapper.o
-LIB_OBJS += write-or-die.o
-LIB_OBJS += ws.o
-LIB_OBJS += wt-status.o
-LIB_OBJS += xdiff-interface.o
-LIB_OBJS += zlib.o
-
-BUILTIN_OBJS += builtin/add.o
-BUILTIN_OBJS += builtin/am.o
-BUILTIN_OBJS += builtin/annotate.o
-BUILTIN_OBJS += builtin/apply.o
-BUILTIN_OBJS += builtin/archive.o
-BUILTIN_OBJS += builtin/bisect--helper.o
-BUILTIN_OBJS += builtin/blame.o
-BUILTIN_OBJS += builtin/branch.o
-BUILTIN_OBJS += builtin/bugreport.o
-BUILTIN_OBJS += builtin/bundle.o
-BUILTIN_OBJS += builtin/cat-file.o
-BUILTIN_OBJS += builtin/check-attr.o
-BUILTIN_OBJS += builtin/check-ignore.o
-BUILTIN_OBJS += builtin/check-mailmap.o
-BUILTIN_OBJS += builtin/check-ref-format.o
-BUILTIN_OBJS += builtin/checkout--worker.o
-BUILTIN_OBJS += builtin/checkout-index.o
-BUILTIN_OBJS += builtin/checkout.o
-BUILTIN_OBJS += builtin/clean.o
-BUILTIN_OBJS += builtin/clone.o
-BUILTIN_OBJS += builtin/column.o
-BUILTIN_OBJS += builtin/commit-graph.o
-BUILTIN_OBJS += builtin/commit-tree.o
-BUILTIN_OBJS += builtin/commit.o
-BUILTIN_OBJS += builtin/config.o
-BUILTIN_OBJS += builtin/count-objects.o
-BUILTIN_OBJS += builtin/credential-cache--daemon.o
-BUILTIN_OBJS += builtin/credential-cache.o
-BUILTIN_OBJS += builtin/credential-store.o
-BUILTIN_OBJS += builtin/credential.o
-BUILTIN_OBJS += builtin/describe.o
-BUILTIN_OBJS += builtin/diff-files.o
-BUILTIN_OBJS += builtin/diff-index.o
-BUILTIN_OBJS += builtin/diff-tree.o
-BUILTIN_OBJS += builtin/diff.o
-BUILTIN_OBJS += builtin/difftool.o
-BUILTIN_OBJS += builtin/env--helper.o
-BUILTIN_OBJS += builtin/fast-export.o
-BUILTIN_OBJS += builtin/fast-import.o
-BUILTIN_OBJS += builtin/fetch-pack.o
-BUILTIN_OBJS += builtin/fetch.o
-BUILTIN_OBJS += builtin/fmt-merge-msg.o
-BUILTIN_OBJS += builtin/for-each-ref.o
-BUILTIN_OBJS += builtin/for-each-repo.o
-BUILTIN_OBJS += builtin/fsck.o
-BUILTIN_OBJS += builtin/gc.o
-BUILTIN_OBJS += builtin/get-tar-commit-id.o
-BUILTIN_OBJS += builtin/grep.o
-BUILTIN_OBJS += builtin/hash-object.o
-BUILTIN_OBJS += builtin/help.o
-BUILTIN_OBJS += builtin/index-pack.o
-BUILTIN_OBJS += builtin/init-db.o
-BUILTIN_OBJS += builtin/interpret-trailers.o
-BUILTIN_OBJS += builtin/log.o
-BUILTIN_OBJS += builtin/ls-files.o
-BUILTIN_OBJS += builtin/ls-remote.o
-BUILTIN_OBJS += builtin/ls-tree.o
-BUILTIN_OBJS += builtin/mailinfo.o
-BUILTIN_OBJS += builtin/mailsplit.o
-BUILTIN_OBJS += builtin/merge-base.o
-BUILTIN_OBJS += builtin/merge-file.o
-BUILTIN_OBJS += builtin/merge-index.o
-BUILTIN_OBJS += builtin/merge-ours.o
-BUILTIN_OBJS += builtin/merge-recursive.o
-BUILTIN_OBJS += builtin/merge-tree.o
-BUILTIN_OBJS += builtin/merge.o
-BUILTIN_OBJS += builtin/mktag.o
-BUILTIN_OBJS += builtin/mktree.o
-BUILTIN_OBJS += builtin/multi-pack-index.o
-BUILTIN_OBJS += builtin/mv.o
-BUILTIN_OBJS += builtin/name-rev.o
-BUILTIN_OBJS += builtin/notes.o
-BUILTIN_OBJS += builtin/pack-objects.o
-BUILTIN_OBJS += builtin/pack-redundant.o
-BUILTIN_OBJS += builtin/pack-refs.o
-BUILTIN_OBJS += builtin/patch-id.o
-BUILTIN_OBJS += builtin/prune-packed.o
-BUILTIN_OBJS += builtin/prune.o
-BUILTIN_OBJS += builtin/pull.o
-BUILTIN_OBJS += builtin/push.o
-BUILTIN_OBJS += builtin/range-diff.o
-BUILTIN_OBJS += builtin/read-tree.o
-BUILTIN_OBJS += builtin/rebase.o
-BUILTIN_OBJS += builtin/receive-pack.o
-BUILTIN_OBJS += builtin/reflog.o
-BUILTIN_OBJS += builtin/remote-ext.o
-BUILTIN_OBJS += builtin/remote-fd.o
-BUILTIN_OBJS += builtin/remote.o
-BUILTIN_OBJS += builtin/repack.o
-BUILTIN_OBJS += builtin/replace.o
-BUILTIN_OBJS += builtin/rerere.o
-BUILTIN_OBJS += builtin/reset.o
-BUILTIN_OBJS += builtin/rev-list.o
-BUILTIN_OBJS += builtin/rev-parse.o
-BUILTIN_OBJS += builtin/revert.o
-BUILTIN_OBJS += builtin/rm.o
-BUILTIN_OBJS += builtin/send-pack.o
-BUILTIN_OBJS += builtin/shortlog.o
-BUILTIN_OBJS += builtin/show-branch.o
-BUILTIN_OBJS += builtin/show-index.o
-BUILTIN_OBJS += builtin/show-ref.o
-BUILTIN_OBJS += builtin/sparse-checkout.o
-BUILTIN_OBJS += builtin/stash.o
-BUILTIN_OBJS += builtin/stripspace.o
-BUILTIN_OBJS += builtin/submodule--helper.o
-BUILTIN_OBJS += builtin/symbolic-ref.o
-BUILTIN_OBJS += builtin/tag.o
-BUILTIN_OBJS += builtin/unpack-file.o
-BUILTIN_OBJS += builtin/unpack-objects.o
-BUILTIN_OBJS += builtin/update-index.o
-BUILTIN_OBJS += builtin/update-ref.o
-BUILTIN_OBJS += builtin/update-server-info.o
-BUILTIN_OBJS += builtin/upload-archive.o
-BUILTIN_OBJS += builtin/upload-pack.o
-BUILTIN_OBJS += builtin/var.o
-BUILTIN_OBJS += builtin/verify-commit.o
-BUILTIN_OBJS += builtin/verify-pack.o
-BUILTIN_OBJS += builtin/verify-tag.o
-BUILTIN_OBJS += builtin/worktree.o
-BUILTIN_OBJS += builtin/write-tree.o
 
 # THIRD_PARTY_SOURCES is a list of patterns compatible with the
 # $(filter) and $(filter-out) family of functions. They specify source
@@ -2427,17 +2049,12 @@  reconfigure config.mak.autogen: config.status
 .PHONY: reconfigure # This is a convenience target.
 endif
 
-XDIFF_OBJS += xdiff/xdiffi.o
-XDIFF_OBJS += xdiff/xemit.o
-XDIFF_OBJS += xdiff/xhistogram.o
-XDIFF_OBJS += xdiff/xmerge.o
-XDIFF_OBJS += xdiff/xpatience.o
-XDIFF_OBJS += xdiff/xprepare.o
-XDIFF_OBJS += xdiff/xutils.o
+XDIFF_SRC += $(wildcard xdiff/*.c)
+XDIFF_OBJS += $(XDIFF_SRC:.c=.o)
 .PHONY: xdiff-objs
 xdiff-objs: $(XDIFF_OBJS)
 
-TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
+TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(TEST_BUILTINS_OBJS)
 .PHONY: test-objs
 test-objs: $(TEST_OBJS)
 
@@ -2448,13 +2065,20 @@  GIT_OBJS += git.o
 .PHONY: git-objs
 git-objs: $(GIT_OBJS)
 
+CURL_SRC += http.c
+CURL_SRC += http-walker.c
+CURL_SRC += remote-curl.c
+CURL_OBJS += $(CURL_SRC:.c=.o)
+.PHONY: curl-objs
+curl-objs: $(CURL_OBJS)
+
 OBJECTS += $(GIT_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
 OBJECTS += $(XDIFF_OBJS)
 OBJECTS += $(FUZZ_OBJS)
 ifndef NO_CURL
-	OBJECTS += http.o http-walker.o remote-curl.o
+OBJECTS += $(CURL_OBJS)
 endif
 .PHONY: objects
 objects: $(OBJECTS)
@@ -2891,7 +2515,7 @@  perf: all
 
 .PRECIOUS: $(TEST_OBJS)
 
-t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))
+t/helper/test-tool$X: $(TEST_BUILTINS_OBJS)
 
 t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)