From: Phillip Wood <phillip.wood123@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>,
Paul Smith <paul@mad-scientist.net>,
Sibi Siddharthan <sibisiddharthan.github@gmail.com>
Subject: Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)
Date: Sat, 6 Nov 2021 10:57:17 +0000 [thread overview]
Message-ID: <24482f96-7d87-1570-a171-95ec182f6091@gmail.com> (raw)
In-Reply-To: <patch-v2-3.3-cd62d8f92d1-20211101T191231Z-avarab@gmail.com>
Hi Ævar
On 01/11/2021 19:19, Æ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.
If I need to split up some uncommitted changes into several commits and
I know it is going to be fiddly to do so I will sometimes copy the
original file to foo.safe.c and then edit foo.c to create each commit.
Then I can easily compile and test each commit and editing the file
directly is often easier than using add -p and editing the hunks. With
this patch running make will fail in that case I think.
> We could make this simpler still for the Makefile by moving
> "unix-socket.c" etc. to e.g. a "conditional-src/" directory, likewise
> for $(PROGRAM_OBJS) to e.g. "programs/". If we did that we would not
> need the "$(filter-out)" for LIB_OBJS. I don't think that's worth it,
> e.g. due to "git log -- <path>" on the files now needing a "--follow".
>
> 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.
>
> On the CMake changes here:
>
> - When CMake support was introduced in was introduced
> 061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)
> there was a discussion about the maintenance burden of maintaining the
> top-level Makefile in parallel with CMakeLists.txt[1] where reviewers
> were assured that doing so would simply be a matter of adding something
> to a list in the CMake recipe.
>
> Between change and some recent changes of mine where the "vs-build"
> job failed to a divergence between the Makefile and CMakeList.txt I
> can confidently say that that doesn't at all match reality. Even
> seemingly trivial changes to the Makefile like this one are forcing
> us to do a deep-dive into CMake internals to make forward progress
> with our main build system.
My recollection is that the discussions were about not having to touch
CMakeList.txt when adding new files to the build and I think that
largely works. I don't think a lot of the changes you have been making
recently were anticipated in that discussion.
> - The promised "We can add a (continue-on-error) to vs-build job to
> make this process less of a hindrance." in [2] never materialized.
> Since 4c2c38e800f (ci: modification of main.yml to use cmake for
> vs-build job, 2020-06-26) got a hard dependency on CMake as far as
> getting the CI to pass goes.
>
> - The "vs-build" CI doesn't actually require that there be no GNU make
> usage in the job, as it itself has a hard dependency on running a
> "make -n artifacts-tar" command. So as far as any vs-specific special-sauce
> goes we don't need a GNU-make free build system for vs-build.
We need GNU-make for the ci job but an individual developer using CMake
does not need GNU-make installed. On linux it is possible to build git
without having make installed by using cmake and ninja [1]
> - The stated goal in 061c2240b1b of avoiding a GNU make dependency
> for developer because it requires an SDK that "occupies around two
> gigabytes" and "three quarters of a gigabyte worth of Git objects"
> hardly seems worthwhile trade-off given the above. Disk space is cheap,
> developer time required to maintain two parallel build systems isn't.
That rather assumes everyone has plenty of disk space and a decent
network connection.
> My attempt to amend/revert 4c2c38e800f to have it use the
> pre-4c2c38e800f "make" invocation as a fallback failed, partially
> because I don't have a Windows development environment, so any attempt
> to change it is a painfully slow round-trip to GitHub CI.
>
> Let's instead have CMake call out to the Makefile asking it what the
> definition of various variables lists is, rather than being forced to
> maintain those lists in a way that CMake can parse with regexes (which
> precludes anything but a giant hardcoded list).
>
> I could familiarize myself enough with CMake to do this in some
> CMake-native way, but that would take "just as long as adding it to
> the Makefile"[2] (I think that took me <5 minutes, but I'm several
> hours into fighting with CMake)
>
> So I consider this both a bugfix to the stated aims of this CMake
> integration, and a better way forward for having an alternate build
> system. I.e. If someone really does care about a having a
> GNU-make-less dependency for the "vs-build" I think this change offers
> a much better way forward for that.
I don't see how relying on GNU-make is a step forward for the CMake
integration when it works without it now.
Overall I'm don't think that moving from a known set of dependencies to
"build whatever C files are lying around in this directory" is an
improvement.
Best Wishes
Phillip
[1] The CMake integration is currently broken for non-windows builds,
I've got some fixes at
https://github.com/phillipwood/git/tree/wip/cmake-fixes
> Once we invoke the Makefile to spew out e.g. its idea of "LIB_OBJS",
> it's going to be trivial to do that via some wrapper script that lives
> in "contrib/buildsystems". Such a script would either invoke "make
> print-{var,list}-%", or alternatively use an in-tree committed text
> file with the last known result of such a "make print-{var,list}-%"
> run.
>
> 1. https://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com
> 2. https://lore.kernel.org/git/CAKiG+9Xtof8Hj3npsS-M0SnT_dcjtHjP_+avWB4oOHkaMdnSbw@mail.gmail.com/
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> Makefile | 485 ++++------------------------
> contrib/buildsystems/CMakeLists.txt | 53 ++-
> 2 files changed, 74 insertions(+), 464 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 4139bcf675c..5d78ab6860a 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
> +CONDITIONAL_OBJS =
> +CURL_OBJS =
> +LIB_OBJS_DIRS =
> +
> # Utility to dump whatever variables are defined here
> print-var-%:
> @echo $($*)
> @@ -697,87 +710,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
> @@ -837,355 +786,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
> +CONDITIONAL_OBJS += unix-socket.o
> +CONDITIONAL_OBJS += unix-stream-server.o
> +CONDITIONAL_OBJS += sha1dc_git.o
> +
> +# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions
> +LIB_OBJS += $(filter-out \
> + $(CONDITIONAL_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,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS))))
> +
> +# 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
> @@ -2076,6 +1698,7 @@ endif
> LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)
>
> BASIC_CFLAGS += $(COMPAT_CFLAGS)
> +LIB_OBJS_NO_COMPAT := $(LIB_OBJS)
> LIB_OBJS += $(COMPAT_OBJS)
>
> # Quote for C
> @@ -2436,17 +2059,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)
>
> @@ -2457,13 +2075,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)
> @@ -2900,7 +2525,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)
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fd1399c440f..d57c9203b66 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -111,32 +111,15 @@ project(git
> #TODO Add pcre support
>
> #macros for parsing the Makefile for sources and scripts
> -macro(parse_makefile_for_sources list_var regex)
> - file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+=(.*)")
> - string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}})
> - string(REPLACE "$(COMPAT_OBJS)" "" ${list_var} ${${list_var}}) #remove "$(COMPAT_OBJS)" This is only for libgit.
> - string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces
> - string(REPLACE ".o" ".c;" ${list_var} ${${list_var}}) #change .o to .c, ; is for converting the string into a list
> - list(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list
> - list(REMOVE_ITEM ${list_var} "") #remove empty list elements
> -endmacro()
> -
> -macro(parse_makefile_for_scripts list_var regex lang)
> - file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+=(.*)")
> - string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}})
> - string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces
> - string(REPLACE " " ";" ${list_var} ${${list_var}}) #convert string to a list
> - if(NOT ${lang}) #exclude for SCRIPT_LIB
> - list(TRANSFORM ${list_var} REPLACE "${lang}" "") #do the replacement
> - endif()
> -endmacro()
> -
> -macro(parse_makefile_for_executables list_var regex)
> - file(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX "^${regex} \\+= git-(.*)")
> - string(REPLACE "${regex} +=" "" ${list_var} ${${list_var}})
> +macro(ask_makefile_for_list list_var var_name)
> + execute_process(COMMAND make print-list-${var_name}
> + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
> + OUTPUT_VARIABLE ${list_var})
> + string(REGEX REPLACE "\\.o\n" ".c\n" ${list_var} ${${list_var}}) #change .o to .c
> string(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces
> - string(REPLACE "git-" "" ${list_var} ${${list_var}}) #strip `git-` prefix
> - string(REPLACE "\$X" ";" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list
> + ## Parse the Makefile print-list-% format
> + string(REGEX REPLACE "${var_name} =\n" "" ${list_var} ${${list_var}})
> + string(REGEX REPLACE "${var_name} \\+?= ([^\n]+)" "\\1;" ${list_var} ${${list_var}})
> list(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list
> list(REMOVE_ITEM ${list_var} "") #remove empty list elements
> endmacro()
> @@ -635,14 +618,14 @@ include_directories(${CMAKE_BINARY_DIR})
>
> #build
> #libgit
> -parse_makefile_for_sources(libgit_SOURCES "LIB_OBJS")
> +ask_makefile_for_list(libgit_SOURCES "LIB_OBJS_NO_COMPAT")
>
> list(TRANSFORM libgit_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> list(TRANSFORM compat_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> add_library(libgit ${libgit_SOURCES} ${compat_SOURCES})
>
> #libxdiff
> -parse_makefile_for_sources(libxdiff_SOURCES "XDIFF_OBJS")
> +ask_makefile_for_list(libxdiff_SOURCES "XDIFF_OBJS")
>
> list(TRANSFORM libxdiff_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> add_library(xdiff STATIC ${libxdiff_SOURCES})
> @@ -693,7 +676,7 @@ elseif(UNIX)
> endif()
>
> #git
> -parse_makefile_for_sources(git_SOURCES "BUILTIN_OBJS")
> +ask_makefile_for_list(git_SOURCES "BUILTIN_OBJS")
>
> list(TRANSFORM git_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})
> @@ -729,7 +712,9 @@ if(CURL_FOUND)
> endif()
> endif()
>
> -parse_makefile_for_executables(git_builtin_extra "BUILT_INS")
> +ask_makefile_for_list(git_builtin_extra "BUILT_INS")
> +list(TRANSFORM git_builtin_extra REPLACE "^git-(.*)" "\\1") #strip `git-` prefix
> +list(TRANSFORM git_builtin_extra REPLACE "\.exe$" "") #strip $X
>
> option(SKIP_DASHED_BUILT_INS "Skip hardlinking the dashed versions of the built-ins")
>
> @@ -766,8 +751,8 @@ set(GITWEBDIR ${FALLBACK_RUNTIME_PREFIX}/share/locale)
> set(INSTLIBDIR ${FALLBACK_RUNTIME_PREFIX}/share/perl5)
>
> #shell scripts
> -parse_makefile_for_scripts(git_sh_scripts "SCRIPT_SH" ".sh")
> -parse_makefile_for_scripts(git_shlib_scripts "SCRIPT_LIB" "")
> +ask_makefile_for_list(git_sh_scripts "SCRIPT_SH_GEN")
> +ask_makefile_for_list(git_shlib_scripts "SCRIPT_LIB_GEN")
> set(git_shell_scripts
> ${git_sh_scripts} ${git_shlib_scripts} git-instaweb)
>
> @@ -787,7 +772,7 @@ foreach(script ${git_shell_scripts})
> endforeach()
>
> #perl scripts
> -parse_makefile_for_scripts(git_perl_scripts "SCRIPT_PERL" ".perl")
> +ask_makefile_for_list(git_perl_scripts "SCRIPT_PERL_GEN")
>
> #create perl header
> file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )
> @@ -910,9 +895,9 @@ add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c)
> target_link_libraries(test-fake-ssh common-main)
>
> #test-tool
> -parse_makefile_for_sources(test-tool_SOURCES "TEST_BUILTINS_OBJS")
> +ask_makefile_for_list(test-tool_SOURCES "TEST_BUILTINS_OBJS")
>
> -list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/t/helper/")
> +list(TRANSFORM test-tool_SOURCES PREPEND "${CMAKE_SOURCE_DIR}/")
> add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES})
> target_link_libraries(test-tool common-main)
>
>
next prev parent reply other threads:[~2021-11-06 10:57 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-30 22:32 [PATCH] Makefile: replace most hardcoded object lists with $(wildcard) Ævar Arnfjörð Bjarmason
2021-10-30 23:15 ` Paul Smith
2021-11-01 20:06 ` Ævar Arnfjörð Bjarmason
2021-10-31 8:29 ` Jeff King
2021-10-31 13:00 ` Ævar Arnfjörð Bjarmason
2021-11-03 11:30 ` Jeff King
2021-11-03 14:57 ` Ævar Arnfjörð Bjarmason
2021-11-04 0:31 ` Johannes Schindelin
2021-11-04 9:46 ` Ævar Arnfjörð Bjarmason
2021-11-04 14:29 ` Philip Oakley
2021-11-04 17:07 ` Junio C Hamano
2021-11-01 19:19 ` [PATCH v2 0/3] " Ævar Arnfjörð Bjarmason
2021-11-01 19:19 ` [PATCH v2 1/3] Makefile: rename $(SCRIPT_LIB) to $(SCRIPT_LIB_GEN) Ævar Arnfjörð Bjarmason
2021-11-01 19:19 ` [PATCH v2 2/3] Makefile: add a utility to dump variables Ævar Arnfjörð Bjarmason
2021-11-01 19:19 ` [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard) Ævar Arnfjörð Bjarmason
2021-11-06 10:57 ` Phillip Wood [this message]
2021-11-06 14:27 ` Ævar Arnfjörð Bjarmason
2021-11-06 16:49 ` Phillip Wood
2021-11-06 21:13 ` Ævar Arnfjörð Bjarmason
2021-11-09 21:38 ` Junio C Hamano
2021-11-10 12:39 ` Johannes Schindelin
2021-11-10 13:21 ` Ævar Arnfjörð Bjarmason
2021-11-10 14:59 ` Johannes Schindelin
2021-11-10 15:58 ` Ævar Arnfjörð Bjarmason
2022-01-21 12:01 ` Ævar Arnfjörð Bjarmason
2022-01-21 17:14 ` Phillip Wood
2022-01-21 18:13 ` Ævar Arnfjörð Bjarmason
2022-01-22 6:36 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=24482f96-7d87-1570-a171-95ec182f6091@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=paul@mad-scientist.net \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=sibisiddharthan.github@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).