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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 31, 2021, 13:00 UTC
Message-ID: <211031.86a6ip47ib.gmgdl@evledraar.gmail.com>
In-Reply-To: <YX5T+wt0hSkxkLHA@coredump.intra.peff.net>

On Sun, Oct 31 2021, Jeff King wrote:

Show 18 quoted lines

> 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