Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)

From: Jeff King <peff@peff.net>
Date: Nov 3, 2021, 11:30 UTC
Message-ID: <YYJy4BuX6JI6p+aV@coredump.intra.peff.net>
In-Reply-To: <211031.86a6ip47ib.gmgdl@evledraar.gmail.com>

On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

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

Show 9 quoted lines

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

Show 7 quoted lines

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

Show 8 quoted lines

> 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