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
