{"thread":{"id":"56818","subject":"[PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","startedAt":"2021-10-30T22:32:34Z","lastAt":"2022-01-22T06:45:56Z","messageCount":28,"participants":["Ævar Arnfjörð Bjarmason","Paul Smith","Jeff King","Johannes Schindelin","Philip Oakley","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"440182","messageId":"patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com","threadId":"56818","inReplyTo":null,"subject":"[PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-30T22:32:26Z","receivedAt":"2021-10-30T22:32:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the hardcoded lists of objects in favor of using\n$(wildcard). This means that every time a built-in, test tool etc. is\nadded we won't need to patch the top-level Makefile, except for the\nfew remaining cases where the asset in question would make it onto one\nof our list of exceptions.\n\nEver since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\nsubdirectory, 2010-02-22) this has been relatively easy to do (and\neven before that we could glob builtin-*.c). This pattern of\nexhaustively enumerating files was then carried forward for\ne.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\ntest-tool program, 2018-03-24).\n\nOne reason not to do this is that now a new *.c file at the top-level\nwill be immediately picked up, so if a new *.c file is being worked on\n\"make\" will error if it doesn't compile, whereas before that file\nwould need to be explicitly listed in the Makefile. I think than small\ntrade-off is worth it.\n\nThere's a few small \"while we're at it\" changes here, since I'm\ntouching the code in question:\n\n - Start splitting up the the \"Guard against the environment\" section\n   at the top, but don't move anything that exists there out to avoid\n   merge conflicts\n\n - The $(TEST_BUILTINS_OBJS) variable was needlessly complex, because\n   it didn't have the full paths we'd pathsubst it back & forth.\n\n - Introduce *_SRC in addition to *_OBJ for the variable I'm\n   touching. Eventually we'll want to do this for all the *.o files,\n   i.e. make the *.c list a source of truth for *.o, which means we can\n   e.g. use that exhaustive list for \"make TAGS\".\n\n - Add a missing \"curl-objs\" target. See 029bac01a87 (Makefile: add\n   {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)\n   for the commit that added the rest.\n\n - De-indent an \"ifndef\" block, we don't usually indent their\n   contents.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nThere's probably never a good time to submit a change like this,\ni.e. it'll likely always conflict with something, perhaps the\naround-release period is paradoxically better than most.\n\nThis conflicts with some existing topics (including one of my own to\nadd the \"hook\" built in), but those merge conflicts are resolvable by\nkeeping this side of the conflict. I.e. we'll no longer need to\nmanually maintain these lists in the Makefile for the common cases.\n\n Makefile | 484 +++++++------------------------------------------------\n 1 file changed, 54 insertions(+), 430 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 12be39ac497..2f20fa54940 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -590,6 +590,19 @@ TEST_OBJS =\n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n+## Guard against env: programs\n+TEST_PROGRAMS =\n+\n+## Guard against env: sources\n+CURL_SRC =\n+TEST_PROGRAMS_NEED_X_SRC =\n+XDIFF_SRC =\n+\n+## Guard against env: objects\n+ALL_COMPAT_OBJS =\n+CURL_OBJS =\n+LIB_OBJS_DIRS =\n+\n # Having this variable in your environment would break pipelines because\n # you cause \"cd\" to echo its destination to stdout.  It can also take\n # scripts to unexpected places.  If you like CDPATH, define it for your\n@@ -688,87 +701,23 @@ X =\n \n PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \n-TEST_BUILTINS_OBJS += test-advise.o\n-TEST_BUILTINS_OBJS += test-bitmap.o\n-TEST_BUILTINS_OBJS += test-bloom.o\n-TEST_BUILTINS_OBJS += test-chmtime.o\n-TEST_BUILTINS_OBJS += test-config.o\n-TEST_BUILTINS_OBJS += test-crontab.o\n-TEST_BUILTINS_OBJS += test-ctype.o\n-TEST_BUILTINS_OBJS += test-date.o\n-TEST_BUILTINS_OBJS += test-delta.o\n-TEST_BUILTINS_OBJS += test-dir-iterator.o\n-TEST_BUILTINS_OBJS += test-drop-caches.o\n-TEST_BUILTINS_OBJS += test-dump-cache-tree.o\n-TEST_BUILTINS_OBJS += test-dump-fsmonitor.o\n-TEST_BUILTINS_OBJS += test-dump-split-index.o\n-TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n-TEST_BUILTINS_OBJS += test-example-decorate.o\n-TEST_BUILTINS_OBJS += test-fast-rebase.o\n-TEST_BUILTINS_OBJS += test-genrandom.o\n-TEST_BUILTINS_OBJS += test-genzeros.o\n-TEST_BUILTINS_OBJS += test-getcwd.o\n-TEST_BUILTINS_OBJS += test-hash-speed.o\n-TEST_BUILTINS_OBJS += test-hash.o\n-TEST_BUILTINS_OBJS += test-hashmap.o\n-TEST_BUILTINS_OBJS += test-index-version.o\n-TEST_BUILTINS_OBJS += test-json-writer.o\n-TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\n-TEST_BUILTINS_OBJS += test-match-trees.o\n-TEST_BUILTINS_OBJS += test-mergesort.o\n-TEST_BUILTINS_OBJS += test-mktemp.o\n-TEST_BUILTINS_OBJS += test-oid-array.o\n-TEST_BUILTINS_OBJS += test-oidmap.o\n-TEST_BUILTINS_OBJS += test-oidtree.o\n-TEST_BUILTINS_OBJS += test-online-cpus.o\n-TEST_BUILTINS_OBJS += test-parse-options.o\n-TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n-TEST_BUILTINS_OBJS += test-partial-clone.o\n-TEST_BUILTINS_OBJS += test-path-utils.o\n-TEST_BUILTINS_OBJS += test-pcre2-config.o\n-TEST_BUILTINS_OBJS += test-pkt-line.o\n-TEST_BUILTINS_OBJS += test-prio-queue.o\n-TEST_BUILTINS_OBJS += test-proc-receive.o\n-TEST_BUILTINS_OBJS += test-progress.o\n-TEST_BUILTINS_OBJS += test-reach.o\n-TEST_BUILTINS_OBJS += test-read-cache.o\n-TEST_BUILTINS_OBJS += test-read-graph.o\n-TEST_BUILTINS_OBJS += test-read-midx.o\n-TEST_BUILTINS_OBJS += test-ref-store.o\n-TEST_BUILTINS_OBJS += test-regex.o\n-TEST_BUILTINS_OBJS += test-repository.o\n-TEST_BUILTINS_OBJS += test-revision-walking.o\n-TEST_BUILTINS_OBJS += test-run-command.o\n-TEST_BUILTINS_OBJS += test-scrap-cache-tree.o\n-TEST_BUILTINS_OBJS += test-serve-v2.o\n-TEST_BUILTINS_OBJS += test-sha1.o\n-TEST_BUILTINS_OBJS += test-sha256.o\n-TEST_BUILTINS_OBJS += test-sigchain.o\n-TEST_BUILTINS_OBJS += test-simple-ipc.o\n-TEST_BUILTINS_OBJS += test-strcmp-offset.o\n-TEST_BUILTINS_OBJS += test-string-list.o\n-TEST_BUILTINS_OBJS += test-submodule-config.o\n-TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n-TEST_BUILTINS_OBJS += test-subprocess.o\n-TEST_BUILTINS_OBJS += test-trace2.o\n-TEST_BUILTINS_OBJS += test-urlmatch-normalization.o\n-TEST_BUILTINS_OBJS += test-userdiff.o\n-TEST_BUILTINS_OBJS += test-wildmatch.o\n-TEST_BUILTINS_OBJS += test-windows-named-pipe.o\n-TEST_BUILTINS_OBJS += test-write-cache.o\n-TEST_BUILTINS_OBJS += test-xml-encode.o\n-\n # Do not add more tests here unless they have extra dependencies. Add\n # them in TEST_BUILTINS_OBJS above.\n TEST_PROGRAMS_NEED_X += test-fake-ssh\n TEST_PROGRAMS_NEED_X += test-tool\n \n-TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))\n+TEST_PROGRAMS_NEED_X_SRC += $(TEST_PROGRAMS_NEED_X:%=t/helper/%.c)\n+TEST_PROGRAMS += $(TEST_PROGRAMS_NEED_X_SRC:%.c=%$X)\n+TEST_BUILTINS_SRC += $(filter-out $(TEST_PROGRAMS_NEED_X_SRC),$(wildcard t/helper/*.c))\n+TEST_BUILTINS_OBJS += $(TEST_BUILTINS_SRC:%.c=%.o)\n \n-# List built-in command $C whose implementation cmd_$C() is not in\n-# builtin/$C.o but is linked in as part of some other command.\n+# List built-in command $C whose implementation cmd_$C() is in\n+# builtin/$C.o\n+BUILTIN_OBJS = $(patsubst %.c,%.o,$(wildcard builtin/*.c))\n BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))\n \n+# List built-in command $C whose implementation cmd_$C() is not in\n+# builtin/$C.o but is linked in as part of some other command.\n BUILT_INS += git-cherry$X\n BUILT_INS += git-cherry-pick$X\n BUILT_INS += git-format-patch$X\n@@ -828,355 +777,28 @@ LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat\n \t-name Documentation -prune -o \\\n \t-name '*.h' -print)))\n \n-LIB_OBJS += abspath.o\n-LIB_OBJS += add-interactive.o\n-LIB_OBJS += add-patch.o\n-LIB_OBJS += advice.o\n-LIB_OBJS += alias.o\n-LIB_OBJS += alloc.o\n-LIB_OBJS += apply.o\n-LIB_OBJS += archive-tar.o\n-LIB_OBJS += archive-zip.o\n-LIB_OBJS += archive.o\n-LIB_OBJS += attr.o\n-LIB_OBJS += base85.o\n-LIB_OBJS += bisect.o\n-LIB_OBJS += blame.o\n-LIB_OBJS += blob.o\n-LIB_OBJS += bloom.o\n-LIB_OBJS += branch.o\n-LIB_OBJS += bulk-checkin.o\n-LIB_OBJS += bundle.o\n-LIB_OBJS += cache-tree.o\n-LIB_OBJS += cbtree.o\n-LIB_OBJS += chdir-notify.o\n-LIB_OBJS += checkout.o\n-LIB_OBJS += chunk-format.o\n-LIB_OBJS += color.o\n-LIB_OBJS += column.o\n-LIB_OBJS += combine-diff.o\n-LIB_OBJS += commit-graph.o\n-LIB_OBJS += commit-reach.o\n-LIB_OBJS += commit.o\n+# LIB_OBJS: compat/* objects that live at the top-level\n+ALL_COMPAT_OBJS += unix-socket.o\n+ALL_COMPAT_OBJS += unix-stream-server.o\n+ALL_COMPAT_OBJS += sha1dc_git.o\n+\n+# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions\n+LIB_OBJS += $(filter-out \\\n+\t$(ALL_COMPAT_OBJS) \\\n+\tgit.o common-main.o $(PROGRAM_OBJS) \\\n+\t$(FUZZ_OBJS) $(CURL_OBJS),\\\n+\t$(patsubst %.c,%.o,$(wildcard *.c)))\n+\n+# LIB_OBJS: Directories that contain only LIB_OBJS\n+LIB_OBJS_DIRS += ewah\n+LIB_OBJS_DIRS += negotiator\n+LIB_OBJS_DIRS += refs\n+LIB_OBJS_DIRS += trace2\n+LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))\n+\n+# LIB_OBJS: unconditional compat/* objects\n LIB_OBJS += compat/obstack.o\n LIB_OBJS += compat/terminal.o\n-LIB_OBJS += config.o\n-LIB_OBJS += connect.o\n-LIB_OBJS += connected.o\n-LIB_OBJS += convert.o\n-LIB_OBJS += copy.o\n-LIB_OBJS += credential.o\n-LIB_OBJS += csum-file.o\n-LIB_OBJS += ctype.o\n-LIB_OBJS += date.o\n-LIB_OBJS += decorate.o\n-LIB_OBJS += delta-islands.o\n-LIB_OBJS += diff-delta.o\n-LIB_OBJS += diff-merges.o\n-LIB_OBJS += diff-lib.o\n-LIB_OBJS += diff-no-index.o\n-LIB_OBJS += diff.o\n-LIB_OBJS += diffcore-break.o\n-LIB_OBJS += diffcore-delta.o\n-LIB_OBJS += diffcore-order.o\n-LIB_OBJS += diffcore-pickaxe.o\n-LIB_OBJS += diffcore-rename.o\n-LIB_OBJS += diffcore-rotate.o\n-LIB_OBJS += dir-iterator.o\n-LIB_OBJS += dir.o\n-LIB_OBJS += editor.o\n-LIB_OBJS += entry.o\n-LIB_OBJS += environment.o\n-LIB_OBJS += ewah/bitmap.o\n-LIB_OBJS += ewah/ewah_bitmap.o\n-LIB_OBJS += ewah/ewah_io.o\n-LIB_OBJS += ewah/ewah_rlw.o\n-LIB_OBJS += exec-cmd.o\n-LIB_OBJS += fetch-negotiator.o\n-LIB_OBJS += fetch-pack.o\n-LIB_OBJS += fmt-merge-msg.o\n-LIB_OBJS += fsck.o\n-LIB_OBJS += fsmonitor.o\n-LIB_OBJS += gettext.o\n-LIB_OBJS += gpg-interface.o\n-LIB_OBJS += graph.o\n-LIB_OBJS += grep.o\n-LIB_OBJS += hash-lookup.o\n-LIB_OBJS += hashmap.o\n-LIB_OBJS += help.o\n-LIB_OBJS += hex.o\n-LIB_OBJS += hook.o\n-LIB_OBJS += ident.o\n-LIB_OBJS += json-writer.o\n-LIB_OBJS += kwset.o\n-LIB_OBJS += levenshtein.o\n-LIB_OBJS += line-log.o\n-LIB_OBJS += line-range.o\n-LIB_OBJS += linear-assignment.o\n-LIB_OBJS += list-objects-filter-options.o\n-LIB_OBJS += list-objects-filter.o\n-LIB_OBJS += list-objects.o\n-LIB_OBJS += ll-merge.o\n-LIB_OBJS += lockfile.o\n-LIB_OBJS += log-tree.o\n-LIB_OBJS += ls-refs.o\n-LIB_OBJS += mailinfo.o\n-LIB_OBJS += mailmap.o\n-LIB_OBJS += match-trees.o\n-LIB_OBJS += mem-pool.o\n-LIB_OBJS += merge-blobs.o\n-LIB_OBJS += merge-ort.o\n-LIB_OBJS += merge-ort-wrappers.o\n-LIB_OBJS += merge-recursive.o\n-LIB_OBJS += merge.o\n-LIB_OBJS += mergesort.o\n-LIB_OBJS += midx.o\n-LIB_OBJS += name-hash.o\n-LIB_OBJS += negotiator/default.o\n-LIB_OBJS += negotiator/noop.o\n-LIB_OBJS += negotiator/skipping.o\n-LIB_OBJS += notes-cache.o\n-LIB_OBJS += notes-merge.o\n-LIB_OBJS += notes-utils.o\n-LIB_OBJS += notes.o\n-LIB_OBJS += object-file.o\n-LIB_OBJS += object-name.o\n-LIB_OBJS += object.o\n-LIB_OBJS += oid-array.o\n-LIB_OBJS += oidmap.o\n-LIB_OBJS += oidset.o\n-LIB_OBJS += oidtree.o\n-LIB_OBJS += pack-bitmap-write.o\n-LIB_OBJS += pack-bitmap.o\n-LIB_OBJS += pack-check.o\n-LIB_OBJS += pack-objects.o\n-LIB_OBJS += pack-revindex.o\n-LIB_OBJS += pack-write.o\n-LIB_OBJS += packfile.o\n-LIB_OBJS += pager.o\n-LIB_OBJS += parallel-checkout.o\n-LIB_OBJS += parse-options-cb.o\n-LIB_OBJS += parse-options.o\n-LIB_OBJS += patch-delta.o\n-LIB_OBJS += patch-ids.o\n-LIB_OBJS += path.o\n-LIB_OBJS += pathspec.o\n-LIB_OBJS += pkt-line.o\n-LIB_OBJS += preload-index.o\n-LIB_OBJS += pretty.o\n-LIB_OBJS += prio-queue.o\n-LIB_OBJS += progress.o\n-LIB_OBJS += promisor-remote.o\n-LIB_OBJS += prompt.o\n-LIB_OBJS += protocol.o\n-LIB_OBJS += protocol-caps.o\n-LIB_OBJS += prune-packed.o\n-LIB_OBJS += quote.o\n-LIB_OBJS += range-diff.o\n-LIB_OBJS += reachable.o\n-LIB_OBJS += read-cache.o\n-LIB_OBJS += rebase-interactive.o\n-LIB_OBJS += rebase.o\n-LIB_OBJS += ref-filter.o\n-LIB_OBJS += reflog-walk.o\n-LIB_OBJS += refs.o\n-LIB_OBJS += refs/debug.o\n-LIB_OBJS += refs/files-backend.o\n-LIB_OBJS += refs/iterator.o\n-LIB_OBJS += refs/packed-backend.o\n-LIB_OBJS += refs/ref-cache.o\n-LIB_OBJS += refspec.o\n-LIB_OBJS += remote.o\n-LIB_OBJS += replace-object.o\n-LIB_OBJS += repo-settings.o\n-LIB_OBJS += repository.o\n-LIB_OBJS += rerere.o\n-LIB_OBJS += reset.o\n-LIB_OBJS += resolve-undo.o\n-LIB_OBJS += revision.o\n-LIB_OBJS += run-command.o\n-LIB_OBJS += send-pack.o\n-LIB_OBJS += sequencer.o\n-LIB_OBJS += serve.o\n-LIB_OBJS += server-info.o\n-LIB_OBJS += setup.o\n-LIB_OBJS += shallow.o\n-LIB_OBJS += sideband.o\n-LIB_OBJS += sigchain.o\n-LIB_OBJS += sparse-index.o\n-LIB_OBJS += split-index.o\n-LIB_OBJS += stable-qsort.o\n-LIB_OBJS += strbuf.o\n-LIB_OBJS += streaming.o\n-LIB_OBJS += string-list.o\n-LIB_OBJS += strmap.o\n-LIB_OBJS += strvec.o\n-LIB_OBJS += sub-process.o\n-LIB_OBJS += submodule-config.o\n-LIB_OBJS += submodule.o\n-LIB_OBJS += symlinks.o\n-LIB_OBJS += tag.o\n-LIB_OBJS += tempfile.o\n-LIB_OBJS += thread-utils.o\n-LIB_OBJS += tmp-objdir.o\n-LIB_OBJS += trace.o\n-LIB_OBJS += trace2.o\n-LIB_OBJS += trace2/tr2_cfg.o\n-LIB_OBJS += trace2/tr2_cmd_name.o\n-LIB_OBJS += trace2/tr2_dst.o\n-LIB_OBJS += trace2/tr2_sid.o\n-LIB_OBJS += trace2/tr2_sysenv.o\n-LIB_OBJS += trace2/tr2_tbuf.o\n-LIB_OBJS += trace2/tr2_tgt_event.o\n-LIB_OBJS += trace2/tr2_tgt_normal.o\n-LIB_OBJS += trace2/tr2_tgt_perf.o\n-LIB_OBJS += trace2/tr2_tls.o\n-LIB_OBJS += trailer.o\n-LIB_OBJS += transport-helper.o\n-LIB_OBJS += transport.o\n-LIB_OBJS += tree-diff.o\n-LIB_OBJS += tree-walk.o\n-LIB_OBJS += tree.o\n-LIB_OBJS += unpack-trees.o\n-LIB_OBJS += upload-pack.o\n-LIB_OBJS += url.o\n-LIB_OBJS += urlmatch.o\n-LIB_OBJS += usage.o\n-LIB_OBJS += userdiff.o\n-LIB_OBJS += utf8.o\n-LIB_OBJS += varint.o\n-LIB_OBJS += version.o\n-LIB_OBJS += versioncmp.o\n-LIB_OBJS += walker.o\n-LIB_OBJS += wildmatch.o\n-LIB_OBJS += worktree.o\n-LIB_OBJS += wrapper.o\n-LIB_OBJS += write-or-die.o\n-LIB_OBJS += ws.o\n-LIB_OBJS += wt-status.o\n-LIB_OBJS += xdiff-interface.o\n-LIB_OBJS += zlib.o\n-\n-BUILTIN_OBJS += builtin/add.o\n-BUILTIN_OBJS += builtin/am.o\n-BUILTIN_OBJS += builtin/annotate.o\n-BUILTIN_OBJS += builtin/apply.o\n-BUILTIN_OBJS += builtin/archive.o\n-BUILTIN_OBJS += builtin/bisect--helper.o\n-BUILTIN_OBJS += builtin/blame.o\n-BUILTIN_OBJS += builtin/branch.o\n-BUILTIN_OBJS += builtin/bugreport.o\n-BUILTIN_OBJS += builtin/bundle.o\n-BUILTIN_OBJS += builtin/cat-file.o\n-BUILTIN_OBJS += builtin/check-attr.o\n-BUILTIN_OBJS += builtin/check-ignore.o\n-BUILTIN_OBJS += builtin/check-mailmap.o\n-BUILTIN_OBJS += builtin/check-ref-format.o\n-BUILTIN_OBJS += builtin/checkout--worker.o\n-BUILTIN_OBJS += builtin/checkout-index.o\n-BUILTIN_OBJS += builtin/checkout.o\n-BUILTIN_OBJS += builtin/clean.o\n-BUILTIN_OBJS += builtin/clone.o\n-BUILTIN_OBJS += builtin/column.o\n-BUILTIN_OBJS += builtin/commit-graph.o\n-BUILTIN_OBJS += builtin/commit-tree.o\n-BUILTIN_OBJS += builtin/commit.o\n-BUILTIN_OBJS += builtin/config.o\n-BUILTIN_OBJS += builtin/count-objects.o\n-BUILTIN_OBJS += builtin/credential-cache--daemon.o\n-BUILTIN_OBJS += builtin/credential-cache.o\n-BUILTIN_OBJS += builtin/credential-store.o\n-BUILTIN_OBJS += builtin/credential.o\n-BUILTIN_OBJS += builtin/describe.o\n-BUILTIN_OBJS += builtin/diff-files.o\n-BUILTIN_OBJS += builtin/diff-index.o\n-BUILTIN_OBJS += builtin/diff-tree.o\n-BUILTIN_OBJS += builtin/diff.o\n-BUILTIN_OBJS += builtin/difftool.o\n-BUILTIN_OBJS += builtin/env--helper.o\n-BUILTIN_OBJS += builtin/fast-export.o\n-BUILTIN_OBJS += builtin/fast-import.o\n-BUILTIN_OBJS += builtin/fetch-pack.o\n-BUILTIN_OBJS += builtin/fetch.o\n-BUILTIN_OBJS += builtin/fmt-merge-msg.o\n-BUILTIN_OBJS += builtin/for-each-ref.o\n-BUILTIN_OBJS += builtin/for-each-repo.o\n-BUILTIN_OBJS += builtin/fsck.o\n-BUILTIN_OBJS += builtin/gc.o\n-BUILTIN_OBJS += builtin/get-tar-commit-id.o\n-BUILTIN_OBJS += builtin/grep.o\n-BUILTIN_OBJS += builtin/hash-object.o\n-BUILTIN_OBJS += builtin/help.o\n-BUILTIN_OBJS += builtin/index-pack.o\n-BUILTIN_OBJS += builtin/init-db.o\n-BUILTIN_OBJS += builtin/interpret-trailers.o\n-BUILTIN_OBJS += builtin/log.o\n-BUILTIN_OBJS += builtin/ls-files.o\n-BUILTIN_OBJS += builtin/ls-remote.o\n-BUILTIN_OBJS += builtin/ls-tree.o\n-BUILTIN_OBJS += builtin/mailinfo.o\n-BUILTIN_OBJS += builtin/mailsplit.o\n-BUILTIN_OBJS += builtin/merge-base.o\n-BUILTIN_OBJS += builtin/merge-file.o\n-BUILTIN_OBJS += builtin/merge-index.o\n-BUILTIN_OBJS += builtin/merge-ours.o\n-BUILTIN_OBJS += builtin/merge-recursive.o\n-BUILTIN_OBJS += builtin/merge-tree.o\n-BUILTIN_OBJS += builtin/merge.o\n-BUILTIN_OBJS += builtin/mktag.o\n-BUILTIN_OBJS += builtin/mktree.o\n-BUILTIN_OBJS += builtin/multi-pack-index.o\n-BUILTIN_OBJS += builtin/mv.o\n-BUILTIN_OBJS += builtin/name-rev.o\n-BUILTIN_OBJS += builtin/notes.o\n-BUILTIN_OBJS += builtin/pack-objects.o\n-BUILTIN_OBJS += builtin/pack-redundant.o\n-BUILTIN_OBJS += builtin/pack-refs.o\n-BUILTIN_OBJS += builtin/patch-id.o\n-BUILTIN_OBJS += builtin/prune-packed.o\n-BUILTIN_OBJS += builtin/prune.o\n-BUILTIN_OBJS += builtin/pull.o\n-BUILTIN_OBJS += builtin/push.o\n-BUILTIN_OBJS += builtin/range-diff.o\n-BUILTIN_OBJS += builtin/read-tree.o\n-BUILTIN_OBJS += builtin/rebase.o\n-BUILTIN_OBJS += builtin/receive-pack.o\n-BUILTIN_OBJS += builtin/reflog.o\n-BUILTIN_OBJS += builtin/remote-ext.o\n-BUILTIN_OBJS += builtin/remote-fd.o\n-BUILTIN_OBJS += builtin/remote.o\n-BUILTIN_OBJS += builtin/repack.o\n-BUILTIN_OBJS += builtin/replace.o\n-BUILTIN_OBJS += builtin/rerere.o\n-BUILTIN_OBJS += builtin/reset.o\n-BUILTIN_OBJS += builtin/rev-list.o\n-BUILTIN_OBJS += builtin/rev-parse.o\n-BUILTIN_OBJS += builtin/revert.o\n-BUILTIN_OBJS += builtin/rm.o\n-BUILTIN_OBJS += builtin/send-pack.o\n-BUILTIN_OBJS += builtin/shortlog.o\n-BUILTIN_OBJS += builtin/show-branch.o\n-BUILTIN_OBJS += builtin/show-index.o\n-BUILTIN_OBJS += builtin/show-ref.o\n-BUILTIN_OBJS += builtin/sparse-checkout.o\n-BUILTIN_OBJS += builtin/stash.o\n-BUILTIN_OBJS += builtin/stripspace.o\n-BUILTIN_OBJS += builtin/submodule--helper.o\n-BUILTIN_OBJS += builtin/symbolic-ref.o\n-BUILTIN_OBJS += builtin/tag.o\n-BUILTIN_OBJS += builtin/unpack-file.o\n-BUILTIN_OBJS += builtin/unpack-objects.o\n-BUILTIN_OBJS += builtin/update-index.o\n-BUILTIN_OBJS += builtin/update-ref.o\n-BUILTIN_OBJS += builtin/update-server-info.o\n-BUILTIN_OBJS += builtin/upload-archive.o\n-BUILTIN_OBJS += builtin/upload-pack.o\n-BUILTIN_OBJS += builtin/var.o\n-BUILTIN_OBJS += builtin/verify-commit.o\n-BUILTIN_OBJS += builtin/verify-pack.o\n-BUILTIN_OBJS += builtin/verify-tag.o\n-BUILTIN_OBJS += builtin/worktree.o\n-BUILTIN_OBJS += builtin/write-tree.o\n \n # THIRD_PARTY_SOURCES is a list of patterns compatible with the\n # $(filter) and $(filter-out) family of functions. They specify source\n@@ -2427,17 +2049,12 @@ reconfigure config.mak.autogen: config.status\n .PHONY: reconfigure # This is a convenience target.\n endif\n \n-XDIFF_OBJS += xdiff/xdiffi.o\n-XDIFF_OBJS += xdiff/xemit.o\n-XDIFF_OBJS += xdiff/xhistogram.o\n-XDIFF_OBJS += xdiff/xmerge.o\n-XDIFF_OBJS += xdiff/xpatience.o\n-XDIFF_OBJS += xdiff/xprepare.o\n-XDIFF_OBJS += xdiff/xutils.o\n+XDIFF_SRC += $(wildcard xdiff/*.c)\n+XDIFF_OBJS += $(XDIFF_SRC:.c=.o)\n .PHONY: xdiff-objs\n xdiff-objs: $(XDIFF_OBJS)\n \n-TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(TEST_BUILTINS_OBJS)\n .PHONY: test-objs\n test-objs: $(TEST_OBJS)\n \n@@ -2448,13 +2065,20 @@ GIT_OBJS += git.o\n .PHONY: git-objs\n git-objs: $(GIT_OBJS)\n \n+CURL_SRC += http.c\n+CURL_SRC += http-walker.c\n+CURL_SRC += remote-curl.c\n+CURL_OBJS += $(CURL_SRC:.c=.o)\n+.PHONY: curl-objs\n+curl-objs: $(CURL_OBJS)\n+\n OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n OBJECTS += $(FUZZ_OBJS)\n ifndef NO_CURL\n-\tOBJECTS += http.o http-walker.o remote-curl.o\n+OBJECTS += $(CURL_OBJS)\n endif\n .PHONY: objects\n objects: $(OBJECTS)\n@@ -2891,7 +2515,7 @@ perf: all\n \n .PRECIOUS: $(TEST_OBJS)\n \n-t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+t/helper/test-tool$X: $(TEST_BUILTINS_OBJS)\n \n t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n-- \n2.33.1.1570.g069344fdd45\n\n"},{"id":"440184","messageId":"f5807fd2eb0c16ab0400e8f3138a90f564fd303c.camel@mad-scientist.net","threadId":"56818","inReplyTo":"patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Paul Smith","fromEmail":"paul@mad-scientist.net","sentAt":"2021-10-30T23:15:14Z","receivedAt":"2021-10-30T23:36:06Z","isPatch":true,"sender":{"key":"paul@mad-scientist.net","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Sun, 2021-10-31 at 00:32 +0200, Ævar Arnfjörð Bjarmason wrote:\n> +LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))\n\nAnother way to write this would be:\n\n   LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS)))\n\nI don't know that there's any reason to choose one over the other.  I\ndon't think there's any real performance difference although one could\nimagine this version to be VERY SLIGHTLY faster.  Also this one is a\nlittle more \"Lisp-ish\"... that might be a pro or a con depending :).\n\nJust kibitzing while waiting for dinner to arrive...\n\n"},{"id":"440192","messageId":"YX5T+wt0hSkxkLHA@coredump.intra.peff.net","threadId":"56818","inReplyTo":"patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-31T08:29:47Z","receivedAt":"2021-10-31T08:29:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 31, 2021 at 12:32:26AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Remove the hardcoded lists of objects in favor of using\n> $(wildcard). This means that every time a built-in, test tool etc. is\n> added we won't need to patch the top-level Makefile, except for the\n> few remaining cases where the asset in question would make it onto one\n> of our list of exceptions.\n> \n> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\n> subdirectory, 2010-02-22) this has been relatively easy to do (and\n> even before that we could glob builtin-*.c). This pattern of\n> exhaustively enumerating files was then carried forward for\n> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\n> test-tool program, 2018-03-24).\n> \n> One reason not to do this is that now a new *.c file at the top-level\n> will be immediately picked up, so if a new *.c file is being worked on\n> \"make\" will error if it doesn't compile, whereas before that file\n> would need to be explicitly listed in the Makefile. I think than small\n> trade-off is worth it.\n\nA more general way of thinking about this is that we are switching from\n\"ignore source files by default\" to \"stick source files into LIB_OBJS by\ndefault\". So it's helpful if you were going to stick that file into\nLIB_OBJS, but harmful otherwise.\n\nYour \"new *.c file\" example is one case, because it wouldn't have been\nadded _yet_. And I agree it's probably not that big a deal in practice.\n\nThe other cases are ones similar to what you had to exclude from\nLIB_OBJS manually here:\n\n> +LIB_OBJS += $(filter-out \\\n> +\t$(ALL_COMPAT_OBJS) \\\n> +\tgit.o common-main.o $(PROGRAM_OBJS) \\\n> +\t$(FUZZ_OBJS) $(CURL_OBJS),\\\n> +\t$(patsubst %.c,%.o,$(wildcard *.c)))\n\nSo if I wanted to add a new external program source but forgot to put it\ninto PROGRAM_OBJS, the default would now be to pick it up in LIB_OBJS.\nThat's weird and definitely not what you'd want, but presumably you'd\nfigure it out pretty quickly because we wouldn't have built the command\nyou expected to exist.\n\nLikewise, there's an interesting tradeoff here for non-program object\nfiles. The current Makefile does not need to mention unix-socket.o\noutside of the NO_UNIX_SOCKETS ifdef block, because that's where we\nstick it in LIB_OBJS. After your patch, it gets mentioned twice: in that\nsame spot, but also as an exception to the LIB_OBJS rule (via the\nALL_COMPAT_OBJS variable above).\n\nSo we're trading off having to remember to do one thing (add stuff to\nLIB_OBJS) for another (add stuff to the exception list). Now one of\nthose happens a lot more than the other, which is why you get such a\nnice diffstat. So it might be worth the tradeoff.\n\nI don't have a very strong opinion either way on this. I felt like we'd\ndiscussed this direction before, and came up with this thread from the\narchive:\n\n  https://lore.kernel.org/git/20110222155637.GC27178@sigill.intra.peff.net/\n\nThere it was coupled with suggestions to actually change the file\nlayout. That could make some of those exceptions go away (say, if all of\nLIB_OBJS was in \"lib/\"), but it's a bigger change overall. So I offer it\nhere mostly for historical context / interest.\n\nI didn't see anything obviously wrong in the patch, but two comments:\n\n>  - De-indent an \"ifndef\" block, we don't usually indent their\n>    contents.\n\nQuite a lot of existing conditional blocks are indented, but I think for\nconditional inclusions of one entry in a larger list (where the rest of\nthe list isn't indented), this makes sense. And that's what you changed\nhere.\n\n> +# LIB_OBJS: compat/* objects that live at the top-level\n> +ALL_COMPAT_OBJS += unix-socket.o\n> +ALL_COMPAT_OBJS += unix-stream-server.o\n> +ALL_COMPAT_OBJS += sha1dc_git.o\n\nI think \"compat\" is a misnomer here. For one thing, they're by\ndefinition not \"compat/*\" objects, because they're not in that\ndirectory. ;) But more importantly, the interesting thing about them is\nnot that they're compatibility layers, but that they're part of a\nconditional compilation. I.e., we might or might not want them, which\nwill be determined elsewhere in the Makefile, so they must not be part\nof the base LIB_OBJS set.\n\nProbably CONDITIONAL_OBJS or something might be more descriptive. That\n_could_ be used to include things like CURL_OBJS, but there's probably\nvalue in keeping those in their own list anyway.\n\nLikewise, they could go into a conditional-src/ directory (or some\nless-horrible name) to keep them distinct without needing an explicit\nlist in the Makefile. That's sort of the flip-side of putting all the\nother LIB_OBJS ones into lib/.\n\n-Peff\n"},{"id":"440196","messageId":"211031.86a6ip47ib.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"YX5T+wt0hSkxkLHA@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-31T13:00:42Z","receivedAt":"2021-10-31T14:02:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Oct 31 2021, Jeff King wrote:\n\n> On Sun, Oct 31, 2021 at 12:32:26AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> [...]\n>> +# LIB_OBJS: compat/* objects that live at the top-level\n>> +ALL_COMPAT_OBJS += unix-socket.o\n>> +ALL_COMPAT_OBJS += unix-stream-server.o\n>> +ALL_COMPAT_OBJS += sha1dc_git.o\n>\n> I think \"compat\" is a misnomer here. For one thing, they're by\n> definition not \"compat/*\" objects, because they're not in that\n> directory. ;) But more importantly, the interesting thing about them is\n> not that they're compatibility layers, but that they're part of a\n> conditional compilation. I.e., we might or might not want them, which\n> will be determined elsewhere in the Makefile, so they must not be part\n> of the base LIB_OBJS set.\n>\n> Probably CONDITIONAL_OBJS or something might be more descriptive. That\n> _could_ be used to include things like CURL_OBJS, but there's probably\n> value in keeping those in their own list anyway.\n\nGood point, will rename them.\n\n> Likewise, they could go into a conditional-src/ directory (or some\n> less-horrible name) to keep them distinct without needing an explicit\n> list in the Makefile. That's sort of the flip-side of putting all the\n> other LIB_OBJS ones into lib/.\n\nThe goal here was just to get us rid of tiresome merge conflicts when\ntwo things are added to adjacent part of these lists going forward,\nrather than some source-tree reorganization. I didn't search around and\ndidn't find that 2011-era thread.\n\nI think overall just maintaining the list of the few exceptions is\nbetter than any sort of general mass-move of these files.\n\nEven if we carefully trickle those in at a rate that doesn't conflict\nwith anything in-flight, the end result will be that e.g.:\n\n    git log -- lib/grep.c\n\nWill stop at that rename commit, similar to builtin/log.c, unless you\nspecify --follow etc. Just that doesn't make it worth it to me. Likewise\nsha1_file.c to sha1-file.c to object-file.c, which is a case I run into\nevery time I get a \"git log\" pathspec glob wrong.\n\nAlso.\n\nI didn't notice before submitting this but this patch breaks the\nvs-build job, because the cmake build in \"contrib\" is screen-scraping\nthe Makefile[1].\n\nWhat's the status of that code? It's rather tiresome to need to patch\ntwo independent and incompatible build systems every time there's some\nstructural change in the Makefile.\n\nI hadn't looked in any detail at that recipe before, but it the vs-build\njob has a hard dependency on GNU make anyway, since we use it for \"make\nartifacts-tar\".\n\nSo whatever cmake special-sauce is happening there I don't see why\nvs-build couldn't call out \"make\" for most of the work it's doing, isn't\nit just some replacement for what the \"vcxproj\" target in\nconfig.mak.uname used to do?\n\n1. https://github.com/avar/git/runs/4057171803?check_suite_focus=true\n"},{"id":"440255","messageId":"cover-v2-0.3-00000000000-20211101T191231Z-avarab@gmail.com","threadId":"56818","inReplyTo":"patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com","subject":"[PATCH v2 0/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T19:19:01Z","receivedAt":"2021-11-01T19:19:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series replaces the hardcoded lists *.o files in the top-level\nMakefile with a reliance on globbing, e.g. built-ins are $(wildcard\nbuiltin/*.c now, instead of an exhausitive list of every built-in in\nthe tree.\n\nThis makes future development easier as we won't be running into merge\nconflicts with these lists, see 3/3 for some more disucssion.\n\nAs noted in the v1 discussion the v1 had changes that broke the\n\"cmake\" integration. In going back and reading the discussions around\nit it seems we've ended up with a state for that \"contrib\" component\nthat was never the intent, and which we tried to explicitly avoid when\nintegrating it.\n\nThere are more details in the updated commit message, but this v2 gets\nit working with passing CI, and in such a way as to reduce future\nmaintenance burden related to that component.\n\nÆvar Arnfjörð Bjarmason (3):\n  Makefile: rename $(SCRIPT_LIB) to $(SCRIPT_LIB_GEN)\n  Makefile: add a utility to dump variables\n  Makefile: replace most hardcoded object lists with $(wildcard)\n\n Makefile                            | 510 ++++------------------------\n contrib/buildsystems/CMakeLists.txt |  53 ++-\n 2 files changed, 91 insertions(+), 472 deletions(-)\n\nRange-diff against v1:\n-:  ----------- > 1:  0b23b8395ec Makefile: rename $(SCRIPT_LIB) to $(SCRIPT_LIB_GEN)\n-:  ----------- > 2:  97738b056cf Makefile: add a utility to dump variables\n1:  bbacbed5c95 ! 3:  cd62d8f92d1 Makefile: replace most hardcoded object lists with $(wildcard)\n    @@ Commit message\n         would need to be explicitly listed in the Makefile. I think than small\n         trade-off is worth it.\n     \n    +    We could make this simpler still for the Makefile by moving\n    +    \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n    +    for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n    +    need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n    +    e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n    +\n         There's a few small \"while we're at it\" changes here, since I'm\n         touching the code in question:\n     \n    @@ Commit message\n          - De-indent an \"ifndef\" block, we don't usually indent their\n            contents.\n     \n    +    On the CMake changes here:\n    +\n    +     - When CMake support was introduced in was introduced\n    +       061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)\n    +       there was a discussion about the maintenance burden of maintaining the\n    +       top-level Makefile in parallel with CMakeLists.txt[1] where reviewers\n    +       were assured that doing so would simply be a matter of adding something\n    +       to a list in the CMake recipe.\n    +\n    +       Between change and some recent changes of mine where the \"vs-build\"\n    +       job failed to a divergence between the Makefile and CMakeList.txt I\n    +       can confidently say that that doesn't at all match reality. Even\n    +       seemingly trivial changes to the Makefile like this one are forcing\n    +       us to do a deep-dive into CMake internals to make forward progress\n    +       with our main build system.\n    +\n    +     - The promised \"We can add a (continue-on-error) to vs-build job to\n    +       make this process less of a hindrance.\" in [2] never materialized.\n    +       Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n    +       vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n    +       getting the CI to pass goes.\n    +\n    +     - The \"vs-build\" CI doesn't actually require that there be no GNU make\n    +       usage in the job, as it itself has a hard dependency on running a\n    +       \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n    +       goes we don't need a GNU-make free build system for vs-build.\n    +\n    +     - The stated goal in 061c2240b1b of avoiding a GNU make dependency\n    +       for developer because it requires an SDK that \"occupies around two\n    +       gigabytes\" and \"three quarters of a gigabyte worth of Git objects\"\n    +       hardly seems worthwhile trade-off given the above. Disk space is cheap,\n    +       developer time required to maintain two parallel build systems isn't.\n    +\n    +    My attempt to amend/revert 4c2c38e800f to have it use the\n    +    pre-4c2c38e800f \"make\" invocation as a fallback failed, partially\n    +    because I don't have a Windows development environment, so any attempt\n    +    to change it is a painfully slow round-trip to GitHub CI.\n    +\n    +    Let's instead have CMake call out to the Makefile asking it what the\n    +    definition of various variables lists is, rather than being forced to\n    +    maintain those lists in a way that CMake can parse with regexes (which\n    +    precludes anything but a giant hardcoded list).\n    +\n    +    I could familiarize myself enough with CMake to do this in some\n    +    CMake-native way, but that would take \"just as long as adding it to\n    +    the Makefile\"[2] (I think that took me <5 minutes, but I'm several\n    +    hours into fighting with CMake)\n    +\n    +    So I consider this both a bugfix to the stated aims of this CMake\n    +    integration, and a better way forward for having an alternate build\n    +    system. I.e. If someone really does care about a having a\n    +    GNU-make-less dependency for the \"vs-build\" I think this change offers\n    +    a much better way forward for that.\n    +\n    +    Once we invoke the Makefile to spew out e.g. its idea of \"LIB_OBJS\",\n    +    it's going to be trivial to do that via some wrapper script that lives\n    +    in \"contrib/buildsystems\". Such a script would either invoke \"make\n    +    print-{var,list}-%\", or alternatively use an in-tree committed text\n    +    file with the last known result of such a \"make print-{var,list}-%\"\n    +    run.\n    +\n    +    1. https://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com\n    +    2. https://lore.kernel.org/git/CAKiG+9Xtof8Hj3npsS-M0SnT_dcjtHjP_+avWB4oOHkaMdnSbw@mail.gmail.com/\n    +\n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## Makefile ##\n    @@ Makefile: TEST_OBJS =\n     +XDIFF_SRC =\n     +\n     +## Guard against env: objects\n    -+ALL_COMPAT_OBJS =\n    ++CONDITIONAL_OBJS =\n     +CURL_OBJS =\n     +LIB_OBJS_DIRS =\n     +\n    - # Having this variable in your environment would break pipelines because\n    - # you cause \"cd\" to echo its destination to stdout.  It can also take\n    - # scripts to unexpected places.  If you like CDPATH, define it for your\n    + # Utility to dump whatever variables are defined here\n    + print-var-%:\n    + \t@echo $($*)\n     @@ Makefile: X =\n      \n      PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n    @@ Makefile: LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!\n     -LIB_OBJS += commit-reach.o\n     -LIB_OBJS += commit.o\n     +# LIB_OBJS: compat/* objects that live at the top-level\n    -+ALL_COMPAT_OBJS += unix-socket.o\n    -+ALL_COMPAT_OBJS += unix-stream-server.o\n    -+ALL_COMPAT_OBJS += sha1dc_git.o\n    ++CONDITIONAL_OBJS += unix-socket.o\n    ++CONDITIONAL_OBJS += unix-stream-server.o\n    ++CONDITIONAL_OBJS += sha1dc_git.o\n     +\n     +# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions\n     +LIB_OBJS += $(filter-out \\\n    -+\t$(ALL_COMPAT_OBJS) \\\n    ++\t$(CONDITIONAL_OBJS) \\\n     +\tgit.o common-main.o $(PROGRAM_OBJS) \\\n     +\t$(FUZZ_OBJS) $(CURL_OBJS),\\\n     +\t$(patsubst %.c,%.o,$(wildcard *.c)))\n    @@ Makefile: LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!\n     +LIB_OBJS_DIRS += negotiator\n     +LIB_OBJS_DIRS += refs\n     +LIB_OBJS_DIRS += trace2\n    -+LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))\n    ++LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS))))\n     +\n     +# LIB_OBJS: unconditional compat/* objects\n      LIB_OBJS += compat/obstack.o\n    @@ Makefile: LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!\n      \n      # THIRD_PARTY_SOURCES is a list of patterns compatible with the\n      # $(filter) and $(filter-out) family of functions. They specify source\n    +@@ Makefile: endif\n    + LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)\n    + \n    + BASIC_CFLAGS += $(COMPAT_CFLAGS)\n    ++LIB_OBJS_NO_COMPAT := $(LIB_OBJS)\n    + LIB_OBJS += $(COMPAT_OBJS)\n    + \n    + # Quote for C\n     @@ Makefile: reconfigure config.mak.autogen: config.status\n      .PHONY: reconfigure # This is a convenience target.\n      endif\n    @@ Makefile: perf: all\n      \n      t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n      \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n    +\n    + ## contrib/buildsystems/CMakeLists.txt ##\n    +@@ contrib/buildsystems/CMakeLists.txt: project(git\n    + #TODO Add pcre support\n    + \n    + #macros for parsing the Makefile for sources and scripts\n    +-macro(parse_makefile_for_sources list_var regex)\n    +-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n    +-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n    +-\tstring(REPLACE \"$(COMPAT_OBJS)\" \"\" ${list_var} ${${list_var}}) #remove \"$(COMPAT_OBJS)\" This is only for libgit.\n    +-\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n    +-\tstring(REPLACE \".o\" \".c;\" ${list_var} ${${list_var}}) #change .o to .c, ; is for converting the string into a list\n    +-\tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n    +-\tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n    +-endmacro()\n    +-\n    +-macro(parse_makefile_for_scripts list_var regex lang)\n    +-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n    +-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n    +-\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n    +-\tstring(REPLACE \" \" \";\" ${list_var} ${${list_var}}) #convert string to a list\n    +-\tif(NOT ${lang}) #exclude for SCRIPT_LIB\n    +-\t\tlist(TRANSFORM ${list_var} REPLACE \"${lang}\" \"\") #do the replacement\n    +-\tendif()\n    +-endmacro()\n    +-\n    +-macro(parse_makefile_for_executables list_var regex)\n    +-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+= git-(.*)\")\n    +-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n    ++macro(ask_makefile_for_list list_var var_name)\n    ++\texecute_process(COMMAND make print-list-${var_name}\n    ++\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}\n    ++\t\t\tOUTPUT_VARIABLE ${list_var})\n    ++\tstring(REGEX REPLACE \"\\\\.o\\n\" \".c\\n\" ${list_var} ${${list_var}}) #change .o to .c\n    + \tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n    +-\tstring(REPLACE \"git-\" \"\" ${list_var} ${${list_var}}) #strip `git-` prefix\n    +-\tstring(REPLACE \"\\$X\" \";\" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list\n    ++\t## Parse the Makefile print-list-% format\n    ++\tstring(REGEX REPLACE \"${var_name} =\\n\" \"\" ${list_var} ${${list_var}})\n    ++\tstring(REGEX REPLACE \"${var_name} \\\\+?= ([^\\n]+)\" \"\\\\1;\" ${list_var} ${${list_var}})\n    + \tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n    + \tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n    + endmacro()\n    +@@ contrib/buildsystems/CMakeLists.txt: include_directories(${CMAKE_BINARY_DIR})\n    + \n    + #build\n    + #libgit\n    +-parse_makefile_for_sources(libgit_SOURCES \"LIB_OBJS\")\n    ++ask_makefile_for_list(libgit_SOURCES \"LIB_OBJS_NO_COMPAT\")\n    + \n    + list(TRANSFORM libgit_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n    + list(TRANSFORM compat_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n    + add_library(libgit ${libgit_SOURCES} ${compat_SOURCES})\n    + \n    + #libxdiff\n    +-parse_makefile_for_sources(libxdiff_SOURCES \"XDIFF_OBJS\")\n    ++ask_makefile_for_list(libxdiff_SOURCES \"XDIFF_OBJS\")\n    + \n    + list(TRANSFORM libxdiff_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n    + add_library(xdiff STATIC ${libxdiff_SOURCES})\n    +@@ contrib/buildsystems/CMakeLists.txt: elseif(UNIX)\n    + endif()\n    + \n    + #git\n    +-parse_makefile_for_sources(git_SOURCES \"BUILTIN_OBJS\")\n    ++ask_makefile_for_list(git_SOURCES \"BUILTIN_OBJS\")\n    + \n    + list(TRANSFORM git_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n    + add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})\n    +@@ contrib/buildsystems/CMakeLists.txt: if(CURL_FOUND)\n    + \tendif()\n    + endif()\n    + \n    +-parse_makefile_for_executables(git_builtin_extra \"BUILT_INS\")\n    ++ask_makefile_for_list(git_builtin_extra \"BUILT_INS\")\n    ++list(TRANSFORM git_builtin_extra REPLACE \"^git-(.*)\" \"\\\\1\") #strip `git-` prefix\n    ++list(TRANSFORM git_builtin_extra REPLACE \"\\.exe$\" \"\") #strip $X\n    + \n    + option(SKIP_DASHED_BUILT_INS \"Skip hardlinking the dashed versions of the built-ins\")\n    + \n    +@@ contrib/buildsystems/CMakeLists.txt: set(GITWEBDIR ${FALLBACK_RUNTIME_PREFIX}/share/locale)\n    + set(INSTLIBDIR ${FALLBACK_RUNTIME_PREFIX}/share/perl5)\n    + \n    + #shell scripts\n    +-parse_makefile_for_scripts(git_sh_scripts \"SCRIPT_SH\" \".sh\")\n    +-parse_makefile_for_scripts(git_shlib_scripts \"SCRIPT_LIB\" \"\")\n    ++ask_makefile_for_list(git_sh_scripts \"SCRIPT_SH_GEN\")\n    ++ask_makefile_for_list(git_shlib_scripts \"SCRIPT_LIB_GEN\")\n    + set(git_shell_scripts\n    + \t${git_sh_scripts} ${git_shlib_scripts} git-instaweb)\n    + \n    +@@ contrib/buildsystems/CMakeLists.txt: foreach(script ${git_shell_scripts})\n    + endforeach()\n    + \n    + #perl scripts\n    +-parse_makefile_for_scripts(git_perl_scripts \"SCRIPT_PERL\" \".perl\")\n    ++ask_makefile_for_list(git_perl_scripts \"SCRIPT_PERL_GEN\")\n    + \n    + #create perl header\n    + file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )\n    +@@ contrib/buildsystems/CMakeLists.txt: add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c)\n    + target_link_libraries(test-fake-ssh common-main)\n    + \n    + #test-tool\n    +-parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n    ++ask_makefile_for_list(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n    + \n    +-list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/t/helper/\")\n    ++list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n    + add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES})\n    + target_link_libraries(test-tool common-main)\n    + \n-- \n2.33.1.1570.g069344fdd45\n\n"},{"id":"440256","messageId":"patch-v2-1.3-0b23b8395ec-20211101T191231Z-avarab@gmail.com","threadId":"56818","inReplyTo":"cover-v2-0.3-00000000000-20211101T191231Z-avarab@gmail.com","subject":"[PATCH v2 1/3] Makefile: rename $(SCRIPT_LIB) to $(SCRIPT_LIB_GEN)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T19:19:02Z","receivedAt":"2021-11-01T19:19:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make the $(SCRIPT_LIB) variable like the rest of its siblings, where\nwe have an extension-less $(*_GEN) variable, but the main one has the\nfull file name. This will be used in subsequent commits to emit the\nfilenames in $(SCRIPT_LIB), $(SCRIPT_PERL) etc.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 17 +++++++++--------\n 1 file changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 12be39ac497..100658dfa43 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -608,9 +608,9 @@ SCRIPT_SH += git-request-pull.sh\n SCRIPT_SH += git-submodule.sh\n SCRIPT_SH += git-web--browse.sh\n \n-SCRIPT_LIB += git-mergetool--lib\n-SCRIPT_LIB += git-sh-i18n\n-SCRIPT_LIB += git-sh-setup\n+SCRIPT_LIB += git-mergetool--lib.sh\n+SCRIPT_LIB += git-sh-i18n.sh\n+SCRIPT_LIB += git-sh-setup.sh\n \n SCRIPT_PERL += git-add--interactive.perl\n SCRIPT_PERL += git-archimport.perl\n@@ -624,6 +624,7 @@ SCRIPT_PYTHON += git-p4.py\n \n # Generated files for scripts\n SCRIPT_SH_GEN = $(patsubst %.sh,%,$(SCRIPT_SH))\n+SCRIPT_LIB_GEN = $(patsubst %.sh,%,$(SCRIPT_LIB))\n SCRIPT_PERL_GEN = $(patsubst %.perl,%,$(SCRIPT_PERL))\n SCRIPT_PYTHON_GEN = $(patsubst %.py,%,$(SCRIPT_PYTHON))\n \n@@ -2141,7 +2142,7 @@ profile-fast: profile-clean\n \t$(MAKE) PROFILE=USE all\n \n \n-all:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n+all:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB_GEN) $(OTHER_PROGRAMS) GIT-BUILD-OPTIONS\n ifneq (,$X)\n \t$(QUIET_BUILT_IN)$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_COMMANDS_TO_INSTALL) git$X)), test -d '$p' -o '$p' -ef '$p$X' || $(RM) '$p';)\n endif\n@@ -2284,7 +2285,7 @@ $(SCRIPT_SH_GEN) : % : %.sh GIT-SCRIPT-DEFINES\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-$(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES\n+$(SCRIPT_LIB_GEN) : % : %.sh GIT-SCRIPT-DEFINES\n \t$(QUIET_GEN)$(cmd_munge_script) && \\\n \tmv $@+ $@\n \n@@ -3013,7 +3014,7 @@ install: all\n \t$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(INSTALL_STRIP) $(PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(SCRIPTS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n-\t$(INSTALL) -m 644 $(SCRIPT_LIB) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n+\t$(INSTALL) -m 644 $(SCRIPT_LIB_GEN) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'\n \t$(INSTALL) $(INSTALL_STRIP) $(install_bindir_xprograms) '$(DESTDIR_SQ)$(bindir_SQ)'\n \t$(INSTALL) $(BINDIR_PROGRAMS_NO_X) '$(DESTDIR_SQ)$(bindir_SQ)'\n \n@@ -3174,7 +3175,7 @@ ifneq ($(INCLUDE_DLLS_IN_ARTIFACTS),)\n OTHER_PROGRAMS += $(shell echo *.dll t/helper/*.dll)\n endif\n \n-artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB) $(OTHER_PROGRAMS) \\\n+artifacts-tar:: $(ALL_COMMANDS_TO_INSTALL) $(SCRIPT_LIB_GEN) $(OTHER_PROGRAMS) \\\n \t\tGIT-BUILD-OPTIONS $(TEST_PROGRAMS) $(test_bindir_programs) \\\n \t\t$(MOFILES)\n \t$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) \\\n@@ -3232,7 +3233,7 @@ clean: profile-clean coverage-clean cocciclean\n \t$(RM) *.res\n \t$(RM) $(OBJECTS)\n \t$(RM) $(LIB_FILE) $(XDIFF_LIB)\n-\t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X\n+\t$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB_GEN) $(BUILT_INS) git$X\n \t$(RM) $(TEST_PROGRAMS)\n \t$(RM) $(FUZZ_PROGRAMS)\n \t$(RM) $(SP_OBJ)\n-- \n2.33.1.1570.g069344fdd45\n\n"},{"id":"440257","messageId":"patch-v2-2.3-97738b056cf-20211101T191231Z-avarab@gmail.com","threadId":"56818","inReplyTo":"cover-v2-0.3-00000000000-20211101T191231Z-avarab@gmail.com","subject":"[PATCH v2 2/3] Makefile: add a utility to dump variables","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T19:19:03Z","receivedAt":"2021-11-01T19:19:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add handy \"print-var-%\" and \"print-list-%\" targets, these can both be\nused for ad-hoc debugging, and to integrate the Makefile into some\nother build system which needs to extract information from it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Makefile b/Makefile\nindex 100658dfa43..4139bcf675c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -590,6 +590,14 @@ TEST_OBJS =\n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n+# Utility to dump whatever variables are defined here\n+print-var-%:\n+\t@echo $($*)\n+\n+print-list-%:\n+\t@echo $* =\n+\t@for v in $($*); do echo $* += $$v; done\n+\n # Having this variable in your environment would break pipelines because\n # you cause \"cd\" to echo its destination to stdout.  It can also take\n # scripts to unexpected places.  If you like CDPATH, define it for your\n-- \n2.33.1.1570.g069344fdd45\n\n"},{"id":"440258","messageId":"patch-v2-3.3-cd62d8f92d1-20211101T191231Z-avarab@gmail.com","threadId":"56818","inReplyTo":"cover-v2-0.3-00000000000-20211101T191231Z-avarab@gmail.com","subject":"[PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T19:19:04Z","receivedAt":"2021-11-01T19:19:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove the hardcoded lists of objects in favor of using\n$(wildcard). This means that every time a built-in, test tool etc. is\nadded we won't need to patch the top-level Makefile, except for the\nfew remaining cases where the asset in question would make it onto one\nof our list of exceptions.\n\nEver since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\nsubdirectory, 2010-02-22) this has been relatively easy to do (and\neven before that we could glob builtin-*.c). This pattern of\nexhaustively enumerating files was then carried forward for\ne.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\ntest-tool program, 2018-03-24).\n\nOne reason not to do this is that now a new *.c file at the top-level\nwill be immediately picked up, so if a new *.c file is being worked on\n\"make\" will error if it doesn't compile, whereas before that file\nwould need to be explicitly listed in the Makefile. I think than small\ntrade-off is worth it.\n\nWe could make this simpler still for the Makefile by moving\n\"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\nfor $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\nneed the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\ne.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n\nThere's a few small \"while we're at it\" changes here, since I'm\ntouching the code in question:\n\n - Start splitting up the the \"Guard against the environment\" section\n   at the top, but don't move anything that exists there out to avoid\n   merge conflicts\n\n - The $(TEST_BUILTINS_OBJS) variable was needlessly complex, because\n   it didn't have the full paths we'd pathsubst it back & forth.\n\n - Introduce *_SRC in addition to *_OBJ for the variable I'm\n   touching. Eventually we'll want to do this for all the *.o files,\n   i.e. make the *.c list a source of truth for *.o, which means we can\n   e.g. use that exhaustive list for \"make TAGS\".\n\n - Add a missing \"curl-objs\" target. See 029bac01a87 (Makefile: add\n   {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)\n   for the commit that added the rest.\n\n - De-indent an \"ifndef\" block, we don't usually indent their\n   contents.\n\nOn the CMake changes here:\n\n - When CMake support was introduced in was introduced\n   061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)\n   there was a discussion about the maintenance burden of maintaining the\n   top-level Makefile in parallel with CMakeLists.txt[1] where reviewers\n   were assured that doing so would simply be a matter of adding something\n   to a list in the CMake recipe.\n\n   Between change and some recent changes of mine where the \"vs-build\"\n   job failed to a divergence between the Makefile and CMakeList.txt I\n   can confidently say that that doesn't at all match reality. Even\n   seemingly trivial changes to the Makefile like this one are forcing\n   us to do a deep-dive into CMake internals to make forward progress\n   with our main build system.\n\n - The promised \"We can add a (continue-on-error) to vs-build job to\n   make this process less of a hindrance.\" in [2] never materialized.\n   Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n   vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n   getting the CI to pass goes.\n\n - The \"vs-build\" CI doesn't actually require that there be no GNU make\n   usage in the job, as it itself has a hard dependency on running a\n   \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n   goes we don't need a GNU-make free build system for vs-build.\n\n - The stated goal in 061c2240b1b of avoiding a GNU make dependency\n   for developer because it requires an SDK that \"occupies around two\n   gigabytes\" and \"three quarters of a gigabyte worth of Git objects\"\n   hardly seems worthwhile trade-off given the above. Disk space is cheap,\n   developer time required to maintain two parallel build systems isn't.\n\nMy attempt to amend/revert 4c2c38e800f to have it use the\npre-4c2c38e800f \"make\" invocation as a fallback failed, partially\nbecause I don't have a Windows development environment, so any attempt\nto change it is a painfully slow round-trip to GitHub CI.\n\nLet's instead have CMake call out to the Makefile asking it what the\ndefinition of various variables lists is, rather than being forced to\nmaintain those lists in a way that CMake can parse with regexes (which\nprecludes anything but a giant hardcoded list).\n\nI could familiarize myself enough with CMake to do this in some\nCMake-native way, but that would take \"just as long as adding it to\nthe Makefile\"[2] (I think that took me <5 minutes, but I'm several\nhours into fighting with CMake)\n\nSo I consider this both a bugfix to the stated aims of this CMake\nintegration, and a better way forward for having an alternate build\nsystem. I.e. If someone really does care about a having a\nGNU-make-less dependency for the \"vs-build\" I think this change offers\na much better way forward for that.\n\nOnce we invoke the Makefile to spew out e.g. its idea of \"LIB_OBJS\",\nit's going to be trivial to do that via some wrapper script that lives\nin \"contrib/buildsystems\". Such a script would either invoke \"make\nprint-{var,list}-%\", or alternatively use an in-tree committed text\nfile with the last known result of such a \"make print-{var,list}-%\"\nrun.\n\n1. https://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com\n2. https://lore.kernel.org/git/CAKiG+9Xtof8Hj3npsS-M0SnT_dcjtHjP_+avWB4oOHkaMdnSbw@mail.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile                            | 485 ++++------------------------\n contrib/buildsystems/CMakeLists.txt |  53 ++-\n 2 files changed, 74 insertions(+), 464 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 4139bcf675c..5d78ab6860a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -590,6 +590,19 @@ TEST_OBJS =\n TEST_PROGRAMS_NEED_X =\n THIRD_PARTY_SOURCES =\n \n+## Guard against env: programs\n+TEST_PROGRAMS =\n+\n+## Guard against env: sources\n+CURL_SRC =\n+TEST_PROGRAMS_NEED_X_SRC =\n+XDIFF_SRC =\n+\n+## Guard against env: objects\n+CONDITIONAL_OBJS =\n+CURL_OBJS =\n+LIB_OBJS_DIRS =\n+\n # Utility to dump whatever variables are defined here\n print-var-%:\n \t@echo $($*)\n@@ -697,87 +710,23 @@ X =\n \n PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n \n-TEST_BUILTINS_OBJS += test-advise.o\n-TEST_BUILTINS_OBJS += test-bitmap.o\n-TEST_BUILTINS_OBJS += test-bloom.o\n-TEST_BUILTINS_OBJS += test-chmtime.o\n-TEST_BUILTINS_OBJS += test-config.o\n-TEST_BUILTINS_OBJS += test-crontab.o\n-TEST_BUILTINS_OBJS += test-ctype.o\n-TEST_BUILTINS_OBJS += test-date.o\n-TEST_BUILTINS_OBJS += test-delta.o\n-TEST_BUILTINS_OBJS += test-dir-iterator.o\n-TEST_BUILTINS_OBJS += test-drop-caches.o\n-TEST_BUILTINS_OBJS += test-dump-cache-tree.o\n-TEST_BUILTINS_OBJS += test-dump-fsmonitor.o\n-TEST_BUILTINS_OBJS += test-dump-split-index.o\n-TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n-TEST_BUILTINS_OBJS += test-example-decorate.o\n-TEST_BUILTINS_OBJS += test-fast-rebase.o\n-TEST_BUILTINS_OBJS += test-genrandom.o\n-TEST_BUILTINS_OBJS += test-genzeros.o\n-TEST_BUILTINS_OBJS += test-getcwd.o\n-TEST_BUILTINS_OBJS += test-hash-speed.o\n-TEST_BUILTINS_OBJS += test-hash.o\n-TEST_BUILTINS_OBJS += test-hashmap.o\n-TEST_BUILTINS_OBJS += test-index-version.o\n-TEST_BUILTINS_OBJS += test-json-writer.o\n-TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\n-TEST_BUILTINS_OBJS += test-match-trees.o\n-TEST_BUILTINS_OBJS += test-mergesort.o\n-TEST_BUILTINS_OBJS += test-mktemp.o\n-TEST_BUILTINS_OBJS += test-oid-array.o\n-TEST_BUILTINS_OBJS += test-oidmap.o\n-TEST_BUILTINS_OBJS += test-oidtree.o\n-TEST_BUILTINS_OBJS += test-online-cpus.o\n-TEST_BUILTINS_OBJS += test-parse-options.o\n-TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n-TEST_BUILTINS_OBJS += test-partial-clone.o\n-TEST_BUILTINS_OBJS += test-path-utils.o\n-TEST_BUILTINS_OBJS += test-pcre2-config.o\n-TEST_BUILTINS_OBJS += test-pkt-line.o\n-TEST_BUILTINS_OBJS += test-prio-queue.o\n-TEST_BUILTINS_OBJS += test-proc-receive.o\n-TEST_BUILTINS_OBJS += test-progress.o\n-TEST_BUILTINS_OBJS += test-reach.o\n-TEST_BUILTINS_OBJS += test-read-cache.o\n-TEST_BUILTINS_OBJS += test-read-graph.o\n-TEST_BUILTINS_OBJS += test-read-midx.o\n-TEST_BUILTINS_OBJS += test-ref-store.o\n-TEST_BUILTINS_OBJS += test-regex.o\n-TEST_BUILTINS_OBJS += test-repository.o\n-TEST_BUILTINS_OBJS += test-revision-walking.o\n-TEST_BUILTINS_OBJS += test-run-command.o\n-TEST_BUILTINS_OBJS += test-scrap-cache-tree.o\n-TEST_BUILTINS_OBJS += test-serve-v2.o\n-TEST_BUILTINS_OBJS += test-sha1.o\n-TEST_BUILTINS_OBJS += test-sha256.o\n-TEST_BUILTINS_OBJS += test-sigchain.o\n-TEST_BUILTINS_OBJS += test-simple-ipc.o\n-TEST_BUILTINS_OBJS += test-strcmp-offset.o\n-TEST_BUILTINS_OBJS += test-string-list.o\n-TEST_BUILTINS_OBJS += test-submodule-config.o\n-TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n-TEST_BUILTINS_OBJS += test-subprocess.o\n-TEST_BUILTINS_OBJS += test-trace2.o\n-TEST_BUILTINS_OBJS += test-urlmatch-normalization.o\n-TEST_BUILTINS_OBJS += test-userdiff.o\n-TEST_BUILTINS_OBJS += test-wildmatch.o\n-TEST_BUILTINS_OBJS += test-windows-named-pipe.o\n-TEST_BUILTINS_OBJS += test-write-cache.o\n-TEST_BUILTINS_OBJS += test-xml-encode.o\n-\n # Do not add more tests here unless they have extra dependencies. Add\n # them in TEST_BUILTINS_OBJS above.\n TEST_PROGRAMS_NEED_X += test-fake-ssh\n TEST_PROGRAMS_NEED_X += test-tool\n \n-TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))\n+TEST_PROGRAMS_NEED_X_SRC += $(TEST_PROGRAMS_NEED_X:%=t/helper/%.c)\n+TEST_PROGRAMS += $(TEST_PROGRAMS_NEED_X_SRC:%.c=%$X)\n+TEST_BUILTINS_SRC += $(filter-out $(TEST_PROGRAMS_NEED_X_SRC),$(wildcard t/helper/*.c))\n+TEST_BUILTINS_OBJS += $(TEST_BUILTINS_SRC:%.c=%.o)\n \n-# List built-in command $C whose implementation cmd_$C() is not in\n-# builtin/$C.o but is linked in as part of some other command.\n+# List built-in command $C whose implementation cmd_$C() is in\n+# builtin/$C.o\n+BUILTIN_OBJS = $(patsubst %.c,%.o,$(wildcard builtin/*.c))\n BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))\n \n+# List built-in command $C whose implementation cmd_$C() is not in\n+# builtin/$C.o but is linked in as part of some other command.\n BUILT_INS += git-cherry$X\n BUILT_INS += git-cherry-pick$X\n BUILT_INS += git-format-patch$X\n@@ -837,355 +786,28 @@ LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat\n \t-name Documentation -prune -o \\\n \t-name '*.h' -print)))\n \n-LIB_OBJS += abspath.o\n-LIB_OBJS += add-interactive.o\n-LIB_OBJS += add-patch.o\n-LIB_OBJS += advice.o\n-LIB_OBJS += alias.o\n-LIB_OBJS += alloc.o\n-LIB_OBJS += apply.o\n-LIB_OBJS += archive-tar.o\n-LIB_OBJS += archive-zip.o\n-LIB_OBJS += archive.o\n-LIB_OBJS += attr.o\n-LIB_OBJS += base85.o\n-LIB_OBJS += bisect.o\n-LIB_OBJS += blame.o\n-LIB_OBJS += blob.o\n-LIB_OBJS += bloom.o\n-LIB_OBJS += branch.o\n-LIB_OBJS += bulk-checkin.o\n-LIB_OBJS += bundle.o\n-LIB_OBJS += cache-tree.o\n-LIB_OBJS += cbtree.o\n-LIB_OBJS += chdir-notify.o\n-LIB_OBJS += checkout.o\n-LIB_OBJS += chunk-format.o\n-LIB_OBJS += color.o\n-LIB_OBJS += column.o\n-LIB_OBJS += combine-diff.o\n-LIB_OBJS += commit-graph.o\n-LIB_OBJS += commit-reach.o\n-LIB_OBJS += commit.o\n+# LIB_OBJS: compat/* objects that live at the top-level\n+CONDITIONAL_OBJS += unix-socket.o\n+CONDITIONAL_OBJS += unix-stream-server.o\n+CONDITIONAL_OBJS += sha1dc_git.o\n+\n+# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions\n+LIB_OBJS += $(filter-out \\\n+\t$(CONDITIONAL_OBJS) \\\n+\tgit.o common-main.o $(PROGRAM_OBJS) \\\n+\t$(FUZZ_OBJS) $(CURL_OBJS),\\\n+\t$(patsubst %.c,%.o,$(wildcard *.c)))\n+\n+# LIB_OBJS: Directories that contain only LIB_OBJS\n+LIB_OBJS_DIRS += ewah\n+LIB_OBJS_DIRS += negotiator\n+LIB_OBJS_DIRS += refs\n+LIB_OBJS_DIRS += trace2\n+LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS))))\n+\n+# LIB_OBJS: unconditional compat/* objects\n LIB_OBJS += compat/obstack.o\n LIB_OBJS += compat/terminal.o\n-LIB_OBJS += config.o\n-LIB_OBJS += connect.o\n-LIB_OBJS += connected.o\n-LIB_OBJS += convert.o\n-LIB_OBJS += copy.o\n-LIB_OBJS += credential.o\n-LIB_OBJS += csum-file.o\n-LIB_OBJS += ctype.o\n-LIB_OBJS += date.o\n-LIB_OBJS += decorate.o\n-LIB_OBJS += delta-islands.o\n-LIB_OBJS += diff-delta.o\n-LIB_OBJS += diff-merges.o\n-LIB_OBJS += diff-lib.o\n-LIB_OBJS += diff-no-index.o\n-LIB_OBJS += diff.o\n-LIB_OBJS += diffcore-break.o\n-LIB_OBJS += diffcore-delta.o\n-LIB_OBJS += diffcore-order.o\n-LIB_OBJS += diffcore-pickaxe.o\n-LIB_OBJS += diffcore-rename.o\n-LIB_OBJS += diffcore-rotate.o\n-LIB_OBJS += dir-iterator.o\n-LIB_OBJS += dir.o\n-LIB_OBJS += editor.o\n-LIB_OBJS += entry.o\n-LIB_OBJS += environment.o\n-LIB_OBJS += ewah/bitmap.o\n-LIB_OBJS += ewah/ewah_bitmap.o\n-LIB_OBJS += ewah/ewah_io.o\n-LIB_OBJS += ewah/ewah_rlw.o\n-LIB_OBJS += exec-cmd.o\n-LIB_OBJS += fetch-negotiator.o\n-LIB_OBJS += fetch-pack.o\n-LIB_OBJS += fmt-merge-msg.o\n-LIB_OBJS += fsck.o\n-LIB_OBJS += fsmonitor.o\n-LIB_OBJS += gettext.o\n-LIB_OBJS += gpg-interface.o\n-LIB_OBJS += graph.o\n-LIB_OBJS += grep.o\n-LIB_OBJS += hash-lookup.o\n-LIB_OBJS += hashmap.o\n-LIB_OBJS += help.o\n-LIB_OBJS += hex.o\n-LIB_OBJS += hook.o\n-LIB_OBJS += ident.o\n-LIB_OBJS += json-writer.o\n-LIB_OBJS += kwset.o\n-LIB_OBJS += levenshtein.o\n-LIB_OBJS += line-log.o\n-LIB_OBJS += line-range.o\n-LIB_OBJS += linear-assignment.o\n-LIB_OBJS += list-objects-filter-options.o\n-LIB_OBJS += list-objects-filter.o\n-LIB_OBJS += list-objects.o\n-LIB_OBJS += ll-merge.o\n-LIB_OBJS += lockfile.o\n-LIB_OBJS += log-tree.o\n-LIB_OBJS += ls-refs.o\n-LIB_OBJS += mailinfo.o\n-LIB_OBJS += mailmap.o\n-LIB_OBJS += match-trees.o\n-LIB_OBJS += mem-pool.o\n-LIB_OBJS += merge-blobs.o\n-LIB_OBJS += merge-ort.o\n-LIB_OBJS += merge-ort-wrappers.o\n-LIB_OBJS += merge-recursive.o\n-LIB_OBJS += merge.o\n-LIB_OBJS += mergesort.o\n-LIB_OBJS += midx.o\n-LIB_OBJS += name-hash.o\n-LIB_OBJS += negotiator/default.o\n-LIB_OBJS += negotiator/noop.o\n-LIB_OBJS += negotiator/skipping.o\n-LIB_OBJS += notes-cache.o\n-LIB_OBJS += notes-merge.o\n-LIB_OBJS += notes-utils.o\n-LIB_OBJS += notes.o\n-LIB_OBJS += object-file.o\n-LIB_OBJS += object-name.o\n-LIB_OBJS += object.o\n-LIB_OBJS += oid-array.o\n-LIB_OBJS += oidmap.o\n-LIB_OBJS += oidset.o\n-LIB_OBJS += oidtree.o\n-LIB_OBJS += pack-bitmap-write.o\n-LIB_OBJS += pack-bitmap.o\n-LIB_OBJS += pack-check.o\n-LIB_OBJS += pack-objects.o\n-LIB_OBJS += pack-revindex.o\n-LIB_OBJS += pack-write.o\n-LIB_OBJS += packfile.o\n-LIB_OBJS += pager.o\n-LIB_OBJS += parallel-checkout.o\n-LIB_OBJS += parse-options-cb.o\n-LIB_OBJS += parse-options.o\n-LIB_OBJS += patch-delta.o\n-LIB_OBJS += patch-ids.o\n-LIB_OBJS += path.o\n-LIB_OBJS += pathspec.o\n-LIB_OBJS += pkt-line.o\n-LIB_OBJS += preload-index.o\n-LIB_OBJS += pretty.o\n-LIB_OBJS += prio-queue.o\n-LIB_OBJS += progress.o\n-LIB_OBJS += promisor-remote.o\n-LIB_OBJS += prompt.o\n-LIB_OBJS += protocol.o\n-LIB_OBJS += protocol-caps.o\n-LIB_OBJS += prune-packed.o\n-LIB_OBJS += quote.o\n-LIB_OBJS += range-diff.o\n-LIB_OBJS += reachable.o\n-LIB_OBJS += read-cache.o\n-LIB_OBJS += rebase-interactive.o\n-LIB_OBJS += rebase.o\n-LIB_OBJS += ref-filter.o\n-LIB_OBJS += reflog-walk.o\n-LIB_OBJS += refs.o\n-LIB_OBJS += refs/debug.o\n-LIB_OBJS += refs/files-backend.o\n-LIB_OBJS += refs/iterator.o\n-LIB_OBJS += refs/packed-backend.o\n-LIB_OBJS += refs/ref-cache.o\n-LIB_OBJS += refspec.o\n-LIB_OBJS += remote.o\n-LIB_OBJS += replace-object.o\n-LIB_OBJS += repo-settings.o\n-LIB_OBJS += repository.o\n-LIB_OBJS += rerere.o\n-LIB_OBJS += reset.o\n-LIB_OBJS += resolve-undo.o\n-LIB_OBJS += revision.o\n-LIB_OBJS += run-command.o\n-LIB_OBJS += send-pack.o\n-LIB_OBJS += sequencer.o\n-LIB_OBJS += serve.o\n-LIB_OBJS += server-info.o\n-LIB_OBJS += setup.o\n-LIB_OBJS += shallow.o\n-LIB_OBJS += sideband.o\n-LIB_OBJS += sigchain.o\n-LIB_OBJS += sparse-index.o\n-LIB_OBJS += split-index.o\n-LIB_OBJS += stable-qsort.o\n-LIB_OBJS += strbuf.o\n-LIB_OBJS += streaming.o\n-LIB_OBJS += string-list.o\n-LIB_OBJS += strmap.o\n-LIB_OBJS += strvec.o\n-LIB_OBJS += sub-process.o\n-LIB_OBJS += submodule-config.o\n-LIB_OBJS += submodule.o\n-LIB_OBJS += symlinks.o\n-LIB_OBJS += tag.o\n-LIB_OBJS += tempfile.o\n-LIB_OBJS += thread-utils.o\n-LIB_OBJS += tmp-objdir.o\n-LIB_OBJS += trace.o\n-LIB_OBJS += trace2.o\n-LIB_OBJS += trace2/tr2_cfg.o\n-LIB_OBJS += trace2/tr2_cmd_name.o\n-LIB_OBJS += trace2/tr2_dst.o\n-LIB_OBJS += trace2/tr2_sid.o\n-LIB_OBJS += trace2/tr2_sysenv.o\n-LIB_OBJS += trace2/tr2_tbuf.o\n-LIB_OBJS += trace2/tr2_tgt_event.o\n-LIB_OBJS += trace2/tr2_tgt_normal.o\n-LIB_OBJS += trace2/tr2_tgt_perf.o\n-LIB_OBJS += trace2/tr2_tls.o\n-LIB_OBJS += trailer.o\n-LIB_OBJS += transport-helper.o\n-LIB_OBJS += transport.o\n-LIB_OBJS += tree-diff.o\n-LIB_OBJS += tree-walk.o\n-LIB_OBJS += tree.o\n-LIB_OBJS += unpack-trees.o\n-LIB_OBJS += upload-pack.o\n-LIB_OBJS += url.o\n-LIB_OBJS += urlmatch.o\n-LIB_OBJS += usage.o\n-LIB_OBJS += userdiff.o\n-LIB_OBJS += utf8.o\n-LIB_OBJS += varint.o\n-LIB_OBJS += version.o\n-LIB_OBJS += versioncmp.o\n-LIB_OBJS += walker.o\n-LIB_OBJS += wildmatch.o\n-LIB_OBJS += worktree.o\n-LIB_OBJS += wrapper.o\n-LIB_OBJS += write-or-die.o\n-LIB_OBJS += ws.o\n-LIB_OBJS += wt-status.o\n-LIB_OBJS += xdiff-interface.o\n-LIB_OBJS += zlib.o\n-\n-BUILTIN_OBJS += builtin/add.o\n-BUILTIN_OBJS += builtin/am.o\n-BUILTIN_OBJS += builtin/annotate.o\n-BUILTIN_OBJS += builtin/apply.o\n-BUILTIN_OBJS += builtin/archive.o\n-BUILTIN_OBJS += builtin/bisect--helper.o\n-BUILTIN_OBJS += builtin/blame.o\n-BUILTIN_OBJS += builtin/branch.o\n-BUILTIN_OBJS += builtin/bugreport.o\n-BUILTIN_OBJS += builtin/bundle.o\n-BUILTIN_OBJS += builtin/cat-file.o\n-BUILTIN_OBJS += builtin/check-attr.o\n-BUILTIN_OBJS += builtin/check-ignore.o\n-BUILTIN_OBJS += builtin/check-mailmap.o\n-BUILTIN_OBJS += builtin/check-ref-format.o\n-BUILTIN_OBJS += builtin/checkout--worker.o\n-BUILTIN_OBJS += builtin/checkout-index.o\n-BUILTIN_OBJS += builtin/checkout.o\n-BUILTIN_OBJS += builtin/clean.o\n-BUILTIN_OBJS += builtin/clone.o\n-BUILTIN_OBJS += builtin/column.o\n-BUILTIN_OBJS += builtin/commit-graph.o\n-BUILTIN_OBJS += builtin/commit-tree.o\n-BUILTIN_OBJS += builtin/commit.o\n-BUILTIN_OBJS += builtin/config.o\n-BUILTIN_OBJS += builtin/count-objects.o\n-BUILTIN_OBJS += builtin/credential-cache--daemon.o\n-BUILTIN_OBJS += builtin/credential-cache.o\n-BUILTIN_OBJS += builtin/credential-store.o\n-BUILTIN_OBJS += builtin/credential.o\n-BUILTIN_OBJS += builtin/describe.o\n-BUILTIN_OBJS += builtin/diff-files.o\n-BUILTIN_OBJS += builtin/diff-index.o\n-BUILTIN_OBJS += builtin/diff-tree.o\n-BUILTIN_OBJS += builtin/diff.o\n-BUILTIN_OBJS += builtin/difftool.o\n-BUILTIN_OBJS += builtin/env--helper.o\n-BUILTIN_OBJS += builtin/fast-export.o\n-BUILTIN_OBJS += builtin/fast-import.o\n-BUILTIN_OBJS += builtin/fetch-pack.o\n-BUILTIN_OBJS += builtin/fetch.o\n-BUILTIN_OBJS += builtin/fmt-merge-msg.o\n-BUILTIN_OBJS += builtin/for-each-ref.o\n-BUILTIN_OBJS += builtin/for-each-repo.o\n-BUILTIN_OBJS += builtin/fsck.o\n-BUILTIN_OBJS += builtin/gc.o\n-BUILTIN_OBJS += builtin/get-tar-commit-id.o\n-BUILTIN_OBJS += builtin/grep.o\n-BUILTIN_OBJS += builtin/hash-object.o\n-BUILTIN_OBJS += builtin/help.o\n-BUILTIN_OBJS += builtin/index-pack.o\n-BUILTIN_OBJS += builtin/init-db.o\n-BUILTIN_OBJS += builtin/interpret-trailers.o\n-BUILTIN_OBJS += builtin/log.o\n-BUILTIN_OBJS += builtin/ls-files.o\n-BUILTIN_OBJS += builtin/ls-remote.o\n-BUILTIN_OBJS += builtin/ls-tree.o\n-BUILTIN_OBJS += builtin/mailinfo.o\n-BUILTIN_OBJS += builtin/mailsplit.o\n-BUILTIN_OBJS += builtin/merge-base.o\n-BUILTIN_OBJS += builtin/merge-file.o\n-BUILTIN_OBJS += builtin/merge-index.o\n-BUILTIN_OBJS += builtin/merge-ours.o\n-BUILTIN_OBJS += builtin/merge-recursive.o\n-BUILTIN_OBJS += builtin/merge-tree.o\n-BUILTIN_OBJS += builtin/merge.o\n-BUILTIN_OBJS += builtin/mktag.o\n-BUILTIN_OBJS += builtin/mktree.o\n-BUILTIN_OBJS += builtin/multi-pack-index.o\n-BUILTIN_OBJS += builtin/mv.o\n-BUILTIN_OBJS += builtin/name-rev.o\n-BUILTIN_OBJS += builtin/notes.o\n-BUILTIN_OBJS += builtin/pack-objects.o\n-BUILTIN_OBJS += builtin/pack-redundant.o\n-BUILTIN_OBJS += builtin/pack-refs.o\n-BUILTIN_OBJS += builtin/patch-id.o\n-BUILTIN_OBJS += builtin/prune-packed.o\n-BUILTIN_OBJS += builtin/prune.o\n-BUILTIN_OBJS += builtin/pull.o\n-BUILTIN_OBJS += builtin/push.o\n-BUILTIN_OBJS += builtin/range-diff.o\n-BUILTIN_OBJS += builtin/read-tree.o\n-BUILTIN_OBJS += builtin/rebase.o\n-BUILTIN_OBJS += builtin/receive-pack.o\n-BUILTIN_OBJS += builtin/reflog.o\n-BUILTIN_OBJS += builtin/remote-ext.o\n-BUILTIN_OBJS += builtin/remote-fd.o\n-BUILTIN_OBJS += builtin/remote.o\n-BUILTIN_OBJS += builtin/repack.o\n-BUILTIN_OBJS += builtin/replace.o\n-BUILTIN_OBJS += builtin/rerere.o\n-BUILTIN_OBJS += builtin/reset.o\n-BUILTIN_OBJS += builtin/rev-list.o\n-BUILTIN_OBJS += builtin/rev-parse.o\n-BUILTIN_OBJS += builtin/revert.o\n-BUILTIN_OBJS += builtin/rm.o\n-BUILTIN_OBJS += builtin/send-pack.o\n-BUILTIN_OBJS += builtin/shortlog.o\n-BUILTIN_OBJS += builtin/show-branch.o\n-BUILTIN_OBJS += builtin/show-index.o\n-BUILTIN_OBJS += builtin/show-ref.o\n-BUILTIN_OBJS += builtin/sparse-checkout.o\n-BUILTIN_OBJS += builtin/stash.o\n-BUILTIN_OBJS += builtin/stripspace.o\n-BUILTIN_OBJS += builtin/submodule--helper.o\n-BUILTIN_OBJS += builtin/symbolic-ref.o\n-BUILTIN_OBJS += builtin/tag.o\n-BUILTIN_OBJS += builtin/unpack-file.o\n-BUILTIN_OBJS += builtin/unpack-objects.o\n-BUILTIN_OBJS += builtin/update-index.o\n-BUILTIN_OBJS += builtin/update-ref.o\n-BUILTIN_OBJS += builtin/update-server-info.o\n-BUILTIN_OBJS += builtin/upload-archive.o\n-BUILTIN_OBJS += builtin/upload-pack.o\n-BUILTIN_OBJS += builtin/var.o\n-BUILTIN_OBJS += builtin/verify-commit.o\n-BUILTIN_OBJS += builtin/verify-pack.o\n-BUILTIN_OBJS += builtin/verify-tag.o\n-BUILTIN_OBJS += builtin/worktree.o\n-BUILTIN_OBJS += builtin/write-tree.o\n \n # THIRD_PARTY_SOURCES is a list of patterns compatible with the\n # $(filter) and $(filter-out) family of functions. They specify source\n@@ -2076,6 +1698,7 @@ endif\n LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)\n \n BASIC_CFLAGS += $(COMPAT_CFLAGS)\n+LIB_OBJS_NO_COMPAT := $(LIB_OBJS)\n LIB_OBJS += $(COMPAT_OBJS)\n \n # Quote for C\n@@ -2436,17 +2059,12 @@ reconfigure config.mak.autogen: config.status\n .PHONY: reconfigure # This is a convenience target.\n endif\n \n-XDIFF_OBJS += xdiff/xdiffi.o\n-XDIFF_OBJS += xdiff/xemit.o\n-XDIFF_OBJS += xdiff/xhistogram.o\n-XDIFF_OBJS += xdiff/xmerge.o\n-XDIFF_OBJS += xdiff/xpatience.o\n-XDIFF_OBJS += xdiff/xprepare.o\n-XDIFF_OBJS += xdiff/xutils.o\n+XDIFF_SRC += $(wildcard xdiff/*.c)\n+XDIFF_OBJS += $(XDIFF_SRC:.c=.o)\n .PHONY: xdiff-objs\n xdiff-objs: $(XDIFF_OBJS)\n \n-TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(TEST_BUILTINS_OBJS)\n .PHONY: test-objs\n test-objs: $(TEST_OBJS)\n \n@@ -2457,13 +2075,20 @@ GIT_OBJS += git.o\n .PHONY: git-objs\n git-objs: $(GIT_OBJS)\n \n+CURL_SRC += http.c\n+CURL_SRC += http-walker.c\n+CURL_SRC += remote-curl.c\n+CURL_OBJS += $(CURL_SRC:.c=.o)\n+.PHONY: curl-objs\n+curl-objs: $(CURL_OBJS)\n+\n OBJECTS += $(GIT_OBJS)\n OBJECTS += $(PROGRAM_OBJS)\n OBJECTS += $(TEST_OBJS)\n OBJECTS += $(XDIFF_OBJS)\n OBJECTS += $(FUZZ_OBJS)\n ifndef NO_CURL\n-\tOBJECTS += http.o http-walker.o remote-curl.o\n+OBJECTS += $(CURL_OBJS)\n endif\n .PHONY: objects\n objects: $(OBJECTS)\n@@ -2900,7 +2525,7 @@ perf: all\n \n .PRECIOUS: $(TEST_OBJS)\n \n-t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n+t/helper/test-tool$X: $(TEST_BUILTINS_OBJS)\n \n t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex fd1399c440f..d57c9203b66 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -111,32 +111,15 @@ project(git\n #TODO Add pcre support\n \n #macros for parsing the Makefile for sources and scripts\n-macro(parse_makefile_for_sources list_var regex)\n-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n-\tstring(REPLACE \"$(COMPAT_OBJS)\" \"\" ${list_var} ${${list_var}}) #remove \"$(COMPAT_OBJS)\" This is only for libgit.\n-\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n-\tstring(REPLACE \".o\" \".c;\" ${list_var} ${${list_var}}) #change .o to .c, ; is for converting the string into a list\n-\tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n-\tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n-endmacro()\n-\n-macro(parse_makefile_for_scripts list_var regex lang)\n-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n-\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n-\tstring(REPLACE \" \" \";\" ${list_var} ${${list_var}}) #convert string to a list\n-\tif(NOT ${lang}) #exclude for SCRIPT_LIB\n-\t\tlist(TRANSFORM ${list_var} REPLACE \"${lang}\" \"\") #do the replacement\n-\tendif()\n-endmacro()\n-\n-macro(parse_makefile_for_executables list_var regex)\n-\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+= git-(.*)\")\n-\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n+macro(ask_makefile_for_list list_var var_name)\n+\texecute_process(COMMAND make print-list-${var_name}\n+\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}\n+\t\t\tOUTPUT_VARIABLE ${list_var})\n+\tstring(REGEX REPLACE \"\\\\.o\\n\" \".c\\n\" ${list_var} ${${list_var}}) #change .o to .c\n \tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n-\tstring(REPLACE \"git-\" \"\" ${list_var} ${${list_var}}) #strip `git-` prefix\n-\tstring(REPLACE \"\\$X\" \";\" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list\n+\t## Parse the Makefile print-list-% format\n+\tstring(REGEX REPLACE \"${var_name} =\\n\" \"\" ${list_var} ${${list_var}})\n+\tstring(REGEX REPLACE \"${var_name} \\\\+?= ([^\\n]+)\" \"\\\\1;\" ${list_var} ${${list_var}})\n \tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n \tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n endmacro()\n@@ -635,14 +618,14 @@ include_directories(${CMAKE_BINARY_DIR})\n \n #build\n #libgit\n-parse_makefile_for_sources(libgit_SOURCES \"LIB_OBJS\")\n+ask_makefile_for_list(libgit_SOURCES \"LIB_OBJS_NO_COMPAT\")\n \n list(TRANSFORM libgit_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n list(TRANSFORM compat_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n add_library(libgit ${libgit_SOURCES} ${compat_SOURCES})\n \n #libxdiff\n-parse_makefile_for_sources(libxdiff_SOURCES \"XDIFF_OBJS\")\n+ask_makefile_for_list(libxdiff_SOURCES \"XDIFF_OBJS\")\n \n list(TRANSFORM libxdiff_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n add_library(xdiff STATIC ${libxdiff_SOURCES})\n@@ -693,7 +676,7 @@ elseif(UNIX)\n endif()\n \n #git\n-parse_makefile_for_sources(git_SOURCES \"BUILTIN_OBJS\")\n+ask_makefile_for_list(git_SOURCES \"BUILTIN_OBJS\")\n \n list(TRANSFORM git_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})\n@@ -729,7 +712,9 @@ if(CURL_FOUND)\n \tendif()\n endif()\n \n-parse_makefile_for_executables(git_builtin_extra \"BUILT_INS\")\n+ask_makefile_for_list(git_builtin_extra \"BUILT_INS\")\n+list(TRANSFORM git_builtin_extra REPLACE \"^git-(.*)\" \"\\\\1\") #strip `git-` prefix\n+list(TRANSFORM git_builtin_extra REPLACE \"\\.exe$\" \"\") #strip $X\n \n option(SKIP_DASHED_BUILT_INS \"Skip hardlinking the dashed versions of the built-ins\")\n \n@@ -766,8 +751,8 @@ set(GITWEBDIR ${FALLBACK_RUNTIME_PREFIX}/share/locale)\n set(INSTLIBDIR ${FALLBACK_RUNTIME_PREFIX}/share/perl5)\n \n #shell scripts\n-parse_makefile_for_scripts(git_sh_scripts \"SCRIPT_SH\" \".sh\")\n-parse_makefile_for_scripts(git_shlib_scripts \"SCRIPT_LIB\" \"\")\n+ask_makefile_for_list(git_sh_scripts \"SCRIPT_SH_GEN\")\n+ask_makefile_for_list(git_shlib_scripts \"SCRIPT_LIB_GEN\")\n set(git_shell_scripts\n \t${git_sh_scripts} ${git_shlib_scripts} git-instaweb)\n \n@@ -787,7 +772,7 @@ foreach(script ${git_shell_scripts})\n endforeach()\n \n #perl scripts\n-parse_makefile_for_scripts(git_perl_scripts \"SCRIPT_PERL\" \".perl\")\n+ask_makefile_for_list(git_perl_scripts \"SCRIPT_PERL_GEN\")\n \n #create perl header\n file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )\n@@ -910,9 +895,9 @@ add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c)\n target_link_libraries(test-fake-ssh common-main)\n \n #test-tool\n-parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n+ask_makefile_for_list(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n \n-list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/t/helper/\")\n+list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES})\n target_link_libraries(test-tool common-main)\n \n-- \n2.33.1.1570.g069344fdd45\n\n"},{"id":"440266","messageId":"211101.867ddr3ak5.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"f5807fd2eb0c16ab0400e8f3138a90f564fd303c.camel@mad-scientist.net","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-01T20:06:24Z","receivedAt":"2021-11-01T20:06:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Oct 30 2021, Paul Smith wrote:\n\n> On Sun, 2021-10-31 at 00:32 +0200, Ævar Arnfjörð Bjarmason wrote:\n>> +LIB_OBJS += $(patsubst %.c,%.o,$(foreach dir,$(LIB_OBJS_DIRS),$(wildcard $(dir)/*.c)))\n>\n> Another way to write this would be:\n>\n>    LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS)))\n>\n> I don't know that there's any reason to choose one over the other.  I\n> don't think there's any real performance difference although one could\n> imagine this version to be VERY SLIGHTLY faster.  Also this one is a\n> little more \"Lisp-ish\"... that might be a pro or a con depending :).\n>\n> Just kibitzing while waiting for dinner to arrive...\n\nThanks. I changed it to use that in the v2.\n"},{"id":"440367","messageId":"YYJy4BuX6JI6p+aV@coredump.intra.peff.net","threadId":"56818","inReplyTo":"211031.86a6ip47ib.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-03T11:30:40Z","receivedAt":"2021-11-03T11:30:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > Likewise, they could go into a conditional-src/ directory (or some\n> > less-horrible name) to keep them distinct without needing an explicit\n> > list in the Makefile. That's sort of the flip-side of putting all the\n> > other LIB_OBJS ones into lib/.\n> \n> The goal here was just to get us rid of tiresome merge conflicts when\n> two things are added to adjacent part of these lists going forward,\n> rather than some source-tree reorganization. I didn't search around and\n> didn't find that 2011-era thread.\n\nRight, sorry, I didn't mean to sidetrack. That was just the only\ndiscussion I could find on the topic. I agree that it's worth\nconsidering the two topics (moving files around vs Makefile changes)\nseparately.\n\n> Even if we carefully trickle those in at a rate that doesn't conflict\n> with anything in-flight, the end result will be that e.g.:\n> \n>     git log -- lib/grep.c\n> \n> Will stop at that rename commit, similar to builtin/log.c, unless you\n> specify --follow etc. Just that doesn't make it worth it to me. Likewise\n> sha1_file.c to sha1-file.c to object-file.c, which is a case I run into\n> every time I get a \"git log\" pathspec glob wrong.\n\nAgreed. One of the arguments back when we started to move around a few\nfiles is that this dog-fooding would encourage us to make --follow mode\nbetter. That hasn't really happened, though.\n\n> I didn't notice before submitting this but this patch breaks the\n> vs-build job, because the cmake build in \"contrib\" is screen-scraping\n> the Makefile[1].\n> \n> What's the status of that code? It's rather tiresome to need to patch\n> two independent and incompatible build systems every time there's some\n> structural change in the Makefile.\n\nMy opinion when we took in the cmake topic was that it would be OK for\npeople working on the main Makefile to break cmake. It's an add-on and\nthe people who care about cmake are the ones who will do the work to\ntrack the Makefile.\n\nBut since there's a CI job that will nag you if it fails, that kind of\nmakes it everybody's problem in practice. That doesn't change my opinion\non how things _should_ work, but I have done small fixups as necessary\nto stop the nagging.\n\n> I hadn't looked in any detail at that recipe before, but it the vs-build\n> job has a hard dependency on GNU make anyway, since we use it for \"make\n> artifacts-tar\".\n> \n> So whatever cmake special-sauce is happening there I don't see why\n> vs-build couldn't call out \"make\" for most of the work it's doing, isn't\n> it just some replacement for what the \"vcxproj\" target in\n> config.mak.uname used to do?\n\nThe big question for me is whether that really is a hard dependency.\nObviously \"make artifacts-tar\" is for the CI job, but is the cmake stuff\nsupposed to work for regular users without relying on having GNU make at\nall? I have no clue.\n\n-Peff\n"},{"id":"440376","messageId":"211103.868ry5jn36.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"YYJy4BuX6JI6p+aV@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-03T14:57:35Z","receivedAt":"2021-11-03T15:06:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 03 2021, Jeff King wrote:\n\n> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> [...]\n>> I didn't notice before submitting this but this patch breaks the\n>> vs-build job, because the cmake build in \"contrib\" is screen-scraping\n>> the Makefile[1].\n>> \n>> What's the status of that code? It's rather tiresome to need to patch\n>> two independent and incompatible build systems every time there's some\n>> structural change in the Makefile.\n>\n> My opinion when we took in the cmake topic was that it would be OK for\n> people working on the main Makefile to break cmake. It's an add-on and\n> the people who care about cmake are the ones who will do the work to\n> track the Makefile.\n>\n> But since there's a CI job that will nag you if it fails, that kind of\n> makes it everybody's problem in practice. That doesn't change my opinion\n> on how things _should_ work, but I have done small fixups as necessary\n> to stop the nagging.\n\nYes, that was clearly the intent from reading the original discussion,\nbut we've crept towards it being an actual hard dependency. I'd also be\nfine with some direction that just removed that vs-build/vs-test job to\nsomething optional...\n\n>> I hadn't looked in any detail at that recipe before, but it the vs-build\n>> job has a hard dependency on GNU make anyway, since we use it for \"make\n>> artifacts-tar\".\n>> \n>> So whatever cmake special-sauce is happening there I don't see why\n>> vs-build couldn't call out \"make\" for most of the work it's doing, isn't\n>> it just some replacement for what the \"vcxproj\" target in\n>> config.mak.uname used to do?\n>\n> The big question for me is whether that really is a hard dependency.\n> Obviously \"make artifacts-tar\" is for the CI job, but is the cmake stuff\n> supposed to work for regular users without relying on having GNU make at\n> all? I have no clue.\n\nIt's a hard dependency for the job, since it tars up its built assets in\nthe first step, and those are then unpacked and used in subsequent\nsteps. It's being used to ferry the built binaries between CI phases.\n\nBut yes, the intent was clearly to not have a dependency on GNU make,\nbut as I argue in\n<patch-v2-3.3-cd62d8f92d1-20211101T191231Z-avarab@gmail.com> I think\nhaving those developers simply install it is better than forcing us to\nmaintain two distinct and incompatible build systems.\n\nThe selling point was that it was going to be really easy to maintain\nthem in parallel, i.e. you'd just add a thing to a list here and a list\nthere, but that assumes that nothing will ever structurally change in\nthe Makefile.\n\nI think the other X-Y problem being solved there was that cmake has some\nbetter integration for Visual Studio somehow. I.e. it does what the\n\"vcxproj\" target in config.mak.uname does/did.\n\nI think that would be a fine use for cmake, and we can clearly\naccomplish that by having our cmake file effectively be a mostly thin\nwrapper for logic that lives in the Makefile.\n\nI.e. it would ask the Makefile what's in this list or other, what the\nCFLAGS are etc. etc., and feed that into relevant cmake variables.\n\nMy patch starts us moving in that direction (but doesn't get anywhere\nclose to that end-goal). I think if we did that the ~1k line\nCMakeLists.txt would be maybe 100-300 lines.\n"},{"id":"440417","messageId":"nycvar.QRO.7.76.6.2111040124430.56@tvgsbejvaqbjf.bet","threadId":"56818","inReplyTo":"YYJy4BuX6JI6p+aV@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-04T00:31:55Z","receivedAt":"2021-11-04T00:32:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 3 Nov 2021, Jeff King wrote:\n\n> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n> > I didn't notice before submitting this but this patch breaks the\n> > vs-build job, because the cmake build in \"contrib\" is screen-scraping\n> > the Makefile[1].\n> >\n> > What's the status of that code? It's rather tiresome to need to patch\n> > two independent and incompatible build systems every time there's some\n> > structural change in the Makefile.\n>\n> My opinion when we took in the cmake topic was that it would be OK for\n> people working on the main Makefile to break cmake. It's an add-on and\n> the people who care about cmake are the ones who will do the work to\n> track the Makefile.\n\nI do try to have a look at breakages in `seen` when I have the time, but\nlately I didn't. That's why you may have felt more of these CMake\nheadaches.\n\n> But since there's a CI job that will nag you if it fails, that kind of\n> makes it everybody's problem in practice. That doesn't change my opinion\n> on how things _should_ work, but I have done small fixups as necessary\n> to stop the nagging.\n\nOne very simple solution is to leave the Makefile alone unless it really,\nreally needs to be changed. There are costs to refactoring, and quite\nhonestly, it might be a good thing that something like a failing vs-build\njob discourages refactoring for refactoring's sake.\n\n> > I hadn't looked in any detail at that recipe before, but it the\n> > vs-build job has a hard dependency on GNU make anyway, since we use it\n> > for \"make artifacts-tar\".\n> >\n> > So whatever cmake special-sauce is happening there I don't see why\n> > vs-build couldn't call out \"make\" for most of the work it's doing,\n> > isn't it just some replacement for what the \"vcxproj\" target in\n> > config.mak.uname used to do?\n>\n> The big question for me is whether that really is a hard dependency.\n> Obviously \"make artifacts-tar\" is for the CI job, but is the cmake stuff\n> supposed to work for regular users without relying on having GNU make at\n> all? I have no clue.\n\nThe entire point of the CMake configuration is to allow developers on\nWindows to use the tools they are used to, to build Git. And believe it or\nnot, GNU make is not one of those tools! I know. Very hard to believe. :-)\n\nSo yeah, the vs-build/vs-test combo tries to pay attention to this\nintended scenario, and avoids using GNU make or any other of those Unix\ntools that much less ubiquitous than some might want to believe.\n\nCiao,\nDscho\n"},{"id":"440442","messageId":"211104.86r1bwi6f7.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"nycvar.QRO.7.76.6.2111040124430.56@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-04T09:46:28Z","receivedAt":"2021-11-04T10:03:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 04 2021, Johannes Schindelin wrote:\n\n> Hi Peff,\n>\n> On Wed, 3 Nov 2021, Jeff King wrote:\n>\n>> On Sun, Oct 31, 2021 at 02:00:42PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> > I didn't notice before submitting this but this patch breaks the\n>> > vs-build job, because the cmake build in \"contrib\" is screen-scraping\n>> > the Makefile[1].\n>> >\n>> > What's the status of that code? It's rather tiresome to need to patch\n>> > two independent and incompatible build systems every time there's some\n>> > structural change in the Makefile.\n>>\n>> My opinion when we took in the cmake topic was that it would be OK for\n>> people working on the main Makefile to break cmake. It's an add-on and\n>> the people who care about cmake are the ones who will do the work to\n>> track the Makefile.\n>\n> I do try to have a look at breakages in `seen` when I have the time, but\n> lately I didn't. That's why you may have felt more of these CMake\n> headaches.\n\nIt's not only things that make it into \"seen\", as most will test their\ntopic in GitHub CI before submission in their own repos.\n\n>> But since there's a CI job that will nag you if it fails, that kind of\n>> makes it everybody's problem in practice. That doesn't change my opinion\n>> on how things _should_ work, but I have done small fixups as necessary\n>> to stop the nagging.\n>\n> One very simple solution is to leave the Makefile alone unless it really,\n> really needs to be changed. There are costs to refactoring, and quite\n> honestly, it might be a good thing that something like a failing vs-build\n> job discourages refactoring for refactoring's sake.\n\nSure, but that's the case with any critical component we're using. A\nquestion of \"is it worth leaving it alone\" is distinct from \"is it\npainful to touch it because you need to implement a fix twice in two\nincompatible languages?\".\n\nIn this case I do think the change is justified. I've personally got a\nfew local topics that I keep having to (even with rerere) solve\nconflicts for due to this list of files, and Junio deals with the same.\n\nDitto for some of the changes I've made recently to make things\nnon-.PHONY. That's resulted in major workflow improvements for me,\n\nBut in any case, the selling point of the original cmake integration was\nnot something to the effect of:\n\n    \"nobody should have to change this in anything but ever so this\n    re-implementation is a one-off\"\n\nBut rather something like:\n\n    \"This re-implementation is a one-off, but any updates to both should\n    be trivial.\"\n\nAs someone who's had a couple of recent run-ins with cmake I can tell\nyou it's really not trivial at all.\n\nSo given that the selling point of the original change didn't turn out\nas was expected I think it's fair to re-visit whether we'd like to take\nthis path going forward, or to choose another trade-off.\n\n>> > I hadn't looked in any detail at that recipe before, but it the\n>> > vs-build job has a hard dependency on GNU make anyway, since we use it\n>> > for \"make artifacts-tar\".\n>> >\n>> > So whatever cmake special-sauce is happening there I don't see why\n>> > vs-build couldn't call out \"make\" for most of the work it's doing,\n>> > isn't it just some replacement for what the \"vcxproj\" target in\n>> > config.mak.uname used to do?\n>>\n>> The big question for me is whether that really is a hard dependency.\n>> Obviously \"make artifacts-tar\" is for the CI job, but is the cmake stuff\n>> supposed to work for regular users without relying on having GNU make at\n>> all? I have no clue.\n>\n> The entire point of the CMake configuration is to allow developers on\n> Windows to use the tools they are used to, to build Git. And believe it or\n> not, GNU make is not one of those tools! I know. Very hard to believe. :-)\n\nI believe that, the question is why it isn't a better trade-off to just\nask those users to install that software. Our Windows CI is doing it\non-the-fly, so clearly it's not that hard to do it.\n\nNote that I'm not saying that whatever integration those users get in VS\nfrom the special-cause CMake integration should change. We're only\ntalking about it invoking \"make\" under the hood in a way that'll be\ninvisible to the user.\n\nPOSIX \"sh\" isn't native to Windows either, and that CMake file invokes\nshellscripts we ship to e.g. build the generated headers, so this\nworkflow is clearly something that's OK for an end-user once the one-off\nhassle of installing a package is over with.\n"},{"id":"440456","messageId":"45402a12-e08a-bb27-09cf-db1c64c76bba@iee.email","threadId":"56818","inReplyTo":"211104.86r1bwi6f7.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-11-04T14:29:39Z","receivedAt":"2021-11-04T14:29:44Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 04/11/2021 09:46, Ævar Arnfjörð Bjarmason wrote:\n>> he entire point of the CMake configuration is to allow developers on\n>> Windows to use the tools they are used to, to build Git. And believe it or\n>> not, GNU make is not one of those tools! I know. Very hard to believe. :-)\n> I believe that, the question is why it isn't a better trade-off to just\n> ask those users to install that software. Our Windows CI is doing it\n> on-the-fly, so clearly it's not that hard to do it.\nJust to say that, while it is real easy to download and install the\nGit-for-Windows SDK (https://gitforwindows.org/#download-sdk), for most\n(Windows) users it's a foreign land, with few friends who understand\nwhat things like `gdb` are all about. It's all doable, but the learning\ncurve can be hard. The CI doesn't need a learning curve ;-)\n\nBeing able to fire up a well 'trusted' tool like Visual Studio to\ninvestigate the code does help contributors understand the code.\n\n--\nPhilip\n"},{"id":"440468","messageId":"xmqqtugr3l4r.fsf@gitster.g","threadId":"56818","inReplyTo":"211104.86r1bwi6f7.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-04T17:07:16Z","receivedAt":"2021-11-04T17:07:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> In this case I do think the change is justified. I've personally got a\n> few local topics that I keep having to (even with rerere) solve\n> conflicts for due to this list of files, and Junio deals with the same.\n>\n> Ditto for some of the changes I've made recently to make things\n> non-.PHONY. That's resulted in major workflow improvements for me,\n\nTo me, I haven't noticed any workflow improvements for me, so I do\nnot see how my name got into the sentence.\n\n> But in any case, the selling point of the original cmake integration was\n> not something to the effect of:\n>\n>     \"nobody should have to change this in anything but ever so this\n>     re-implementation is a one-off\"\n\nI agree that this wasn't how it was sold, but ...\n\n> But rather something like:\n>\n>     \"This re-implementation is a one-off, but any updates to both should\n>     be trivial.\"\n\n... I do not think this was how it was sold, either.  As far as I\nrecall, it was rather: this may double the maintenance burden, but\nthe reward to help casual Windows builders is large enough that\nthose who want to add the CMake support are willing to bear their\nshare of the burden.\n\n> As someone who's had a couple of recent run-ins with cmake I can tell\n> you it's really not trivial at all.\n\nSurely.\n\n> So given that the selling point of the original change didn't turn out\n> as was expected I think it's fair to re-visit whether we'd like to take\n> this path going forward, or to choose another trade-off.\n\nOK.  The rest of the message I am responding to is your revisiting,\nI guess.\n\n>> The entire point of the CMake configuration is to allow developers on\n>> Windows to use the tools they are used to, to build Git. And believe it or\n>> not, GNU make is not one of those tools! I know. Very hard to believe. :-)\n>\n> I believe that, the question is why it isn't a better trade-off to just\n> ask those users to install that software. Our Windows CI is doing it\n> on-the-fly, so clearly it's not that hard to do it.\n>\n> Note that I'm not saying that whatever integration those users get in VS\n> from the special-cause CMake integration should change. We're only\n> talking about it invoking \"make\" under the hood in a way that'll be\n> invisible to the user.\n>\n> POSIX \"sh\" isn't native to Windows either, and that CMake file invokes\n> shellscripts we ship to e.g. build the generated headers, so this\n> workflow is clearly something that's OK for an end-user once the one-off\n> hassle of installing a package is over with.\n"},{"id":"440570","messageId":"24482f96-7d87-1570-a171-95ec182f6091@gmail.com","threadId":"56818","inReplyTo":"patch-v2-3.3-cd62d8f92d1-20211101T191231Z-avarab@gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-11-06T10:57:17Z","receivedAt":"2021-11-06T10:57:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nOn 01/11/2021 19:19, Ævar Arnfjörð Bjarmason wrote:\n> Remove the hardcoded lists of objects in favor of using\n> $(wildcard). This means that every time a built-in, test tool etc. is\n> added we won't need to patch the top-level Makefile, except for the\n> few remaining cases where the asset in question would make it onto one\n> of our list of exceptions.\n> \n> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\n> subdirectory, 2010-02-22) this has been relatively easy to do (and\n> even before that we could glob builtin-*.c). This pattern of\n> exhaustively enumerating files was then carried forward for\n> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\n> test-tool program, 2018-03-24).\n> \n> One reason not to do this is that now a new *.c file at the top-level\n> will be immediately picked up, so if a new *.c file is being worked on\n> \"make\" will error if it doesn't compile, whereas before that file\n> would need to be explicitly listed in the Makefile. I think than small\n> trade-off is worth it.\n\nIf I need to split up some uncommitted changes into several commits and \nI know it is going to be fiddly to do so I will sometimes copy the \noriginal file to foo.safe.c and then edit foo.c to create each commit. \nThen I can easily compile and test each commit and editing the file \ndirectly is often easier than using add -p and editing the hunks. With \nthis patch running make will fail in that case I think.\n\n> We could make this simpler still for the Makefile by moving\n> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n> \n> There's a few small \"while we're at it\" changes here, since I'm\n> touching the code in question:\n> \n>   - Start splitting up the the \"Guard against the environment\" section\n>     at the top, but don't move anything that exists there out to avoid\n>     merge conflicts\n> \n>   - The $(TEST_BUILTINS_OBJS) variable was needlessly complex, because\n>     it didn't have the full paths we'd pathsubst it back & forth.\n> \n>   - Introduce *_SRC in addition to *_OBJ for the variable I'm\n>     touching. Eventually we'll want to do this for all the *.o files,\n>     i.e. make the *.c list a source of truth for *.o, which means we can\n>     e.g. use that exhaustive list for \"make TAGS\".\n> \n>   - Add a missing \"curl-objs\" target. See 029bac01a87 (Makefile: add\n>     {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)\n>     for the commit that added the rest.\n> \n>   - De-indent an \"ifndef\" block, we don't usually indent their\n>     contents.\n> \n> On the CMake changes here:\n> \n>   - When CMake support was introduced in was introduced\n>     061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)\n>     there was a discussion about the maintenance burden of maintaining the\n>     top-level Makefile in parallel with CMakeLists.txt[1] where reviewers\n>     were assured that doing so would simply be a matter of adding something\n>     to a list in the CMake recipe.\n> \n>     Between change and some recent changes of mine where the \"vs-build\"\n>     job failed to a divergence between the Makefile and CMakeList.txt I\n>     can confidently say that that doesn't at all match reality. Even\n>     seemingly trivial changes to the Makefile like this one are forcing\n>     us to do a deep-dive into CMake internals to make forward progress\n>     with our main build system.\n\nMy recollection is that the discussions were about not having to touch \nCMakeList.txt when adding new files to the build and I think that \nlargely works. I don't think a lot of the changes you have been making \nrecently were anticipated in that discussion.\n\n>   - The promised \"We can add a (continue-on-error) to vs-build job to\n>     make this process less of a hindrance.\" in [2] never materialized.\n>     Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n>     vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n>     getting the CI to pass goes.\n> \n>   - The \"vs-build\" CI doesn't actually require that there be no GNU make\n>     usage in the job, as it itself has a hard dependency on running a\n>     \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n>     goes we don't need a GNU-make free build system for vs-build.\n\nWe need GNU-make for the ci job but an individual developer using CMake \ndoes not need GNU-make installed. On linux it is possible to build git \nwithout having make installed by using cmake and ninja [1]\n\n>   - The stated goal in 061c2240b1b of avoiding a GNU make dependency\n>     for developer because it requires an SDK that \"occupies around two\n>     gigabytes\" and \"three quarters of a gigabyte worth of Git objects\"\n>     hardly seems worthwhile trade-off given the above. Disk space is cheap,\n>     developer time required to maintain two parallel build systems isn't.\n\nThat rather assumes everyone has plenty of disk space and a decent \nnetwork connection.\n\n> My attempt to amend/revert 4c2c38e800f to have it use the\n> pre-4c2c38e800f \"make\" invocation as a fallback failed, partially\n> because I don't have a Windows development environment, so any attempt\n> to change it is a painfully slow round-trip to GitHub CI.\n> \n> Let's instead have CMake call out to the Makefile asking it what the\n> definition of various variables lists is, rather than being forced to\n> maintain those lists in a way that CMake can parse with regexes (which\n> precludes anything but a giant hardcoded list).\n> \n> I could familiarize myself enough with CMake to do this in some\n> CMake-native way, but that would take \"just as long as adding it to\n> the Makefile\"[2] (I think that took me <5 minutes, but I'm several\n> hours into fighting with CMake)\n> \n> So I consider this both a bugfix to the stated aims of this CMake\n> integration, and a better way forward for having an alternate build\n> system. I.e. If someone really does care about a having a\n> GNU-make-less dependency for the \"vs-build\" I think this change offers\n> a much better way forward for that.\n\nI don't see how relying on GNU-make is a step forward for the CMake \nintegration when it works without it now.\n\nOverall I'm don't think that moving from a known set of dependencies to \n\"build whatever C files are lying around in this directory\" is an \nimprovement.\n\nBest Wishes\n\nPhillip\n\n[1] The CMake integration is currently broken for non-windows builds, \nI've got some fixes at \nhttps://github.com/phillipwood/git/tree/wip/cmake-fixes\n\n> Once we invoke the Makefile to spew out e.g. its idea of \"LIB_OBJS\",\n> it's going to be trivial to do that via some wrapper script that lives\n> in \"contrib/buildsystems\". Such a script would either invoke \"make\n> print-{var,list}-%\", or alternatively use an in-tree committed text\n> file with the last known result of such a \"make print-{var,list}-%\"\n> run.\n> \n> 1. https://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com\n> 2. https://lore.kernel.org/git/CAKiG+9Xtof8Hj3npsS-M0SnT_dcjtHjP_+avWB4oOHkaMdnSbw@mail.gmail.com/\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>   Makefile                            | 485 ++++------------------------\n>   contrib/buildsystems/CMakeLists.txt |  53 ++-\n>   2 files changed, 74 insertions(+), 464 deletions(-)\n> \n> diff --git a/Makefile b/Makefile\n> index 4139bcf675c..5d78ab6860a 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -590,6 +590,19 @@ TEST_OBJS =\n>   TEST_PROGRAMS_NEED_X =\n>   THIRD_PARTY_SOURCES =\n>   \n> +## Guard against env: programs\n> +TEST_PROGRAMS =\n> +\n> +## Guard against env: sources\n> +CURL_SRC =\n> +TEST_PROGRAMS_NEED_X_SRC =\n> +XDIFF_SRC =\n> +\n> +## Guard against env: objects\n> +CONDITIONAL_OBJS =\n> +CURL_OBJS =\n> +LIB_OBJS_DIRS =\n> +\n>   # Utility to dump whatever variables are defined here\n>   print-var-%:\n>   \t@echo $($*)\n> @@ -697,87 +710,23 @@ X =\n>   \n>   PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))\n>   \n> -TEST_BUILTINS_OBJS += test-advise.o\n> -TEST_BUILTINS_OBJS += test-bitmap.o\n> -TEST_BUILTINS_OBJS += test-bloom.o\n> -TEST_BUILTINS_OBJS += test-chmtime.o\n> -TEST_BUILTINS_OBJS += test-config.o\n> -TEST_BUILTINS_OBJS += test-crontab.o\n> -TEST_BUILTINS_OBJS += test-ctype.o\n> -TEST_BUILTINS_OBJS += test-date.o\n> -TEST_BUILTINS_OBJS += test-delta.o\n> -TEST_BUILTINS_OBJS += test-dir-iterator.o\n> -TEST_BUILTINS_OBJS += test-drop-caches.o\n> -TEST_BUILTINS_OBJS += test-dump-cache-tree.o\n> -TEST_BUILTINS_OBJS += test-dump-fsmonitor.o\n> -TEST_BUILTINS_OBJS += test-dump-split-index.o\n> -TEST_BUILTINS_OBJS += test-dump-untracked-cache.o\n> -TEST_BUILTINS_OBJS += test-example-decorate.o\n> -TEST_BUILTINS_OBJS += test-fast-rebase.o\n> -TEST_BUILTINS_OBJS += test-genrandom.o\n> -TEST_BUILTINS_OBJS += test-genzeros.o\n> -TEST_BUILTINS_OBJS += test-getcwd.o\n> -TEST_BUILTINS_OBJS += test-hash-speed.o\n> -TEST_BUILTINS_OBJS += test-hash.o\n> -TEST_BUILTINS_OBJS += test-hashmap.o\n> -TEST_BUILTINS_OBJS += test-index-version.o\n> -TEST_BUILTINS_OBJS += test-json-writer.o\n> -TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o\n> -TEST_BUILTINS_OBJS += test-match-trees.o\n> -TEST_BUILTINS_OBJS += test-mergesort.o\n> -TEST_BUILTINS_OBJS += test-mktemp.o\n> -TEST_BUILTINS_OBJS += test-oid-array.o\n> -TEST_BUILTINS_OBJS += test-oidmap.o\n> -TEST_BUILTINS_OBJS += test-oidtree.o\n> -TEST_BUILTINS_OBJS += test-online-cpus.o\n> -TEST_BUILTINS_OBJS += test-parse-options.o\n> -TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n> -TEST_BUILTINS_OBJS += test-partial-clone.o\n> -TEST_BUILTINS_OBJS += test-path-utils.o\n> -TEST_BUILTINS_OBJS += test-pcre2-config.o\n> -TEST_BUILTINS_OBJS += test-pkt-line.o\n> -TEST_BUILTINS_OBJS += test-prio-queue.o\n> -TEST_BUILTINS_OBJS += test-proc-receive.o\n> -TEST_BUILTINS_OBJS += test-progress.o\n> -TEST_BUILTINS_OBJS += test-reach.o\n> -TEST_BUILTINS_OBJS += test-read-cache.o\n> -TEST_BUILTINS_OBJS += test-read-graph.o\n> -TEST_BUILTINS_OBJS += test-read-midx.o\n> -TEST_BUILTINS_OBJS += test-ref-store.o\n> -TEST_BUILTINS_OBJS += test-regex.o\n> -TEST_BUILTINS_OBJS += test-repository.o\n> -TEST_BUILTINS_OBJS += test-revision-walking.o\n> -TEST_BUILTINS_OBJS += test-run-command.o\n> -TEST_BUILTINS_OBJS += test-scrap-cache-tree.o\n> -TEST_BUILTINS_OBJS += test-serve-v2.o\n> -TEST_BUILTINS_OBJS += test-sha1.o\n> -TEST_BUILTINS_OBJS += test-sha256.o\n> -TEST_BUILTINS_OBJS += test-sigchain.o\n> -TEST_BUILTINS_OBJS += test-simple-ipc.o\n> -TEST_BUILTINS_OBJS += test-strcmp-offset.o\n> -TEST_BUILTINS_OBJS += test-string-list.o\n> -TEST_BUILTINS_OBJS += test-submodule-config.o\n> -TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o\n> -TEST_BUILTINS_OBJS += test-subprocess.o\n> -TEST_BUILTINS_OBJS += test-trace2.o\n> -TEST_BUILTINS_OBJS += test-urlmatch-normalization.o\n> -TEST_BUILTINS_OBJS += test-userdiff.o\n> -TEST_BUILTINS_OBJS += test-wildmatch.o\n> -TEST_BUILTINS_OBJS += test-windows-named-pipe.o\n> -TEST_BUILTINS_OBJS += test-write-cache.o\n> -TEST_BUILTINS_OBJS += test-xml-encode.o\n> -\n>   # Do not add more tests here unless they have extra dependencies. Add\n>   # them in TEST_BUILTINS_OBJS above.\n>   TEST_PROGRAMS_NEED_X += test-fake-ssh\n>   TEST_PROGRAMS_NEED_X += test-tool\n>   \n> -TEST_PROGRAMS = $(patsubst %,t/helper/%$X,$(TEST_PROGRAMS_NEED_X))\n> +TEST_PROGRAMS_NEED_X_SRC += $(TEST_PROGRAMS_NEED_X:%=t/helper/%.c)\n> +TEST_PROGRAMS += $(TEST_PROGRAMS_NEED_X_SRC:%.c=%$X)\n> +TEST_BUILTINS_SRC += $(filter-out $(TEST_PROGRAMS_NEED_X_SRC),$(wildcard t/helper/*.c))\n> +TEST_BUILTINS_OBJS += $(TEST_BUILTINS_SRC:%.c=%.o)\n>   \n> -# List built-in command $C whose implementation cmd_$C() is not in\n> -# builtin/$C.o but is linked in as part of some other command.\n> +# List built-in command $C whose implementation cmd_$C() is in\n> +# builtin/$C.o\n> +BUILTIN_OBJS = $(patsubst %.c,%.o,$(wildcard builtin/*.c))\n>   BUILT_INS += $(patsubst builtin/%.o,git-%$X,$(BUILTIN_OBJS))\n>   \n> +# List built-in command $C whose implementation cmd_$C() is not in\n> +# builtin/$C.o but is linked in as part of some other command.\n>   BUILT_INS += git-cherry$X\n>   BUILT_INS += git-cherry-pick$X\n>   BUILT_INS += git-format-patch$X\n> @@ -837,355 +786,28 @@ LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentat\n>   \t-name Documentation -prune -o \\\n>   \t-name '*.h' -print)))\n>   \n> -LIB_OBJS += abspath.o\n> -LIB_OBJS += add-interactive.o\n> -LIB_OBJS += add-patch.o\n> -LIB_OBJS += advice.o\n> -LIB_OBJS += alias.o\n> -LIB_OBJS += alloc.o\n> -LIB_OBJS += apply.o\n> -LIB_OBJS += archive-tar.o\n> -LIB_OBJS += archive-zip.o\n> -LIB_OBJS += archive.o\n> -LIB_OBJS += attr.o\n> -LIB_OBJS += base85.o\n> -LIB_OBJS += bisect.o\n> -LIB_OBJS += blame.o\n> -LIB_OBJS += blob.o\n> -LIB_OBJS += bloom.o\n> -LIB_OBJS += branch.o\n> -LIB_OBJS += bulk-checkin.o\n> -LIB_OBJS += bundle.o\n> -LIB_OBJS += cache-tree.o\n> -LIB_OBJS += cbtree.o\n> -LIB_OBJS += chdir-notify.o\n> -LIB_OBJS += checkout.o\n> -LIB_OBJS += chunk-format.o\n> -LIB_OBJS += color.o\n> -LIB_OBJS += column.o\n> -LIB_OBJS += combine-diff.o\n> -LIB_OBJS += commit-graph.o\n> -LIB_OBJS += commit-reach.o\n> -LIB_OBJS += commit.o\n> +# LIB_OBJS: compat/* objects that live at the top-level\n> +CONDITIONAL_OBJS += unix-socket.o\n> +CONDITIONAL_OBJS += unix-stream-server.o\n> +CONDITIONAL_OBJS += sha1dc_git.o\n> +\n> +# LIB_OBJS: Mostly glob *.c at the top-level, with some exlusions\n> +LIB_OBJS += $(filter-out \\\n> +\t$(CONDITIONAL_OBJS) \\\n> +\tgit.o common-main.o $(PROGRAM_OBJS) \\\n> +\t$(FUZZ_OBJS) $(CURL_OBJS),\\\n> +\t$(patsubst %.c,%.o,$(wildcard *.c)))\n> +\n> +# LIB_OBJS: Directories that contain only LIB_OBJS\n> +LIB_OBJS_DIRS += ewah\n> +LIB_OBJS_DIRS += negotiator\n> +LIB_OBJS_DIRS += refs\n> +LIB_OBJS_DIRS += trace2\n> +LIB_OBJS += $(patsubst %.c,%.o,$(wildcard $(addsuffix /*.c,$(LIB_OBJS_DIRS))))\n> +\n> +# LIB_OBJS: unconditional compat/* objects\n>   LIB_OBJS += compat/obstack.o\n>   LIB_OBJS += compat/terminal.o\n> -LIB_OBJS += config.o\n> -LIB_OBJS += connect.o\n> -LIB_OBJS += connected.o\n> -LIB_OBJS += convert.o\n> -LIB_OBJS += copy.o\n> -LIB_OBJS += credential.o\n> -LIB_OBJS += csum-file.o\n> -LIB_OBJS += ctype.o\n> -LIB_OBJS += date.o\n> -LIB_OBJS += decorate.o\n> -LIB_OBJS += delta-islands.o\n> -LIB_OBJS += diff-delta.o\n> -LIB_OBJS += diff-merges.o\n> -LIB_OBJS += diff-lib.o\n> -LIB_OBJS += diff-no-index.o\n> -LIB_OBJS += diff.o\n> -LIB_OBJS += diffcore-break.o\n> -LIB_OBJS += diffcore-delta.o\n> -LIB_OBJS += diffcore-order.o\n> -LIB_OBJS += diffcore-pickaxe.o\n> -LIB_OBJS += diffcore-rename.o\n> -LIB_OBJS += diffcore-rotate.o\n> -LIB_OBJS += dir-iterator.o\n> -LIB_OBJS += dir.o\n> -LIB_OBJS += editor.o\n> -LIB_OBJS += entry.o\n> -LIB_OBJS += environment.o\n> -LIB_OBJS += ewah/bitmap.o\n> -LIB_OBJS += ewah/ewah_bitmap.o\n> -LIB_OBJS += ewah/ewah_io.o\n> -LIB_OBJS += ewah/ewah_rlw.o\n> -LIB_OBJS += exec-cmd.o\n> -LIB_OBJS += fetch-negotiator.o\n> -LIB_OBJS += fetch-pack.o\n> -LIB_OBJS += fmt-merge-msg.o\n> -LIB_OBJS += fsck.o\n> -LIB_OBJS += fsmonitor.o\n> -LIB_OBJS += gettext.o\n> -LIB_OBJS += gpg-interface.o\n> -LIB_OBJS += graph.o\n> -LIB_OBJS += grep.o\n> -LIB_OBJS += hash-lookup.o\n> -LIB_OBJS += hashmap.o\n> -LIB_OBJS += help.o\n> -LIB_OBJS += hex.o\n> -LIB_OBJS += hook.o\n> -LIB_OBJS += ident.o\n> -LIB_OBJS += json-writer.o\n> -LIB_OBJS += kwset.o\n> -LIB_OBJS += levenshtein.o\n> -LIB_OBJS += line-log.o\n> -LIB_OBJS += line-range.o\n> -LIB_OBJS += linear-assignment.o\n> -LIB_OBJS += list-objects-filter-options.o\n> -LIB_OBJS += list-objects-filter.o\n> -LIB_OBJS += list-objects.o\n> -LIB_OBJS += ll-merge.o\n> -LIB_OBJS += lockfile.o\n> -LIB_OBJS += log-tree.o\n> -LIB_OBJS += ls-refs.o\n> -LIB_OBJS += mailinfo.o\n> -LIB_OBJS += mailmap.o\n> -LIB_OBJS += match-trees.o\n> -LIB_OBJS += mem-pool.o\n> -LIB_OBJS += merge-blobs.o\n> -LIB_OBJS += merge-ort.o\n> -LIB_OBJS += merge-ort-wrappers.o\n> -LIB_OBJS += merge-recursive.o\n> -LIB_OBJS += merge.o\n> -LIB_OBJS += mergesort.o\n> -LIB_OBJS += midx.o\n> -LIB_OBJS += name-hash.o\n> -LIB_OBJS += negotiator/default.o\n> -LIB_OBJS += negotiator/noop.o\n> -LIB_OBJS += negotiator/skipping.o\n> -LIB_OBJS += notes-cache.o\n> -LIB_OBJS += notes-merge.o\n> -LIB_OBJS += notes-utils.o\n> -LIB_OBJS += notes.o\n> -LIB_OBJS += object-file.o\n> -LIB_OBJS += object-name.o\n> -LIB_OBJS += object.o\n> -LIB_OBJS += oid-array.o\n> -LIB_OBJS += oidmap.o\n> -LIB_OBJS += oidset.o\n> -LIB_OBJS += oidtree.o\n> -LIB_OBJS += pack-bitmap-write.o\n> -LIB_OBJS += pack-bitmap.o\n> -LIB_OBJS += pack-check.o\n> -LIB_OBJS += pack-objects.o\n> -LIB_OBJS += pack-revindex.o\n> -LIB_OBJS += pack-write.o\n> -LIB_OBJS += packfile.o\n> -LIB_OBJS += pager.o\n> -LIB_OBJS += parallel-checkout.o\n> -LIB_OBJS += parse-options-cb.o\n> -LIB_OBJS += parse-options.o\n> -LIB_OBJS += patch-delta.o\n> -LIB_OBJS += patch-ids.o\n> -LIB_OBJS += path.o\n> -LIB_OBJS += pathspec.o\n> -LIB_OBJS += pkt-line.o\n> -LIB_OBJS += preload-index.o\n> -LIB_OBJS += pretty.o\n> -LIB_OBJS += prio-queue.o\n> -LIB_OBJS += progress.o\n> -LIB_OBJS += promisor-remote.o\n> -LIB_OBJS += prompt.o\n> -LIB_OBJS += protocol.o\n> -LIB_OBJS += protocol-caps.o\n> -LIB_OBJS += prune-packed.o\n> -LIB_OBJS += quote.o\n> -LIB_OBJS += range-diff.o\n> -LIB_OBJS += reachable.o\n> -LIB_OBJS += read-cache.o\n> -LIB_OBJS += rebase-interactive.o\n> -LIB_OBJS += rebase.o\n> -LIB_OBJS += ref-filter.o\n> -LIB_OBJS += reflog-walk.o\n> -LIB_OBJS += refs.o\n> -LIB_OBJS += refs/debug.o\n> -LIB_OBJS += refs/files-backend.o\n> -LIB_OBJS += refs/iterator.o\n> -LIB_OBJS += refs/packed-backend.o\n> -LIB_OBJS += refs/ref-cache.o\n> -LIB_OBJS += refspec.o\n> -LIB_OBJS += remote.o\n> -LIB_OBJS += replace-object.o\n> -LIB_OBJS += repo-settings.o\n> -LIB_OBJS += repository.o\n> -LIB_OBJS += rerere.o\n> -LIB_OBJS += reset.o\n> -LIB_OBJS += resolve-undo.o\n> -LIB_OBJS += revision.o\n> -LIB_OBJS += run-command.o\n> -LIB_OBJS += send-pack.o\n> -LIB_OBJS += sequencer.o\n> -LIB_OBJS += serve.o\n> -LIB_OBJS += server-info.o\n> -LIB_OBJS += setup.o\n> -LIB_OBJS += shallow.o\n> -LIB_OBJS += sideband.o\n> -LIB_OBJS += sigchain.o\n> -LIB_OBJS += sparse-index.o\n> -LIB_OBJS += split-index.o\n> -LIB_OBJS += stable-qsort.o\n> -LIB_OBJS += strbuf.o\n> -LIB_OBJS += streaming.o\n> -LIB_OBJS += string-list.o\n> -LIB_OBJS += strmap.o\n> -LIB_OBJS += strvec.o\n> -LIB_OBJS += sub-process.o\n> -LIB_OBJS += submodule-config.o\n> -LIB_OBJS += submodule.o\n> -LIB_OBJS += symlinks.o\n> -LIB_OBJS += tag.o\n> -LIB_OBJS += tempfile.o\n> -LIB_OBJS += thread-utils.o\n> -LIB_OBJS += tmp-objdir.o\n> -LIB_OBJS += trace.o\n> -LIB_OBJS += trace2.o\n> -LIB_OBJS += trace2/tr2_cfg.o\n> -LIB_OBJS += trace2/tr2_cmd_name.o\n> -LIB_OBJS += trace2/tr2_dst.o\n> -LIB_OBJS += trace2/tr2_sid.o\n> -LIB_OBJS += trace2/tr2_sysenv.o\n> -LIB_OBJS += trace2/tr2_tbuf.o\n> -LIB_OBJS += trace2/tr2_tgt_event.o\n> -LIB_OBJS += trace2/tr2_tgt_normal.o\n> -LIB_OBJS += trace2/tr2_tgt_perf.o\n> -LIB_OBJS += trace2/tr2_tls.o\n> -LIB_OBJS += trailer.o\n> -LIB_OBJS += transport-helper.o\n> -LIB_OBJS += transport.o\n> -LIB_OBJS += tree-diff.o\n> -LIB_OBJS += tree-walk.o\n> -LIB_OBJS += tree.o\n> -LIB_OBJS += unpack-trees.o\n> -LIB_OBJS += upload-pack.o\n> -LIB_OBJS += url.o\n> -LIB_OBJS += urlmatch.o\n> -LIB_OBJS += usage.o\n> -LIB_OBJS += userdiff.o\n> -LIB_OBJS += utf8.o\n> -LIB_OBJS += varint.o\n> -LIB_OBJS += version.o\n> -LIB_OBJS += versioncmp.o\n> -LIB_OBJS += walker.o\n> -LIB_OBJS += wildmatch.o\n> -LIB_OBJS += worktree.o\n> -LIB_OBJS += wrapper.o\n> -LIB_OBJS += write-or-die.o\n> -LIB_OBJS += ws.o\n> -LIB_OBJS += wt-status.o\n> -LIB_OBJS += xdiff-interface.o\n> -LIB_OBJS += zlib.o\n> -\n> -BUILTIN_OBJS += builtin/add.o\n> -BUILTIN_OBJS += builtin/am.o\n> -BUILTIN_OBJS += builtin/annotate.o\n> -BUILTIN_OBJS += builtin/apply.o\n> -BUILTIN_OBJS += builtin/archive.o\n> -BUILTIN_OBJS += builtin/bisect--helper.o\n> -BUILTIN_OBJS += builtin/blame.o\n> -BUILTIN_OBJS += builtin/branch.o\n> -BUILTIN_OBJS += builtin/bugreport.o\n> -BUILTIN_OBJS += builtin/bundle.o\n> -BUILTIN_OBJS += builtin/cat-file.o\n> -BUILTIN_OBJS += builtin/check-attr.o\n> -BUILTIN_OBJS += builtin/check-ignore.o\n> -BUILTIN_OBJS += builtin/check-mailmap.o\n> -BUILTIN_OBJS += builtin/check-ref-format.o\n> -BUILTIN_OBJS += builtin/checkout--worker.o\n> -BUILTIN_OBJS += builtin/checkout-index.o\n> -BUILTIN_OBJS += builtin/checkout.o\n> -BUILTIN_OBJS += builtin/clean.o\n> -BUILTIN_OBJS += builtin/clone.o\n> -BUILTIN_OBJS += builtin/column.o\n> -BUILTIN_OBJS += builtin/commit-graph.o\n> -BUILTIN_OBJS += builtin/commit-tree.o\n> -BUILTIN_OBJS += builtin/commit.o\n> -BUILTIN_OBJS += builtin/config.o\n> -BUILTIN_OBJS += builtin/count-objects.o\n> -BUILTIN_OBJS += builtin/credential-cache--daemon.o\n> -BUILTIN_OBJS += builtin/credential-cache.o\n> -BUILTIN_OBJS += builtin/credential-store.o\n> -BUILTIN_OBJS += builtin/credential.o\n> -BUILTIN_OBJS += builtin/describe.o\n> -BUILTIN_OBJS += builtin/diff-files.o\n> -BUILTIN_OBJS += builtin/diff-index.o\n> -BUILTIN_OBJS += builtin/diff-tree.o\n> -BUILTIN_OBJS += builtin/diff.o\n> -BUILTIN_OBJS += builtin/difftool.o\n> -BUILTIN_OBJS += builtin/env--helper.o\n> -BUILTIN_OBJS += builtin/fast-export.o\n> -BUILTIN_OBJS += builtin/fast-import.o\n> -BUILTIN_OBJS += builtin/fetch-pack.o\n> -BUILTIN_OBJS += builtin/fetch.o\n> -BUILTIN_OBJS += builtin/fmt-merge-msg.o\n> -BUILTIN_OBJS += builtin/for-each-ref.o\n> -BUILTIN_OBJS += builtin/for-each-repo.o\n> -BUILTIN_OBJS += builtin/fsck.o\n> -BUILTIN_OBJS += builtin/gc.o\n> -BUILTIN_OBJS += builtin/get-tar-commit-id.o\n> -BUILTIN_OBJS += builtin/grep.o\n> -BUILTIN_OBJS += builtin/hash-object.o\n> -BUILTIN_OBJS += builtin/help.o\n> -BUILTIN_OBJS += builtin/index-pack.o\n> -BUILTIN_OBJS += builtin/init-db.o\n> -BUILTIN_OBJS += builtin/interpret-trailers.o\n> -BUILTIN_OBJS += builtin/log.o\n> -BUILTIN_OBJS += builtin/ls-files.o\n> -BUILTIN_OBJS += builtin/ls-remote.o\n> -BUILTIN_OBJS += builtin/ls-tree.o\n> -BUILTIN_OBJS += builtin/mailinfo.o\n> -BUILTIN_OBJS += builtin/mailsplit.o\n> -BUILTIN_OBJS += builtin/merge-base.o\n> -BUILTIN_OBJS += builtin/merge-file.o\n> -BUILTIN_OBJS += builtin/merge-index.o\n> -BUILTIN_OBJS += builtin/merge-ours.o\n> -BUILTIN_OBJS += builtin/merge-recursive.o\n> -BUILTIN_OBJS += builtin/merge-tree.o\n> -BUILTIN_OBJS += builtin/merge.o\n> -BUILTIN_OBJS += builtin/mktag.o\n> -BUILTIN_OBJS += builtin/mktree.o\n> -BUILTIN_OBJS += builtin/multi-pack-index.o\n> -BUILTIN_OBJS += builtin/mv.o\n> -BUILTIN_OBJS += builtin/name-rev.o\n> -BUILTIN_OBJS += builtin/notes.o\n> -BUILTIN_OBJS += builtin/pack-objects.o\n> -BUILTIN_OBJS += builtin/pack-redundant.o\n> -BUILTIN_OBJS += builtin/pack-refs.o\n> -BUILTIN_OBJS += builtin/patch-id.o\n> -BUILTIN_OBJS += builtin/prune-packed.o\n> -BUILTIN_OBJS += builtin/prune.o\n> -BUILTIN_OBJS += builtin/pull.o\n> -BUILTIN_OBJS += builtin/push.o\n> -BUILTIN_OBJS += builtin/range-diff.o\n> -BUILTIN_OBJS += builtin/read-tree.o\n> -BUILTIN_OBJS += builtin/rebase.o\n> -BUILTIN_OBJS += builtin/receive-pack.o\n> -BUILTIN_OBJS += builtin/reflog.o\n> -BUILTIN_OBJS += builtin/remote-ext.o\n> -BUILTIN_OBJS += builtin/remote-fd.o\n> -BUILTIN_OBJS += builtin/remote.o\n> -BUILTIN_OBJS += builtin/repack.o\n> -BUILTIN_OBJS += builtin/replace.o\n> -BUILTIN_OBJS += builtin/rerere.o\n> -BUILTIN_OBJS += builtin/reset.o\n> -BUILTIN_OBJS += builtin/rev-list.o\n> -BUILTIN_OBJS += builtin/rev-parse.o\n> -BUILTIN_OBJS += builtin/revert.o\n> -BUILTIN_OBJS += builtin/rm.o\n> -BUILTIN_OBJS += builtin/send-pack.o\n> -BUILTIN_OBJS += builtin/shortlog.o\n> -BUILTIN_OBJS += builtin/show-branch.o\n> -BUILTIN_OBJS += builtin/show-index.o\n> -BUILTIN_OBJS += builtin/show-ref.o\n> -BUILTIN_OBJS += builtin/sparse-checkout.o\n> -BUILTIN_OBJS += builtin/stash.o\n> -BUILTIN_OBJS += builtin/stripspace.o\n> -BUILTIN_OBJS += builtin/submodule--helper.o\n> -BUILTIN_OBJS += builtin/symbolic-ref.o\n> -BUILTIN_OBJS += builtin/tag.o\n> -BUILTIN_OBJS += builtin/unpack-file.o\n> -BUILTIN_OBJS += builtin/unpack-objects.o\n> -BUILTIN_OBJS += builtin/update-index.o\n> -BUILTIN_OBJS += builtin/update-ref.o\n> -BUILTIN_OBJS += builtin/update-server-info.o\n> -BUILTIN_OBJS += builtin/upload-archive.o\n> -BUILTIN_OBJS += builtin/upload-pack.o\n> -BUILTIN_OBJS += builtin/var.o\n> -BUILTIN_OBJS += builtin/verify-commit.o\n> -BUILTIN_OBJS += builtin/verify-pack.o\n> -BUILTIN_OBJS += builtin/verify-tag.o\n> -BUILTIN_OBJS += builtin/worktree.o\n> -BUILTIN_OBJS += builtin/write-tree.o\n>   \n>   # THIRD_PARTY_SOURCES is a list of patterns compatible with the\n>   # $(filter) and $(filter-out) family of functions. They specify source\n> @@ -2076,6 +1698,7 @@ endif\n>   LIBS = $(filter-out %.o, $(GITLIBS)) $(EXTLIBS)\n>   \n>   BASIC_CFLAGS += $(COMPAT_CFLAGS)\n> +LIB_OBJS_NO_COMPAT := $(LIB_OBJS)\n>   LIB_OBJS += $(COMPAT_OBJS)\n>   \n>   # Quote for C\n> @@ -2436,17 +2059,12 @@ reconfigure config.mak.autogen: config.status\n>   .PHONY: reconfigure # This is a convenience target.\n>   endif\n>   \n> -XDIFF_OBJS += xdiff/xdiffi.o\n> -XDIFF_OBJS += xdiff/xemit.o\n> -XDIFF_OBJS += xdiff/xhistogram.o\n> -XDIFF_OBJS += xdiff/xmerge.o\n> -XDIFF_OBJS += xdiff/xpatience.o\n> -XDIFF_OBJS += xdiff/xprepare.o\n> -XDIFF_OBJS += xdiff/xutils.o\n> +XDIFF_SRC += $(wildcard xdiff/*.c)\n> +XDIFF_OBJS += $(XDIFF_SRC:.c=.o)\n>   .PHONY: xdiff-objs\n>   xdiff-objs: $(XDIFF_OBJS)\n>   \n> -TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n> +TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(TEST_BUILTINS_OBJS)\n>   .PHONY: test-objs\n>   test-objs: $(TEST_OBJS)\n>   \n> @@ -2457,13 +2075,20 @@ GIT_OBJS += git.o\n>   .PHONY: git-objs\n>   git-objs: $(GIT_OBJS)\n>   \n> +CURL_SRC += http.c\n> +CURL_SRC += http-walker.c\n> +CURL_SRC += remote-curl.c\n> +CURL_OBJS += $(CURL_SRC:.c=.o)\n> +.PHONY: curl-objs\n> +curl-objs: $(CURL_OBJS)\n> +\n>   OBJECTS += $(GIT_OBJS)\n>   OBJECTS += $(PROGRAM_OBJS)\n>   OBJECTS += $(TEST_OBJS)\n>   OBJECTS += $(XDIFF_OBJS)\n>   OBJECTS += $(FUZZ_OBJS)\n>   ifndef NO_CURL\n> -\tOBJECTS += http.o http-walker.o remote-curl.o\n> +OBJECTS += $(CURL_OBJS)\n>   endif\n>   .PHONY: objects\n>   objects: $(OBJECTS)\n> @@ -2900,7 +2525,7 @@ perf: all\n>   \n>   .PRECIOUS: $(TEST_OBJS)\n>   \n> -t/helper/test-tool$X: $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n> +t/helper/test-tool$X: $(TEST_BUILTINS_OBJS)\n>   \n>   t/helper/test-%$X: t/helper/test-%.o GIT-LDFLAGS $(GITLIBS)\n>   \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(filter %.a,$^) $(LIBS)\n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index fd1399c440f..d57c9203b66 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -111,32 +111,15 @@ project(git\n>   #TODO Add pcre support\n>   \n>   #macros for parsing the Makefile for sources and scripts\n> -macro(parse_makefile_for_sources list_var regex)\n> -\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n> -\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n> -\tstring(REPLACE \"$(COMPAT_OBJS)\" \"\" ${list_var} ${${list_var}}) #remove \"$(COMPAT_OBJS)\" This is only for libgit.\n> -\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n> -\tstring(REPLACE \".o\" \".c;\" ${list_var} ${${list_var}}) #change .o to .c, ; is for converting the string into a list\n> -\tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n> -\tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n> -endmacro()\n> -\n> -macro(parse_makefile_for_scripts list_var regex lang)\n> -\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+=(.*)\")\n> -\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n> -\tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n> -\tstring(REPLACE \" \" \";\" ${list_var} ${${list_var}}) #convert string to a list\n> -\tif(NOT ${lang}) #exclude for SCRIPT_LIB\n> -\t\tlist(TRANSFORM ${list_var} REPLACE \"${lang}\" \"\") #do the replacement\n> -\tendif()\n> -endmacro()\n> -\n> -macro(parse_makefile_for_executables list_var regex)\n> -\tfile(STRINGS ${CMAKE_SOURCE_DIR}/Makefile ${list_var} REGEX \"^${regex} \\\\+= git-(.*)\")\n> -\tstring(REPLACE \"${regex} +=\" \"\" ${list_var} ${${list_var}})\n> +macro(ask_makefile_for_list list_var var_name)\n> +\texecute_process(COMMAND make print-list-${var_name}\n> +\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}\n> +\t\t\tOUTPUT_VARIABLE ${list_var})\n> +\tstring(REGEX REPLACE \"\\\\.o\\n\" \".c\\n\" ${list_var} ${${list_var}}) #change .o to .c\n>   \tstring(STRIP ${${list_var}} ${list_var}) #remove trailing/leading whitespaces\n> -\tstring(REPLACE \"git-\" \"\" ${list_var} ${${list_var}}) #strip `git-` prefix\n> -\tstring(REPLACE \"\\$X\" \";\" ${list_var} ${${list_var}}) #strip $X, ; is for converting the string into a list\n> +\t## Parse the Makefile print-list-% format\n> +\tstring(REGEX REPLACE \"${var_name} =\\n\" \"\" ${list_var} ${${list_var}})\n> +\tstring(REGEX REPLACE \"${var_name} \\\\+?= ([^\\n]+)\" \"\\\\1;\" ${list_var} ${${list_var}})\n>   \tlist(TRANSFORM ${list_var} STRIP) #remove trailing/leading whitespaces for each element in list\n>   \tlist(REMOVE_ITEM ${list_var} \"\") #remove empty list elements\n>   endmacro()\n> @@ -635,14 +618,14 @@ include_directories(${CMAKE_BINARY_DIR})\n>   \n>   #build\n>   #libgit\n> -parse_makefile_for_sources(libgit_SOURCES \"LIB_OBJS\")\n> +ask_makefile_for_list(libgit_SOURCES \"LIB_OBJS_NO_COMPAT\")\n>   \n>   list(TRANSFORM libgit_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n>   list(TRANSFORM compat_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n>   add_library(libgit ${libgit_SOURCES} ${compat_SOURCES})\n>   \n>   #libxdiff\n> -parse_makefile_for_sources(libxdiff_SOURCES \"XDIFF_OBJS\")\n> +ask_makefile_for_list(libxdiff_SOURCES \"XDIFF_OBJS\")\n>   \n>   list(TRANSFORM libxdiff_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n>   add_library(xdiff STATIC ${libxdiff_SOURCES})\n> @@ -693,7 +676,7 @@ elseif(UNIX)\n>   endif()\n>   \n>   #git\n> -parse_makefile_for_sources(git_SOURCES \"BUILTIN_OBJS\")\n> +ask_makefile_for_list(git_SOURCES \"BUILTIN_OBJS\")\n>   \n>   list(TRANSFORM git_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n>   add_executable(git ${CMAKE_SOURCE_DIR}/git.c ${git_SOURCES})\n> @@ -729,7 +712,9 @@ if(CURL_FOUND)\n>   \tendif()\n>   endif()\n>   \n> -parse_makefile_for_executables(git_builtin_extra \"BUILT_INS\")\n> +ask_makefile_for_list(git_builtin_extra \"BUILT_INS\")\n> +list(TRANSFORM git_builtin_extra REPLACE \"^git-(.*)\" \"\\\\1\") #strip `git-` prefix\n> +list(TRANSFORM git_builtin_extra REPLACE \"\\.exe$\" \"\") #strip $X\n>   \n>   option(SKIP_DASHED_BUILT_INS \"Skip hardlinking the dashed versions of the built-ins\")\n>   \n> @@ -766,8 +751,8 @@ set(GITWEBDIR ${FALLBACK_RUNTIME_PREFIX}/share/locale)\n>   set(INSTLIBDIR ${FALLBACK_RUNTIME_PREFIX}/share/perl5)\n>   \n>   #shell scripts\n> -parse_makefile_for_scripts(git_sh_scripts \"SCRIPT_SH\" \".sh\")\n> -parse_makefile_for_scripts(git_shlib_scripts \"SCRIPT_LIB\" \"\")\n> +ask_makefile_for_list(git_sh_scripts \"SCRIPT_SH_GEN\")\n> +ask_makefile_for_list(git_shlib_scripts \"SCRIPT_LIB_GEN\")\n>   set(git_shell_scripts\n>   \t${git_sh_scripts} ${git_shlib_scripts} git-instaweb)\n>   \n> @@ -787,7 +772,7 @@ foreach(script ${git_shell_scripts})\n>   endforeach()\n>   \n>   #perl scripts\n> -parse_makefile_for_scripts(git_perl_scripts \"SCRIPT_PERL\" \".perl\")\n> +ask_makefile_for_list(git_perl_scripts \"SCRIPT_PERL_GEN\")\n>   \n>   #create perl header\n>   file(STRINGS ${CMAKE_SOURCE_DIR}/perl/header_templates/fixed_prefix.template.pl perl_header )\n> @@ -910,9 +895,9 @@ add_executable(test-fake-ssh ${CMAKE_SOURCE_DIR}/t/helper/test-fake-ssh.c)\n>   target_link_libraries(test-fake-ssh common-main)\n>   \n>   #test-tool\n> -parse_makefile_for_sources(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n> +ask_makefile_for_list(test-tool_SOURCES \"TEST_BUILTINS_OBJS\")\n>   \n> -list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/t/helper/\")\n> +list(TRANSFORM test-tool_SOURCES PREPEND \"${CMAKE_SOURCE_DIR}/\")\n>   add_executable(test-tool ${CMAKE_SOURCE_DIR}/t/helper/test-tool.c ${test-tool_SOURCES})\n>   target_link_libraries(test-tool common-main)\n>   \n> \n"},{"id":"440575","messageId":"211106.86tugpfikn.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"24482f96-7d87-1570-a171-95ec182f6091@gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-06T14:27:01Z","receivedAt":"2021-11-06T14:46:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Nov 06 2021, Phillip Wood wrote:\n\n> Hi Ævar\n>\n> On 01/11/2021 19:19, Ævar Arnfjörð Bjarmason wrote:\n>> Remove the hardcoded lists of objects in favor of using\n>> $(wildcard). This means that every time a built-in, test tool etc. is\n>> added we won't need to patch the top-level Makefile, except for the\n>> few remaining cases where the asset in question would make it onto one\n>> of our list of exceptions.\n>> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\n>> subdirectory, 2010-02-22) this has been relatively easy to do (and\n>> even before that we could glob builtin-*.c). This pattern of\n>> exhaustively enumerating files was then carried forward for\n>> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\n>> test-tool program, 2018-03-24).\n>> One reason not to do this is that now a new *.c file at the\n>> top-level\n>> will be immediately picked up, so if a new *.c file is being worked on\n>> \"make\" will error if it doesn't compile, whereas before that file\n>> would need to be explicitly listed in the Makefile. I think than small\n>> trade-off is worth it.\n>\n> If I need to split up some uncommitted changes into several commits\n> and I know it is going to be fiddly to do so I will sometimes copy the \n> original file to foo.safe.c and then edit foo.c to create each\n> commit. Then I can easily compile and test each commit and editing the\n> file directly is often easier than using add -p and editing the\n> hunks. With this patch running make will fail in that case I think.\n\nWould being able to do something like:\n\n    make EXCLUDE_WILDCARD=foo.safe.c\n\nSatisfy this use-case?\n\nWe can also have some DEVOPTS knob so we'll prune out files found if a\n$(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\nthat (and could implement it) if it was optional.\n\nAlso note that you've got some of this already, e.g. we'll pick up *.h\nfiles via a glob for \"make TAGS\", the dependency graph etc.\n\n>> We could make this simpler still for the Makefile by moving\n>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n>> There's a few small \"while we're at it\" changes here, since I'm\n>> touching the code in question:\n>>   - Start splitting up the the \"Guard against the environment\"\n>> section\n>>     at the top, but don't move anything that exists there out to avoid\n>>     merge conflicts\n>>   - The $(TEST_BUILTINS_OBJS) variable was needlessly complex,\n>> because\n>>     it didn't have the full paths we'd pathsubst it back & forth.\n>>   - Introduce *_SRC in addition to *_OBJ for the variable I'm\n>>     touching. Eventually we'll want to do this for all the *.o files,\n>>     i.e. make the *.c list a source of truth for *.o, which means we can\n>>     e.g. use that exhaustive list for \"make TAGS\".\n>>   - Add a missing \"curl-objs\" target. See 029bac01a87 (Makefile: add\n>>     {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)\n>>     for the commit that added the rest.\n>>   - De-indent an \"ifndef\" block, we don't usually indent their\n>>     contents.\n>> On the CMake changes here:\n>>   - When CMake support was introduced in was introduced\n>>     061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)\n>>     there was a discussion about the maintenance burden of maintaining the\n>>     top-level Makefile in parallel with CMakeLists.txt[1] where reviewers\n>>     were assured that doing so would simply be a matter of adding something\n>>     to a list in the CMake recipe.\n>>     Between change and some recent changes of mine where the\n>> \"vs-build\"\n>>     job failed to a divergence between the Makefile and CMakeList.txt I\n>>     can confidently say that that doesn't at all match reality. Even\n>>     seemingly trivial changes to the Makefile like this one are forcing\n>>     us to do a deep-dive into CMake internals to make forward progress\n>>     with our main build system.\n>\n> My recollection is that the discussions were about not having to touch\n> CMakeList.txt when adding new files to the build and I think that \n> largely works. I don't think a lot of the changes you have been making\n> recently were anticipated in that discussion.\n\nThat's part of it, but the concern about needing to maintain two systems\nin perpetuity was also brought up, and it not being a hard dependency,\nhaving that vs-{build,test} job soft-fail on it etc. were brought up but\nthat's not what we've got now.\n\nIn any case, having to maintain two build systems and the maintainer(s)\nof the CMake being inactive one is the situation we're in now.\n\n>>   - The promised \"We can add a (continue-on-error) to vs-build job to\n>>     make this process less of a hindrance.\" in [2] never materialized.\n>>     Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n>>     vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n>>     getting the CI to pass goes.\n>>   - The \"vs-build\" CI doesn't actually require that there be no GNU\n>> make\n>>     usage in the job, as it itself has a hard dependency on running a\n>>     \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n>>     goes we don't need a GNU-make free build system for vs-build.\n>\n> We need GNU-make for the ci job but an individual developer using\n> CMake does not need GNU-make installed. On linux it is possible to\n> build git without having make installed by using cmake and ninja [1]\n\nI'd tried that, but didn't know about your [1] topic. If you or someone\nis actively willing to fix things up in CMake....\n\n>>   - The stated goal in 061c2240b1b of avoiding a GNU make dependency\n>>     for developer because it requires an SDK that \"occupies around two\n>>     gigabytes\" and \"three quarters of a gigabyte worth of Git objects\"\n>>     hardly seems worthwhile trade-off given the above. Disk space is cheap,\n>>     developer time required to maintain two parallel build systems isn't.\n>\n> That rather assumes everyone has plenty of disk space and a decent\n> network connection.\n\nI realize that there's going to be a person who's got 1.5GB of disk\nspace and needs to delete a movie they've downloaded or whatever. I just\nthink that's worth it v.s. maintenance trade-off.\n\nDoesn't just a Windows base installation need something in the tens of\nGB of disk space these days? We're not really talking about embedded\nsystems.\n\nJust the gcc etc. I've got on my Debian box is approaching a\nGB. Whatever we think about other trade-offs optimizing for disk space\ndoesn't seem very compelling.\n\n>> My attempt to amend/revert 4c2c38e800f to have it use the\n>> pre-4c2c38e800f \"make\" invocation as a fallback failed, partially\n>> because I don't have a Windows development environment, so any attempt\n>> to change it is a painfully slow round-trip to GitHub CI.\n>> Let's instead have CMake call out to the Makefile asking it what the\n>> definition of various variables lists is, rather than being forced to\n>> maintain those lists in a way that CMake can parse with regexes (which\n>> precludes anything but a giant hardcoded list).\n>> I could familiarize myself enough with CMake to do this in some\n>> CMake-native way, but that would take \"just as long as adding it to\n>> the Makefile\"[2] (I think that took me <5 minutes, but I'm several\n>> hours into fighting with CMake)\n>> So I consider this both a bugfix to the stated aims of this CMake\n>> integration, and a better way forward for having an alternate build\n>> system. I.e. If someone really does care about a having a\n>> GNU-make-less dependency for the \"vs-build\" I think this change offers\n>> a much better way forward for that.\n>\n> I don't see how relying on GNU-make is a step forward for the CMake\n> integration when it works without it now.\n\nIt's not if we assume we've got infinite man hours to maintain these\nsystems, but we don't.\n\nI have some pending patches to make various common cases when using make\nmuch better, mainly speeding up no-op runs so things in rebase --exec go\nfaster.\n\nSo far I've been submitting the parts of that that don't step on the\ntoes of this cmake integration, and realistically if I've got to\nimplement everything in lockstep in two systems I'll probably just give\nup on it.\n\nHence asking if there's some middle ground we can find here.\n\nSo you don't want to install \"make\" on Windows, but how about if we had\na script in contrib/ that generated these extractions of lists from the\nMakefile instead of doing it on the fly, we could even commit those to\nthe repo.\n\nThen I'd effectively get what I'm aiming for here, and cmake users could\njust re-run that script, and if one of them did they could push the\nresult somewhere, and others could just fetch the generated assets.\n\n> Overall I'm don't think that moving from a known set of dependencies\n> to \"build whatever C files are lying around in this directory\" is an \n> improvement.\n\nThe caveat you note above with \"foo.safe.c\" is something we've got\nalready, see various \"$(wildcard)\", \"find\" and \"git ls-files\" in the\nMakefile. So this way we'll at least be consistent. Now we'll add stray\nfiles to TAGS, apply \"coccicheck\" to them etc.\n\nSo one thing I was aiming for here was closing that gap.\n\nI do tihnk having a well understood hierarchy does help a lot, since you\ncan know that t/helpers/*.c is always one sort of thing etc.\n\n> [1] The CMake integration is currently broken for non-windows builds,\n> I've got some fixes at \n> https://github.com/phillipwood/git/tree/wip/cmake-fixes\n\n"},{"id":"440576","messageId":"40dbf962-2ccd-b4d6-7110-31317eb35e34@gmail.com","threadId":"56818","inReplyTo":"211106.86tugpfikn.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-11-06T16:49:18Z","receivedAt":"2021-11-06T16:49:24Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/11/2021 14:27, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sat, Nov 06 2021, Phillip Wood wrote:\n> \n>> Hi Ævar\n>>\n>> On 01/11/2021 19:19, Ævar Arnfjörð Bjarmason wrote:\n>>> Remove the hardcoded lists of objects in favor of using\n>>> $(wildcard). This means that every time a built-in, test tool etc. is\n>>> added we won't need to patch the top-level Makefile, except for the\n>>> few remaining cases where the asset in question would make it onto one\n>>> of our list of exceptions.\n>>> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\n>>> subdirectory, 2010-02-22) this has been relatively easy to do (and\n>>> even before that we could glob builtin-*.c). This pattern of\n>>> exhaustively enumerating files was then carried forward for\n>>> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\n>>> test-tool program, 2018-03-24).\n>>> One reason not to do this is that now a new *.c file at the\n>>> top-level\n>>> will be immediately picked up, so if a new *.c file is being worked on\n>>> \"make\" will error if it doesn't compile, whereas before that file\n>>> would need to be explicitly listed in the Makefile. I think than small\n>>> trade-off is worth it.\n>>\n>> If I need to split up some uncommitted changes into several commits\n>> and I know it is going to be fiddly to do so I will sometimes copy the\n>> original file to foo.safe.c and then edit foo.c to create each\n>> commit. Then I can easily compile and test each commit and editing the\n>> file directly is often easier than using add -p and editing the\n>> hunks. With this patch running make will fail in that case I think.\n> \n> Would being able to do something like:\n> \n>      make EXCLUDE_WILDCARD=foo.safe.c\n> \n> Satisfy this use-case?\n> \n> We can also have some DEVOPTS knob so we'll prune out files found if a\n> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n> that (and could implement it) if it was optional.\n> \n> Also note that you've got some of this already, e.g. we'll pick up *.h\n> files via a glob for \"make TAGS\", the dependency graph etc.\n\nI'd be happier using 'git ls-files' with a glob if we need to move away \nfrom listing the files explicitly rather than having to pass some \nexclude list when running make. Having seen your comments below about \nls-files/find I had a look at the Makefile and they always seem to be \nused together as \"git ls-files ... || find ...\". Doing that would mean \nwe wouldn't try to build any untracked files but still find everything \nin a tarball.\n\n>>> We could make this simpler still for the Makefile by moving\n>>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n>>> There's a few small \"while we're at it\" changes here, since I'm\n>>> touching the code in question:\n>>>    - Start splitting up the the \"Guard against the environment\"\n>>> section\n>>>      at the top, but don't move anything that exists there out to avoid\n>>>      merge conflicts\n>>>    - The $(TEST_BUILTINS_OBJS) variable was needlessly complex,\n>>> because\n>>>      it didn't have the full paths we'd pathsubst it back & forth.\n>>>    - Introduce *_SRC in addition to *_OBJ for the variable I'm\n>>>      touching. Eventually we'll want to do this for all the *.o files,\n>>>      i.e. make the *.c list a source of truth for *.o, which means we can\n>>>      e.g. use that exhaustive list for \"make TAGS\".\n>>>    - Add a missing \"curl-objs\" target. See 029bac01a87 (Makefile: add\n>>>      {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23)\n>>>      for the commit that added the rest.\n>>>    - De-indent an \"ifndef\" block, we don't usually indent their\n>>>      contents.\n>>> On the CMake changes here:\n>>>    - When CMake support was introduced in was introduced\n>>>      061c2240b1b (Introduce CMake support for configuring Git, 2020-06-12)\n>>>      there was a discussion about the maintenance burden of maintaining the\n>>>      top-level Makefile in parallel with CMakeLists.txt[1] where reviewers\n>>>      were assured that doing so would simply be a matter of adding something\n>>>      to a list in the CMake recipe.\n>>>      Between change and some recent changes of mine where the\n>>> \"vs-build\"\n>>>      job failed to a divergence between the Makefile and CMakeList.txt I\n>>>      can confidently say that that doesn't at all match reality. Even\n>>>      seemingly trivial changes to the Makefile like this one are forcing\n>>>      us to do a deep-dive into CMake internals to make forward progress\n>>>      with our main build system.\n>>\n>> My recollection is that the discussions were about not having to touch\n>> CMakeList.txt when adding new files to the build and I think that\n>> largely works. I don't think a lot of the changes you have been making\n>> recently were anticipated in that discussion.\n> \n> That's part of it, but the concern about needing to maintain two systems\n> in perpetuity was also brought up, and it not being a hard dependency,\n> having that vs-{build,test} job soft-fail on it etc. were brought up but\n> that's not what we've got now.\n> \n> In any case, having to maintain two build systems and the maintainer(s)\n> of the CMake being inactive one is the situation we're in now.\n> \n>>>    - The promised \"We can add a (continue-on-error) to vs-build job to\n>>>      make this process less of a hindrance.\" in [2] never materialized.\n>>>      Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n>>>      vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n>>>      getting the CI to pass goes.\n>>>    - The \"vs-build\" CI doesn't actually require that there be no GNU\n>>> make\n>>>      usage in the job, as it itself has a hard dependency on running a\n>>>      \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n>>>      goes we don't need a GNU-make free build system for vs-build.\n>>\n>> We need GNU-make for the ci job but an individual developer using\n>> CMake does not need GNU-make installed. On linux it is possible to\n>> build git without having make installed by using cmake and ninja [1]\n> \n> I'd tried that, but didn't know about your [1] topic. If you or someone\n> is actively willing to fix things up in CMake....\n\nI only started working on that a few days ago. I was interested to see \nif using ninja was faster for building when just a few files have been \nchanged but it doesn't make much difference unless absolutely nothing \nhas changed - I guess we don't have that many files in the grand scheme \nof things. The CMake support still lags the Makefile as currently things \nlike GIT-VERSION-FILE and git-hooks.h are not regenerated when their \ninputs change. I'm not sure if I want to commit to doing much more on \nthat at the moment.\n\n>>>    - The stated goal in 061c2240b1b of avoiding a GNU make dependency\n>>>      for developer because it requires an SDK that \"occupies around two\n>>>      gigabytes\" and \"three quarters of a gigabyte worth of Git objects\"\n>>>      hardly seems worthwhile trade-off given the above. Disk space is cheap,\n>>>      developer time required to maintain two parallel build systems isn't.\n>>\n>> That rather assumes everyone has plenty of disk space and a decent\n>> network connection.\n> \n> I realize that there's going to be a person who's got 1.5GB of disk\n> space and needs to delete a movie they've downloaded or whatever. I just\n> think that's worth it v.s. maintenance trade-off.\n> \n> Doesn't just a Windows base installation need something in the tens of\n> GB of disk space these days? We're not really talking about embedded\n> systems.\n> \n> Just the gcc etc. I've got on my Debian box is approaching a\n> GB. Whatever we think about other trade-offs optimizing for disk space\n> doesn't seem very compelling.\n> \n>>> My attempt to amend/revert 4c2c38e800f to have it use the\n>>> pre-4c2c38e800f \"make\" invocation as a fallback failed, partially\n>>> because I don't have a Windows development environment, so any attempt\n>>> to change it is a painfully slow round-trip to GitHub CI.\n>>> Let's instead have CMake call out to the Makefile asking it what the\n>>> definition of various variables lists is, rather than being forced to\n>>> maintain those lists in a way that CMake can parse with regexes (which\n>>> precludes anything but a giant hardcoded list).\n>>> I could familiarize myself enough with CMake to do this in some\n>>> CMake-native way, but that would take \"just as long as adding it to\n>>> the Makefile\"[2] (I think that took me <5 minutes, but I'm several\n>>> hours into fighting with CMake)\n>>> So I consider this both a bugfix to the stated aims of this CMake\n>>> integration, and a better way forward for having an alternate build\n>>> system. I.e. If someone really does care about a having a\n>>> GNU-make-less dependency for the \"vs-build\" I think this change offers\n>>> a much better way forward for that.\n>>\n>> I don't see how relying on GNU-make is a step forward for the CMake\n>> integration when it works without it now.\n> \n> It's not if we assume we've got infinite man hours to maintain these\n> systems, but we don't.\n> \n> I have some pending patches to make various common cases when using make\n> much better, mainly speeding up no-op runs so things in rebase --exec go\n> faster.\n> \n> So far I've been submitting the parts of that that don't step on the\n> toes of this cmake integration, and realistically if I've got to\n> implement everything in lockstep in two systems I'll probably just give\n> up on it.\n> \n> Hence asking if there's some middle ground we can find here.\n\nWhat is it about listing the input files explicitly that slows things \ndown? Surely it's faster than globbing the filesystem.\n\n> So you don't want to install \"make\" on Windows, but how about if we had\n> a script in contrib/ that generated these extractions of lists from the\n> Makefile instead of doing it on the fly, we could even commit those to\n> the repo.\n\nIf I understand there would be some make rule that generates a list of \ndependencies and we'd commit that list to the repo and consume it in \nCMakeLists.txt. I'd be fine with that\n\n> Then I'd effectively get what I'm aiming for here, and cmake users could\n> just re-run that script, and if one of them did they could push the\n> result somewhere, and others could just fetch the generated assets.\n> \n>> Overall I'm don't think that moving from a known set of dependencies\n>> to \"build whatever C files are lying around in this directory\" is an\n>> improvement.\n> \n> The caveat you note above with \"foo.safe.c\" is something we've got\n> already, see various \"$(wildcard)\", \"find\" and \"git ls-files\" in the\n> Makefile. So this way we'll at least be consistent. Now we'll add stray\n> files to TAGS, apply \"coccicheck\" to them etc.\n\nI'm not so worried about those other targets, but being able to reliably \nbuild and test git with some cruft lying around is useful though. I'm \nstill not entirely sure what the motivation for this change is (adding \nnew files is not that common) but I think using the established \"git \nls-files || find\" pattern would be a good way of globbing without \npicking up rubbish if there is a compelling reason to drop the lists.\n\nBest Wishes\n\nPhillip\n\n> So one thing I was aiming for here was closing that gap.\n> \n> I do tihnk having a well understood hierarchy does help a lot, since you\n> can know that t/helpers/*.c is always one sort of thing etc.\n> \n>> [1] The CMake integration is currently broken for non-windows builds,\n>> I've got some fixes at\n>> https://github.com/phillipwood/git/tree/wip/cmake-fixes\n> \n"},{"id":"440615","messageId":"211106.86pmrdezts.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"40dbf962-2ccd-b4d6-7110-31317eb35e34@gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-06T21:13:51Z","receivedAt":"2021-11-06T21:31:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Nov 06 2021, Phillip Wood wrote:\n\n> On 06/11/2021 14:27, Ævar Arnfjörð Bjarmason wrote:\n>> On Sat, Nov 06 2021, Phillip Wood wrote:\n>> \n>>> Hi Ævar\n>>>\n>>> On 01/11/2021 19:19, Ævar Arnfjörð Bjarmason wrote:\n>>>> Remove the hardcoded lists of objects in favor of using\n>>>> $(wildcard). This means that every time a built-in, test tool etc. is\n>>>> added we won't need to patch the top-level Makefile, except for the\n>>>> few remaining cases where the asset in question would make it onto one\n>>>> of our list of exceptions.\n>>>> Ever since 81b50f3ce40 (Move 'builtin-*' into a 'builtin/'\n>>>> subdirectory, 2010-02-22) this has been relatively easy to do (and\n>>>> even before that we could glob builtin-*.c). This pattern of\n>>>> exhaustively enumerating files was then carried forward for\n>>>> e.g. TEST_BUILTINS_OBJS in efd71f8913a (t/helper: add an empty\n>>>> test-tool program, 2018-03-24).\n>>>> One reason not to do this is that now a new *.c file at the\n>>>> top-level\n>>>> will be immediately picked up, so if a new *.c file is being worked on\n>>>> \"make\" will error if it doesn't compile, whereas before that file\n>>>> would need to be explicitly listed in the Makefile. I think than small\n>>>> trade-off is worth it.\n>>>\n>>> If I need to split up some uncommitted changes into several commits\n>>> and I know it is going to be fiddly to do so I will sometimes copy the\n>>> original file to foo.safe.c and then edit foo.c to create each\n>>> commit. Then I can easily compile and test each commit and editing the\n>>> file directly is often easier than using add -p and editing the\n>>> hunks. With this patch running make will fail in that case I think.\n>> Would being able to do something like:\n>>      make EXCLUDE_WILDCARD=foo.safe.c\n>> Satisfy this use-case?\n>> We can also have some DEVOPTS knob so we'll prune out files found if\n>> a\n>> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n>> that (and could implement it) if it was optional.\n>> Also note that you've got some of this already, e.g. we'll pick up\n>> *.h\n>> files via a glob for \"make TAGS\", the dependency graph etc.\n>\n> I'd be happier using 'git ls-files' with a glob if we need to move\n> away from listing the files explicitly rather than having to pass some \n> exclude list when running make. Having seen your comments below about\n> ls-files/find I had a look at the Makefile and they always seem to be \n> used together as \"git ls-files ... || find ...\". Doing that would mean\n> we wouldn't try to build any untracked files but still find everything \n> in a tarball.\n\nYes we could use the $(wildcard) in a tarball, the issue of picking up\nuntracked files is a dev-checkout-only issue, no?\n\n>>> [...]\n>>> My recollection is that the discussions were about not having to touch\n>>> CMakeList.txt when adding new files to the build and I think that\n>>> largely works. I don't think a lot of the changes you have been making\n>>> recently were anticipated in that discussion.\n>> That's part of it, but the concern about needing to maintain two\n>> systems\n>> in perpetuity was also brought up, and it not being a hard dependency,\n>> having that vs-{build,test} job soft-fail on it etc. were brought up but\n>> that's not what we've got now.\n>> In any case, having to maintain two build systems and the\n>> maintainer(s)\n>> of the CMake being inactive one is the situation we're in now.\n>> \n>>>>    - The promised \"We can add a (continue-on-error) to vs-build job to\n>>>>      make this process less of a hindrance.\" in [2] never materialized.\n>>>>      Since 4c2c38e800f (ci: modification of main.yml to use cmake for\n>>>>      vs-build job, 2020-06-26) got a hard dependency on CMake as far as\n>>>>      getting the CI to pass goes.\n>>>>    - The \"vs-build\" CI doesn't actually require that there be no GNU\n>>>> make\n>>>>      usage in the job, as it itself has a hard dependency on running a\n>>>>      \"make -n artifacts-tar\" command. So as far as any vs-specific special-sauce\n>>>>      goes we don't need a GNU-make free build system for vs-build.\n>>>\n>>> We need GNU-make for the ci job but an individual developer using\n>>> CMake does not need GNU-make installed. On linux it is possible to\n>>> build git without having make installed by using cmake and ninja [1]\n>> I'd tried that, but didn't know about your [1] topic. If you or\n>> someone\n>> is actively willing to fix things up in CMake....\n>\n> I only started working on that a few days ago. I was interested to see\n> if using ninja was faster for building when just a few files have been \n> changed but it doesn't make much difference unless absolutely nothing\n> has changed - I guess we don't have that many files in the grand\n> scheme of things. The CMake support still lags the Makefile as\n> currently things like GIT-VERSION-FILE and git-hooks.h are not\n> regenerated when their inputs change. I'm not sure if I want to commit\n> to doing much more on that at the moment.\n\nI suspect that any other build system is faster because it doesn't\nactually do what our Makefile does, which is something I've been trying\nto slowly fix.\n\nE.g. getting rid of the FORCE dependencies we've got. I submitted a\nseries just now to do some prep work for that:\nhttps://lore.kernel.org/git/cover-00.16-00000000000-20211106T205717Z-avarab@gmail.com/\n\nThe performance of that:\n\n0 $ hyperfine --warmup 3 -L v master,next,avar/Makefile-perl-python-shell-quote-various-fixes,avar/Makefile-get-rid-of-FORCE-rules-for-gmake-4.2-dependency -p 'git checkout {v} && make -j8 all' 'echo {v} && make -j8 all'\nBenchmark #1: echo master && make -j8 all\n  Time (mean ± σ):     127.7 ms ±  40.8 ms    [User: 111.5 ms, System: 17.4 ms]\n  Range (min … max):   113.2 ms … 243.8 ms    10 runs\n \noptions.\n \nBenchmark #2: echo next && make -j8 all\n  Time (mean ± σ):     114.9 ms ±   0.9 ms    [User: 105.6 ms, System: 23.1 ms]\n  Range (min … max):   113.5 ms … 116.3 ms    11 runs\n \nBenchmark #3: echo avar/Makefile-perl-python-shell-quote-various-fixes && make -j8 all\n  Time (mean ± σ):     121.6 ms ±  16.8 ms    [User: 107.9 ms, System: 22.2 ms]\n  Range (min … max):   113.6 ms … 167.4 ms    10 runs\n \nBenchmark #4: echo avar/Makefile-get-rid-of-FORCE-rules-for-gmake-4.2-dependency && make -j8 all\n  Time (mean ± σ):      71.4 ms ±  17.2 ms    [User: 61.7 ms, System: 11.1 ms]\n  Range (min … max):    64.2 ms … 131.9 ms    15 runs\n \nSummary\n  'echo avar/Makefile-get-rid-of-FORCE-rules-for-gmake-4.2-dependency && make -j8 all' ran\n    1.61 ± 0.39 times faster than 'echo next && make -j8 all'\n    1.70 ± 0.47 times faster than 'echo avar/Makefile-perl-python-shell-quote-various-fixes && make -j8 all'\n    1.79 ± 0.72 times faster than 'echo master && make -j8 all'\n\nI.e. so far I haven't really optimized anything much, but the next step\nafter that series above speeds it up by almost 2x for a noop run, and\nthere's other low-hanging fruit after that that I've got in some WIP\ncode somewhere. I think I managed to get it down to the 10-20ms range\nfor noop runs in one of those.\n\n>>> [...]\n>>>> My attempt to amend/revert 4c2c38e800f to have it use the\n>>>> pre-4c2c38e800f \"make\" invocation as a fallback failed, partially\n>>>> because I don't have a Windows development environment, so any attempt\n>>>> to change it is a painfully slow round-trip to GitHub CI.\n>>>> Let's instead have CMake call out to the Makefile asking it what the\n>>>> definition of various variables lists is, rather than being forced to\n>>>> maintain those lists in a way that CMake can parse with regexes (which\n>>>> precludes anything but a giant hardcoded list).\n>>>> I could familiarize myself enough with CMake to do this in some\n>>>> CMake-native way, but that would take \"just as long as adding it to\n>>>> the Makefile\"[2] (I think that took me <5 minutes, but I'm several\n>>>> hours into fighting with CMake)\n>>>> So I consider this both a bugfix to the stated aims of this CMake\n>>>> integration, and a better way forward for having an alternate build\n>>>> system. I.e. If someone really does care about a having a\n>>>> GNU-make-less dependency for the \"vs-build\" I think this change offers\n>>>> a much better way forward for that.\n>>>\n>>> I don't see how relying on GNU-make is a step forward for the CMake\n>>> integration when it works without it now.\n>> It's not if we assume we've got infinite man hours to maintain these\n>> systems, but we don't.\n>> I have some pending patches to make various common cases when using\n>> make\n>> much better, mainly speeding up no-op runs so things in rebase --exec go\n>> faster.\n>> So far I've been submitting the parts of that that don't step on the\n>> toes of this cmake integration, and realistically if I've got to\n>> implement everything in lockstep in two systems I'll probably just give\n>> up on it.\n>> Hence asking if there's some middle ground we can find here.\n>\n> What is it about listing the input files explicitly that slows things\n> down? Surely it's faster than globbing the filesystem.\n\nIt doesn't slow things down. I'd like to get rid of these big hardcoded\nlists to make it easier to add new [test] commands, anything that cuts\ndown on the \"modify these N files, copy/pasting the last similar commit\"\nhelps with that.\n\nAnd we've got churn related to these lists being unsorted, being\nre-sorted, and occasional tiresome merge conflicts etc.\n\nThe initial reason I finished this up was because I had a WIP patch to\nadd a \"sort assertion\" for these lists, per the discussion here:\nhttps://lore.kernel.org/git/87mtv2dk18.fsf@evledraar.gmail.com/\n\nBut I realized instead of doing that we could just use $(wildcard)\ninstead.\n\n>> So you don't want to install \"make\" on Windows, but how about if we had\n>> a script in contrib/ that generated these extractions of lists from the\n>> Makefile instead of doing it on the fly, we could even commit those to\n>> the repo.\n>\n> If I understand there would be some make rule that generates a list of\n> dependencies and we'd commit that list to the repo and consume it in \n> CMakeLists.txt. I'd be fine with that\n\nOk that certainly helps & is a viable workaround.\n\n>> Then I'd effectively get what I'm aiming for here, and cmake users could\n>> just re-run that script, and if one of them did they could push the\n>> result somewhere, and others could just fetch the generated assets.\n>> \n>>> Overall I'm don't think that moving from a known set of dependencies\n>>> to \"build whatever C files are lying around in this directory\" is an\n>>> improvement.\n>> The caveat you note above with \"foo.safe.c\" is something we've got\n>> already, see various \"$(wildcard)\", \"find\" and \"git ls-files\" in the\n>> Makefile. So this way we'll at least be consistent. Now we'll add stray\n>> files to TAGS, apply \"coccicheck\" to them etc.\n>\n> I'm not so worried about those other targets, but being able to\n> reliably build and test git with some cruft lying around is useful\n> though. I'm still not entirely sure what the motivation for this\n> change is (adding new files is not that common) but I think using the\n> established \"git ls-files || find\" pattern would be a good way of\n> globbing without picking up rubbish if there is a compelling reason to\n> drop the lists.\n\nI would like to get rid of those ls-files or whatever globs entirely,\nwhether that's by moving everything to wildcards or having them consume\nthe hardcoded lists doesn't really matter.\n\nBut it's a a wart that we're e.g. running coccicheck on one globbed set\nof files, and compile a hardcoded list of files. Partially it's because\nwe conflate \"list of known files\" with \"here's what we'd like to compile\non this platform\".\n"},{"id":"440768","messageId":"xmqqtugl102l.fsf@gitster.g","threadId":"56818","inReplyTo":"40dbf962-2ccd-b4d6-7110-31317eb35e34@gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-09T21:38:42Z","receivedAt":"2021-11-09T21:38:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> We can also have some DEVOPTS knob so we'll prune out files found if\n>> a\n>> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n>> that (and could implement it) if it was optional.\n>> Also note that you've got some of this already, e.g. we'll pick up\n>> *.h\n>> files via a glob for \"make TAGS\", the dependency graph etc.\n>\n> I'd be happier using 'git ls-files' with a glob if we need to move\n> away from listing the files explicitly rather than having to pass some \n> exclude list when running make. Having seen your comments below about\n> ls-files/find I had a look at the Makefile and they always seem to be \n> used together as \"git ls-files ... || find ...\". Doing that would mean\n> we wouldn't try to build any untracked files but still find everything \n> in a tarball.\n\nI've been quiet on this topic because honestly I do not find the\npros-and-cons favourable for more use of wildcards [*].  Tools like\ngit (especially .gitignore) and Makefile are to help users to be\nsafely sloppy by ensuring that random crufts the users may create in\nthe working tree for their convenience are not picked up by default\nunless the project to consciously expresses the desire to use them.\n\nAllowing to be sloppy while maintaining Makefile feels like a false\neconomy, and having to paper it over by adding exceptions and\nforcing developers to learn such ad-hoc rules even more so.\n\n    Side note: TAGS generation and some other minor things may use\n    $(wildcard) and can throw tokens in cruft files in the output,\n    which is not ideal, but the damage is local.  We cannot treat\n    that the same as building binaries and tarballs.\n\nIf we could use \"git ls-files\" consistently, that may make it\nsomewhat safer; you'd at least need to \"git add\" a new file before\nit gets into the picture.  But it would be impossible, because we\nneed to be able to bootstrap Git from a tarball extract.\n\n>>>> We could make this simpler still for the Makefile by moving\n>>>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>>>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>>>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>>>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n\nAnd it is not quite clear to me why we want to even more pile\nworkaround like this on top.  This also is to paper over the mistake\nof being sloppy and using $(wildcard), which makes it unable to\ndistinguish, among the ones that match a pattern, between FOO_OBJS\nand BAR_OBJS, no?  Moving files around in the working tree to group\nrelated things together is a good thing, and it has been a good move\nto separate built-ins and library-ish parts into different\ndirectories.  But the above does not sound like it.\n\nOther than \"these source files may or may not be compiled\ndepending\", what trait do files in conditional-src/ share, other\nthan \"dividing them into a separate category makes it simpler to\nwrite Makefile using $(wildcard)\"?  I do not think of a good one.\n\nThe only time I found that the large list of files in Makefile was\nproblematic was *NOT* when multiple topics added, renamed or removed\nthe files (it is pretty much bog standard merge conflicts that do\nnot happen very often to begin with).  It is when this kind of\n\"large scale refactoring\" for the sake of refactoring happens.\n\n> I'm not so worried about those other targets, but being able to\n> reliably build and test git with some cruft lying around is useful\n> though. I'm still not entirely sure what the motivation for this\n> change is (adding new files is not that common) but I think using the\n> established \"git ls-files || find\" pattern would be a good way of\n> globbing without picking up rubbish if there is a compelling reason to\n> drop the lists.\n\nYes.\n"},{"id":"440835","messageId":"nycvar.QRO.7.76.6.2111101332130.21127@tvgsbejvaqbjf.bet","threadId":"56818","inReplyTo":"xmqqtugl102l.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-10T12:39:41Z","receivedAt":"2021-11-10T12:40:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 9 Nov 2021, Junio C Hamano wrote:\n\n> Allowing to be sloppy while maintaining Makefile feels like a false\n> economy, and having to paper it over by adding exceptions and\n> forcing developers to learn such ad-hoc rules even more so.\n\nIf you ever needed another opinion to back you up on this: I fully agree.\n\n> If we could use \"git ls-files\" consistently, that may make it\n> somewhat safer; you'd at least need to \"git add\" a new file before\n> it gets into the picture.  But it would be impossible, because we\n> need to be able to bootstrap Git from a tarball extract.\n\nIndeed, the ability to build from a `.tar` extract is important. That's\nwhy we were careful to use `ls-files` in `LIB_H` and in\n`FIND_SOURCE_FILE`, falling back on using `find` if the `ls-files` call\nfailed.\n\nAnd to be honest, even `LIB_H` and `FIND_SOURCE_FILE` would quite\npotentially better be hard-coded (with a CI check to ensure that they're\nup to date).\n\nCiao,\nDscho\n"},{"id":"440840","messageId":"211110.86h7cki0uo.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"nycvar.QRO.7.76.6.2111101332130.21127@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-10T13:21:32Z","receivedAt":"2021-11-10T13:42:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 10 2021, Johannes Schindelin wrote:\n\n> Hi Junio,\n>\n> On Tue, 9 Nov 2021, Junio C Hamano wrote:\n>\n>> Allowing to be sloppy while maintaining Makefile feels like a false\n>> economy, and having to paper it over by adding exceptions and\n>> forcing developers to learn such ad-hoc rules even more so.\n>\n> If you ever needed another opinion to back you up on this: I fully agree.\n\nI could go either way on that, but in terms of Makefile maintenance it\ndoes suck a lot less to pick one or the other.\n\nA (I realize, unstated) eventual goal I had was to move these wildcard\ndeclarations to some common list you can include from various Makefiles,\ncurrently we've got dependency bugs in e.g. Makefile &\nDocumentation/Makefile interaction.\n\nIf we're not OK with $(wildcard) as a pattern that would mean changing\nall of these to hardcoded (in some cases quite big) lists somewhere:\n    \n    $ git -P grep -E '^[^~]+\\$\\(wildcard.+\\*' ':!git-gui' ':!gitk-git' ':!contrib'\n    Documentation/Makefile:         $(wildcard git-*.txt))\n    Documentation/Makefile:HOWTO_TXT += $(wildcard howto/*.txt)\n    Documentation/Makefile:DOC_DEP_TXT += $(wildcard *.txt)\n    Documentation/Makefile:DOC_DEP_TXT += $(wildcard config/*.txt)\n    Documentation/Makefile:API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technical/api-index.txt, $(wildcard technical/api-*.txt)))\n    Documentation/Makefile:mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)\n    Documentation/Makefile:%.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n    Makefile:command-list.h: $(wildcard Documentation/git*.txt)\n    Makefile:POFILES := $(wildcard po/*.po)\n    Makefile:LIB_PERL := $(wildcard perl/Git.pm perl/Git/*.pm perl/Git/*/*.pm perl/Git/*/*/*.pm)\n    Makefile:LIB_CPAN := $(wildcard perl/FromCPAN/*.pm perl/FromCPAN/*/*.pm)\n    Makefile:coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci)))\n    Makefile:coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci))\n    t/Makefile:T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))\n    t/Makefile:TGITWEB = $(sort $(wildcard t95[0-9][0-9]-*.sh))\n    t/Makefile:THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))\n    t/Makefile:TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))\n    t/Makefile:CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))\n    t/interop/Makefile:T = $(sort $(wildcard i[0-9][0-9][0-9][0-9]-*.sh))\n\nWhat do you & Junio think about that?\n\nI don't really mind either way, as long as I stop running into\noccasional bugs where I need to run \"git clean -dxf\" because the\nMakeefile was too stupid to properly manage its dependencies.\n\n>> If we could use \"git ls-files\" consistently, that may make it\n>> somewhat safer; you'd at least need to \"git add\" a new file before\n>> it gets into the picture.  But it would be impossible, because we\n>> need to be able to bootstrap Git from a tarball extract.\n>\n> Indeed, the ability to build from a `.tar` extract is important. That's\n> why we were careful to use `ls-files` in `LIB_H` and in\n> `FIND_SOURCE_FILE`, falling back on using `find` if the `ls-files` call\n> failed.\n\nWhy would you need any of that to *build* from a .tar extract? I think\nwe should remove that LIB_H thing entirely.\n\nIts only purpose is to support someone who:\n\n 1. Wants to do an *incremental* build, not a \"build from tar\". I.e. you\n    build already, changed a header, and now you want to not over-build\n    again.\n\n    Your compiler is perfectly capable of locating headers in an -I dir\n    for you.\n\n 2. Doesn't have gcc or clang installed. Note \"installed\", not to\n    build.\n\n    Well, currently we require you to build with those to use .depends &\n    COMPUTE_HEADER_DEPENDENCIES, but that's an easily fixable\n    implementation detail.\n\n    We can easily make the .depend files with gcc/clang and build with\n    another compiler. I had a 5-10 line local change at some point to do\n    that.\n\n 3. Doesn't find it acceptable to have a fallback of just a glob like\n    \"**.h\" for that \"depends\" target.\n\n    I.e. we'd over-rebuild if you dropped in a new *.h we're not\n    actually using into your extracted tarball, but really, who cares?\n\n 4. Wants to run \"make hdr-check\" or \"make pot\", both of which I think\n    are OK to say \"you need to run this on a box that has .depends (or\n    in the case of *.pot, we can use a greedier glob).\n\n> And to be honest, even `LIB_H` and `FIND_SOURCE_FILE` would quite\n> potentially better be hard-coded (with a CI check to ensure that they're\n> up to date).\n\nThat would be a bug, just because I don't build on Windows doesn't mean\nthat I wouldn't like \"make TAGS coccicheck\" to find compat/win32/ at\nall.\n\nIt doesn't do that now for a different reason, but that's a bug that\nshould be fixed.\n"},{"id":"440848","messageId":"nycvar.QRO.7.76.6.2111101547120.21127@tvgsbejvaqbjf.bet","threadId":"56818","inReplyTo":"211110.86h7cki0uo.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-10T14:59:47Z","receivedAt":"2021-11-10T15:00:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Wed, 10 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> If we're not OK with $(wildcard) as a pattern that would mean changing\n> all of these to hardcoded (in some cases quite big) lists somewhere:\n>\n>     [...]\n\nNo, it would only mean changing these instances if we have a concrete\nneed. I fail to see a concrete need.\n\nThat does not mean that we should make the situation even worse by\nconverting currently hard-coded lists to wildcards. There is, once again,\nno concrete need for that, and there is the good reason Junio brought up\nagainst such a churn: it is too sloppy.\n\n> [...] I think we should remove that LIB_H thing entirely.\n\nI think we should take a break from refactoring code where it is unclear\nwhat purpose the refactoring serves.\n\n> > And to be honest, even `LIB_H` and `FIND_SOURCE_FILE` would quite\n> > potentially better be hard-coded (with a CI check to ensure that\n> > they're up to date).\n>\n> That would be a bug, just because I don't build on Windows doesn't mean\n> that I wouldn't like \"make TAGS coccicheck\" to find compat/win32/ at\n> all.\n\nTalking about `coccicheck` in the context of the discussion whether we\nshould make sweeping changes in our Makefiles, all while we're supposedly\nin the -rc phase, strikes me as a distant tangent of a distant tangent.\n\nCiao,\nJohannes\n"},{"id":"440851","messageId":"211110.86mtmcgeyt.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"nycvar.QRO.7.76.6.2111101547120.21127@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-10T15:58:59Z","receivedAt":"2021-11-10T16:20:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 10 2021, Johannes Schindelin wrote:\n\n> Hi Ævar,\n>\n> On Wed, 10 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n>\n>> If we're not OK with $(wildcard) as a pattern that would mean changing\n>> all of these to hardcoded (in some cases quite big) lists somewhere:\n>>\n>>     [...]\n>\n> No, it would only mean changing these instances if we have a concrete\n> need. I fail to see a concrete need.\n>\n> That does not mean that we should make the situation even worse by\n> converting currently hard-coded lists to wildcards. There is, once again,\n> no concrete need for that, and there is the good reason Junio brought up\n> against such a churn: it is too sloppy.\n\nI don't it's sloppy. It's just a different approach. It's also an\napproach we use now. Add a new build in and your addition in t/*.sh and\nDocumentation/*.txt will be picked up & built. We just won't pick up the\n*.h or builtin/*.c implicitly.\n\nSo whether we need to do this now is one thing, but saying it's a big\nchange in workflow seems to be rather exaggerated.\n\n>> [...] I think we should remove that LIB_H thing entirely.\n>\n> I think we should take a break from refactoring code where it is unclear\n> what purpose the refactoring serves.\n\nI'm not advocating ripping LIB_H out right now, and have not submitted\nany patches to do so.\n\nI'm asking you a follow-up question about your claim that LIB_H is\nneeded for building from a tarball. I don't think it is, but perhaps I'm\nmissing something.\n\nIt would be useful to get an answer to that for the list records, so\nthat while it's fresh in your mind we can get an answer one way or the\nother.\n\nI agree there's no a strong reason to change it now, but being able to\ndo so in the future might be useful.\n\nAt that point someone will probably dig up this thread. Whether \"do we\nneed LIB_H for what Johannes suggested?\" is a dead end or not I'll leave\nto you.\n\n>> > And to be honest, even `LIB_H` and `FIND_SOURCE_FILE` would quite\n>> > potentially better be hard-coded (with a CI check to ensure that\n>> > they're up to date).\n>>\n>> That would be a bug, just because I don't build on Windows doesn't mean\n>> that I wouldn't like \"make TAGS coccicheck\" to find compat/win32/ at\n>> all.\n>\n> Talking about `coccicheck` in the context of the discussion whether we\n> should make sweeping changes in our Makefiles, all while we're supposedly\n> in the -rc phase, strikes me as a distant tangent of a distant tangent.\n\nFor the past few releases I think I've probably submitted more\nlast-minute rc fixes than most. In my case it's mostly a matter of\nstarting some builds and waiting for them to complete:\nhttps://xkcd.com/303/\n\nSo yeah, I think we should focus around release time, but saying that\nany other ongoing discussion is a needless distraction seems like a\nbridge too far.\n\nThat being said the reason I'm submitting these sorts of fixes now is\ndirectly related to making rc testing easier.\n\nI test on some obnoxiously slow VMs or otherwise limited computers on\nthe GCC farm. When something breaks between releases having each step of\na bisect take 30m or 60m makes a big difference.\n\nSo having a Makefile that doesn't over-build stuff is important to\nrelease testing.\n\nBut getting that across has been frustrating at times. I've pretty much\nstopped testing on AIX because my few-lines of patches to the Makefile\nto make that drastically easier were categorically rejected. The\nresponse to some other things like over-building <xyz> has been\nsomewhere between \"I don't see why you care, my computer is fast enough\"\nand \"why don't you port ccache to <90s era *nix OS that barely compiles\nHello World without issues\".\n"},{"id":"446626","messageId":"220121.86o845jnvv.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"xmqqtugl102l.fsf@gitster.g","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-21T12:01:40Z","receivedAt":"2022-01-21T12:04:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 09 2021, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n>>> We can also have some DEVOPTS knob so we'll prune out files found if\n>>> a\n>>> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n>>> that (and could implement it) if it was optional.\n>>> Also note that you've got some of this already, e.g. we'll pick up\n>>> *.h\n>>> files via a glob for \"make TAGS\", the dependency graph etc.\n>>\n>> I'd be happier using 'git ls-files' with a glob if we need to move\n>> away from listing the files explicitly rather than having to pass some \n>> exclude list when running make. Having seen your comments below about\n>> ls-files/find I had a look at the Makefile and they always seem to be \n>> used together as \"git ls-files ... || find ...\". Doing that would mean\n>> we wouldn't try to build any untracked files but still find everything \n>> in a tarball.\n>\n> I've been quiet on this topic because honestly I do not find the\n> pros-and-cons favourable for more use of wildcards [*].  Tools like\n> git (especially .gitignore) and Makefile are to help users to be\n> safely sloppy by ensuring that random crufts the users may create in\n> the working tree for their convenience are not picked up by default\n> unless the project to consciously expresses the desire to use them.\n>\n> Allowing to be sloppy while maintaining Makefile feels like a false\n> economy, and having to paper it over by adding exceptions and\n> forcing developers to learn such ad-hoc rules even more so.\n>\n>     Side note: TAGS generation and some other minor things may use\n>     $(wildcard) and can throw tokens in cruft files in the output,\n>     which is not ideal, but the damage is local.  We cannot treat\n>     that the same as building binaries and tarballs.\n>\n> If we could use \"git ls-files\" consistently, that may make it\n> somewhat safer; you'd at least need to \"git add\" a new file before\n> it gets into the picture.  But it would be impossible, because we\n> need to be able to bootstrap Git from a tarball extract.\n>\n>>>>> We could make this simpler still for the Makefile by moving\n>>>>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>>>>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>>>>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>>>>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n>\n> And it is not quite clear to me why we want to even more pile\n> workaround like this on top.  This also is to paper over the mistake\n> of being sloppy and using $(wildcard), which makes it unable to\n> distinguish, among the ones that match a pattern, between FOO_OBJS\n> and BAR_OBJS, no?  Moving files around in the working tree to group\n> related things together is a good thing, and it has been a good move\n> to separate built-ins and library-ish parts into different\n> directories.  But the above does not sound like it.\n>\n> Other than \"these source files may or may not be compiled\n> depending\", what trait do files in conditional-src/ share, other\n> than \"dividing them into a separate category makes it simpler to\n> write Makefile using $(wildcard)\"?  I do not think of a good one.\n>\n> The only time I found that the large list of files in Makefile was\n> problematic was *NOT* when multiple topics added, renamed or removed\n> the files (it is pretty much bog standard merge conflicts that do\n> not happen very often to begin with).  It is when this kind of\n> \"large scale refactoring\" for the sake of refactoring happens.\n>\n>> I'm not so worried about those other targets, but being able to\n>> reliably build and test git with some cruft lying around is useful\n>> though. I'm still not entirely sure what the motivation for this\n>> change is (adding new files is not that common) but I think using the\n>> established \"git ls-files || find\" pattern would be a good way of\n>> globbing without picking up rubbish if there is a compelling reason to\n>> drop the lists.\n>\n> Yes.\n\nReviewing the reftable coverity topic I was reminded of this\npatch. I.e. in it we have this fix:\nhttps://lore.kernel.org/git/xmqqtugl102l.fsf@gitster.g/\n\nWhich shows another advantage of using this sort of $(wildcard) pattern,\ni.e. if we had this:\n\t\n\tdiff --git a/Makefile b/Makefile\n\tindex 5580859afdb..48ea18afa53 100644\n\t--- a/Makefile\n\t+++ b/Makefile\n\t@@ -2443,33 +2443,9 @@ XDIFF_OBJS += xdiff/xutils.o\n\t .PHONY: xdiff-objs\n\t xdiff-objs: $(XDIFF_OBJS)\n\t \n\t-REFTABLE_OBJS += reftable/basics.o\n\t-REFTABLE_OBJS += reftable/error.o\n\t-REFTABLE_OBJS += reftable/block.o\n\t-REFTABLE_OBJS += reftable/blocksource.o\n\t-REFTABLE_OBJS += reftable/iter.o\n\t-REFTABLE_OBJS += reftable/publicbasics.o\n\t-REFTABLE_OBJS += reftable/merged.o\n\t-REFTABLE_OBJS += reftable/pq.o\n\t-REFTABLE_OBJS += reftable/reader.o\n\t-REFTABLE_OBJS += reftable/record.o\n\t-REFTABLE_OBJS += reftable/refname.o\n\t-REFTABLE_OBJS += reftable/generic.o\n\t-REFTABLE_OBJS += reftable/stack.o\n\t-REFTABLE_OBJS += reftable/tree.o\n\t-REFTABLE_OBJS += reftable/writer.o\n\t-\n\t-REFTABLE_TEST_OBJS += reftable/basics_test.o\n\t-REFTABLE_TEST_OBJS += reftable/block_test.o\n\t-REFTABLE_TEST_OBJS += reftable/dump.o\n\t-REFTABLE_TEST_OBJS += reftable/merged_test.o\n\t-REFTABLE_TEST_OBJS += reftable/pq_test.o\n\t-REFTABLE_TEST_OBJS += reftable/record_test.o\n\t-REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n\t-REFTABLE_TEST_OBJS += reftable/refname_test.o\n\t-REFTABLE_TEST_OBJS += reftable/stack_test.o\n\t-REFTABLE_TEST_OBJS += reftable/test_framework.o\n\t-REFTABLE_TEST_OBJS += reftable/tree_test.o\n\t+REFTABLE_SOURCES = $(wildcard reftable/*.c)\n\t+REFTABLE_OBJS += $(filter-out test,$(REFTABLE_SOURCES:%.c=%.o))\n\t+REFTABLE_TEST_OBJS += $(filter test,$(REFTABLE_SOURCES:%.c=%.o))\n\t \n\t TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n\nWe'd have a shorter Makefile, not need to manually maintain the list,\nand we'd have been getting linker errors all along on the dead code\n(just showing one of many here):\n\n\t$ make\n\t[...]\n\t/usr/bin/ld: reftable/libreftable.a(generic.o): in function `reftable_table_seek_ref':\n\t/home/avar/g/git/reftable/generic.c:17: multiple definition of `reftable_table_seek_ref'; reftable/libreftable.a(reftable.o):/home/avar/g/git/reftable/reftable.c:17: first defined here\n\t[...]\n\tclang: error: linker command failed with exit code 1 (use -v to see invocation)\n\tmake: *** [Makefile:2925: t/helper/test-tool] Error 1\n\tmake: Target 'all' not remade because of errors.\n"},{"id":"446634","messageId":"4caf3c95-c19c-80c6-daea-84dcb3ec9663@gmail.com","threadId":"56818","inReplyTo":"220121.86o845jnvv.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-01-21T17:14:23Z","receivedAt":"2022-01-21T17:14:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nOn 21/01/2022 12:01, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Nov 09 2021, Junio C Hamano wrote:\n> \n>> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>\n>>>> We can also have some DEVOPTS knob so we'll prune out files found if\n>>>> a\n>>>> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n>>>> that (and could implement it) if it was optional.\n>>>> Also note that you've got some of this already, e.g. we'll pick up\n>>>> *.h\n>>>> files via a glob for \"make TAGS\", the dependency graph etc.\n>>>\n>>> I'd be happier using 'git ls-files' with a glob if we need to move\n>>> away from listing the files explicitly rather than having to pass some\n>>> exclude list when running make. Having seen your comments below about\n>>> ls-files/find I had a look at the Makefile and they always seem to be\n>>> used together as \"git ls-files ... || find ...\". Doing that would mean\n>>> we wouldn't try to build any untracked files but still find everything\n>>> in a tarball.\n>>\n>> I've been quiet on this topic because honestly I do not find the\n>> pros-and-cons favourable for more use of wildcards [*].  Tools like\n>> git (especially .gitignore) and Makefile are to help users to be\n>> safely sloppy by ensuring that random crufts the users may create in\n>> the working tree for their convenience are not picked up by default\n>> unless the project to consciously expresses the desire to use them.\n>>\n>> Allowing to be sloppy while maintaining Makefile feels like a false\n>> economy, and having to paper it over by adding exceptions and\n>> forcing developers to learn such ad-hoc rules even more so.\n>>\n>>      Side note: TAGS generation and some other minor things may use\n>>      $(wildcard) and can throw tokens in cruft files in the output,\n>>      which is not ideal, but the damage is local.  We cannot treat\n>>      that the same as building binaries and tarballs.\n>>\n>> If we could use \"git ls-files\" consistently, that may make it\n>> somewhat safer; you'd at least need to \"git add\" a new file before\n>> it gets into the picture.  But it would be impossible, because we\n>> need to be able to bootstrap Git from a tarball extract.\n>>\n>>>>>> We could make this simpler still for the Makefile by moving\n>>>>>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>>>>>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>>>>>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>>>>>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n>>\n>> And it is not quite clear to me why we want to even more pile\n>> workaround like this on top.  This also is to paper over the mistake\n>> of being sloppy and using $(wildcard), which makes it unable to\n>> distinguish, among the ones that match a pattern, between FOO_OBJS\n>> and BAR_OBJS, no?  Moving files around in the working tree to group\n>> related things together is a good thing, and it has been a good move\n>> to separate built-ins and library-ish parts into different\n>> directories.  But the above does not sound like it.\n>>\n>> Other than \"these source files may or may not be compiled\n>> depending\", what trait do files in conditional-src/ share, other\n>> than \"dividing them into a separate category makes it simpler to\n>> write Makefile using $(wildcard)\"?  I do not think of a good one.\n>>\n>> The only time I found that the large list of files in Makefile was\n>> problematic was *NOT* when multiple topics added, renamed or removed\n>> the files (it is pretty much bog standard merge conflicts that do\n>> not happen very often to begin with).  It is when this kind of\n>> \"large scale refactoring\" for the sake of refactoring happens.\n>>\n>>> I'm not so worried about those other targets, but being able to\n>>> reliably build and test git with some cruft lying around is useful\n>>> though. I'm still not entirely sure what the motivation for this\n>>> change is (adding new files is not that common) but I think using the\n>>> established \"git ls-files || find\" pattern would be a good way of\n>>> globbing without picking up rubbish if there is a compelling reason to\n>>> drop the lists.\n>>\n>> Yes.\n> \n> Reviewing the reftable coverity topic I was reminded of this\n> patch. I.e. in it we have this fix:\n> https://lore.kernel.org/git/xmqqtugl102l.fsf@gitster.g/\n> \n> Which shows another advantage of using this sort of $(wildcard) pattern,\n> i.e. if we had this:\n> \t\n> \tdiff --git a/Makefile b/Makefile\n> \tindex 5580859afdb..48ea18afa53 100644\n> \t--- a/Makefile\n> \t+++ b/Makefile\n> \t@@ -2443,33 +2443,9 @@ XDIFF_OBJS += xdiff/xutils.o\n> \t .PHONY: xdiff-objs\n> \t xdiff-objs: $(XDIFF_OBJS)\n> \t\n> \t-REFTABLE_OBJS += reftable/basics.o\n> \t-REFTABLE_OBJS += reftable/error.o\n> \t-REFTABLE_OBJS += reftable/block.o\n> \t-REFTABLE_OBJS += reftable/blocksource.o\n> \t-REFTABLE_OBJS += reftable/iter.o\n> \t-REFTABLE_OBJS += reftable/publicbasics.o\n> \t-REFTABLE_OBJS += reftable/merged.o\n> \t-REFTABLE_OBJS += reftable/pq.o\n> \t-REFTABLE_OBJS += reftable/reader.o\n> \t-REFTABLE_OBJS += reftable/record.o\n> \t-REFTABLE_OBJS += reftable/refname.o\n> \t-REFTABLE_OBJS += reftable/generic.o\n> \t-REFTABLE_OBJS += reftable/stack.o\n> \t-REFTABLE_OBJS += reftable/tree.o\n> \t-REFTABLE_OBJS += reftable/writer.o\n> \t-\n> \t-REFTABLE_TEST_OBJS += reftable/basics_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/block_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/dump.o\n> \t-REFTABLE_TEST_OBJS += reftable/merged_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/pq_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/record_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/refname_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/stack_test.o\n> \t-REFTABLE_TEST_OBJS += reftable/test_framework.o\n> \t-REFTABLE_TEST_OBJS += reftable/tree_test.o\n> \t+REFTABLE_SOURCES = $(wildcard reftable/*.c)\n> \t+REFTABLE_OBJS += $(filter-out test,$(REFTABLE_SOURCES:%.c=%.o))\n> \t+REFTABLE_TEST_OBJS += $(filter test,$(REFTABLE_SOURCES:%.c=%.o))\n> \t\n> \t TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n> \n> We'd have a shorter Makefile, not need to manually maintain the list,\n> and we'd have been getting linker errors all along on the dead code\n> (just showing one of many here):\n> \n> \t$ make\n> \t[...]\n> \t/usr/bin/ld: reftable/libreftable.a(generic.o): in function `reftable_table_seek_ref':\n> \t/home/avar/g/git/reftable/generic.c:17: multiple definition of `reftable_table_seek_ref'; reftable/libreftable.a(reftable.o):/home/avar/g/git/reftable/reftable.c:17: first defined here\n> \t[...]\n> \tclang: error: linker command failed with exit code 1 (use -v to see invocation)\n> \tmake: *** [Makefile:2925: t/helper/test-tool] Error 1\n> \tmake: Target 'all' not remade because of errors.\n\nRandom cruft breaking the build was the reason I objected to this \nchange, just because the cruft was being tracked by git in this case \ndoes not change that.\n\nBest Wishes\n\nPhillip\n"},{"id":"446638","messageId":"220121.867datj6kv.gmgdl@evledraar.gmail.com","threadId":"56818","inReplyTo":"4caf3c95-c19c-80c6-daea-84dcb3ec9663@gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-01-21T18:13:44Z","receivedAt":"2022-01-21T18:18:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jan 21 2022, Phillip Wood wrote:\n\n> Hi Ævar\n>\n> On 21/01/2022 12:01, Ævar Arnfjörð Bjarmason wrote:\n>> On Tue, Nov 09 2021, Junio C Hamano wrote:\n>> \n>>> Phillip Wood <phillip.wood123@gmail.com> writes:\n>>>\n>>>>> We can also have some DEVOPTS knob so we'll prune out files found if\n>>>>> a\n>>>>> $(shell)-out to \"git status\" tells us they're untracked. I wouldn't mind\n>>>>> that (and could implement it) if it was optional.\n>>>>> Also note that you've got some of this already, e.g. we'll pick up\n>>>>> *.h\n>>>>> files via a glob for \"make TAGS\", the dependency graph etc.\n>>>>\n>>>> I'd be happier using 'git ls-files' with a glob if we need to move\n>>>> away from listing the files explicitly rather than having to pass some\n>>>> exclude list when running make. Having seen your comments below about\n>>>> ls-files/find I had a look at the Makefile and they always seem to be\n>>>> used together as \"git ls-files ... || find ...\". Doing that would mean\n>>>> we wouldn't try to build any untracked files but still find everything\n>>>> in a tarball.\n>>>\n>>> I've been quiet on this topic because honestly I do not find the\n>>> pros-and-cons favourable for more use of wildcards [*].  Tools like\n>>> git (especially .gitignore) and Makefile are to help users to be\n>>> safely sloppy by ensuring that random crufts the users may create in\n>>> the working tree for their convenience are not picked up by default\n>>> unless the project to consciously expresses the desire to use them.\n>>>\n>>> Allowing to be sloppy while maintaining Makefile feels like a false\n>>> economy, and having to paper it over by adding exceptions and\n>>> forcing developers to learn such ad-hoc rules even more so.\n>>>\n>>>      Side note: TAGS generation and some other minor things may use\n>>>      $(wildcard) and can throw tokens in cruft files in the output,\n>>>      which is not ideal, but the damage is local.  We cannot treat\n>>>      that the same as building binaries and tarballs.\n>>>\n>>> If we could use \"git ls-files\" consistently, that may make it\n>>> somewhat safer; you'd at least need to \"git add\" a new file before\n>>> it gets into the picture.  But it would be impossible, because we\n>>> need to be able to bootstrap Git from a tarball extract.\n>>>\n>>>>>>> We could make this simpler still for the Makefile by moving\n>>>>>>> \"unix-socket.c\" etc. to e.g. a \"conditional-src/\" directory, likewise\n>>>>>>> for $(PROGRAM_OBJS) to e.g. \"programs/\". If we did that we would not\n>>>>>>> need the \"$(filter-out)\" for LIB_OBJS. I don't think that's worth it,\n>>>>>>> e.g. due to \"git log -- <path>\" on the files now needing a \"--follow\".\n>>>\n>>> And it is not quite clear to me why we want to even more pile\n>>> workaround like this on top.  This also is to paper over the mistake\n>>> of being sloppy and using $(wildcard), which makes it unable to\n>>> distinguish, among the ones that match a pattern, between FOO_OBJS\n>>> and BAR_OBJS, no?  Moving files around in the working tree to group\n>>> related things together is a good thing, and it has been a good move\n>>> to separate built-ins and library-ish parts into different\n>>> directories.  But the above does not sound like it.\n>>>\n>>> Other than \"these source files may or may not be compiled\n>>> depending\", what trait do files in conditional-src/ share, other\n>>> than \"dividing them into a separate category makes it simpler to\n>>> write Makefile using $(wildcard)\"?  I do not think of a good one.\n>>>\n>>> The only time I found that the large list of files in Makefile was\n>>> problematic was *NOT* when multiple topics added, renamed or removed\n>>> the files (it is pretty much bog standard merge conflicts that do\n>>> not happen very often to begin with).  It is when this kind of\n>>> \"large scale refactoring\" for the sake of refactoring happens.\n>>>\n>>>> I'm not so worried about those other targets, but being able to\n>>>> reliably build and test git with some cruft lying around is useful\n>>>> though. I'm still not entirely sure what the motivation for this\n>>>> change is (adding new files is not that common) but I think using the\n>>>> established \"git ls-files || find\" pattern would be a good way of\n>>>> globbing without picking up rubbish if there is a compelling reason to\n>>>> drop the lists.\n>>>\n>>> Yes.\n>> Reviewing the reftable coverity topic I was reminded of this\n>> patch. I.e. in it we have this fix:\n>> https://lore.kernel.org/git/xmqqtugl102l.fsf@gitster.g/\n>> Which shows another advantage of using this sort of $(wildcard)\n>> pattern,\n>> i.e. if we had this:\n>> \t\n>> \tdiff --git a/Makefile b/Makefile\n>> \tindex 5580859afdb..48ea18afa53 100644\n>> \t--- a/Makefile\n>> \t+++ b/Makefile\n>> \t@@ -2443,33 +2443,9 @@ XDIFF_OBJS += xdiff/xutils.o\n>> \t .PHONY: xdiff-objs\n>> \t xdiff-objs: $(XDIFF_OBJS)\n>> \t\n>> \t-REFTABLE_OBJS += reftable/basics.o\n>> \t-REFTABLE_OBJS += reftable/error.o\n>> \t-REFTABLE_OBJS += reftable/block.o\n>> \t-REFTABLE_OBJS += reftable/blocksource.o\n>> \t-REFTABLE_OBJS += reftable/iter.o\n>> \t-REFTABLE_OBJS += reftable/publicbasics.o\n>> \t-REFTABLE_OBJS += reftable/merged.o\n>> \t-REFTABLE_OBJS += reftable/pq.o\n>> \t-REFTABLE_OBJS += reftable/reader.o\n>> \t-REFTABLE_OBJS += reftable/record.o\n>> \t-REFTABLE_OBJS += reftable/refname.o\n>> \t-REFTABLE_OBJS += reftable/generic.o\n>> \t-REFTABLE_OBJS += reftable/stack.o\n>> \t-REFTABLE_OBJS += reftable/tree.o\n>> \t-REFTABLE_OBJS += reftable/writer.o\n>> \t-\n>> \t-REFTABLE_TEST_OBJS += reftable/basics_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/block_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/dump.o\n>> \t-REFTABLE_TEST_OBJS += reftable/merged_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/pq_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/record_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/readwrite_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/refname_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/stack_test.o\n>> \t-REFTABLE_TEST_OBJS += reftable/test_framework.o\n>> \t-REFTABLE_TEST_OBJS += reftable/tree_test.o\n>> \t+REFTABLE_SOURCES = $(wildcard reftable/*.c)\n>> \t+REFTABLE_OBJS += $(filter-out test,$(REFTABLE_SOURCES:%.c=%.o))\n>> \t+REFTABLE_TEST_OBJS += $(filter test,$(REFTABLE_SOURCES:%.c=%.o))\n>> \t\n>> \t TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n>> We'd have a shorter Makefile, not need to manually maintain the\n>> list,\n>> and we'd have been getting linker errors all along on the dead code\n>> (just showing one of many here):\n>> \t$ make\n>> \t[...]\n>> \t/usr/bin/ld: reftable/libreftable.a(generic.o): in function `reftable_table_seek_ref':\n>> \t/home/avar/g/git/reftable/generic.c:17: multiple definition of `reftable_table_seek_ref'; reftable/libreftable.a(reftable.o):/home/avar/g/git/reftable/reftable.c:17: first defined here\n>> \t[...]\n>> \tclang: error: linker command failed with exit code 1 (use -v to see invocation)\n>> \tmake: *** [Makefile:2925: t/helper/test-tool] Error 1\n>> \tmake: Target 'all' not remade because of errors.\n>\n> Random cruft breaking the build was the reason I objected to this\n> change, just because the cruft was being tracked by git in this case \n> does not change that.\n\nThe difference is that the above is a \"good\" breakage, i.e. one where we\nended up carrying dead code in-tree, so it wouldn't have escaped the\nlab.\n\nWhereas I think that you're concerned that adding a WIP foobar.c to the\ntree would break your compilation the next time you invoke 'make'.\n\nWould something like this way of having our cake and eating it too\naddress your concerns?\n\nI.e. if we have 'git' installed already we could just ask it about any\nuntracked files, and filter those out:\n    \n    ifndef NO_WILDCARD_SECOND_GUESSING\n    UNTRACKED_FILES := $(shell git status --porcelain=1 2>/dev/null | sed -n -e '/^\\?? / { s/^.. //; p}')\n    else\n    UNTRACKED_FILES =\n    endif\n    \n    REFTABLE_SOURCES = $(filter-out $(UNTRACKED_FILES),$(wildcard reftable/*.c))\n    REFTABLE_OBJS += $(filter-out test,$(REFTABLE_SOURCES:%.c=%.o))\n    REFTABLE_TEST_OBJS += $(filter test,$(REFTABLE_SOURCES:%.c=%.o))\n\nA non-demo implementation of this would piggy-back on the \"git ls-files\"\nshell-out we already do, but since that requires some refactoring I\nthought it would be better to demo it like this.\n"},{"id":"446679","messageId":"xmqq35lgb7l5.fsf@gitster.g","threadId":"56818","inReplyTo":"220121.86o845jnvv.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v2 3/3] Makefile: replace most hardcoded object lists with $(wildcard)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-22T06:36:06Z","receivedAt":"2022-01-22T06:45:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>>> change is (adding new files is not that common) but I think using the\n>>> established \"git ls-files || find\" pattern would be a good way of\n>>> globbing without picking up rubbish if there is a compelling reason to\n>>> drop the lists.\n>>\n>> Yes.\n\nTo avoid any misunderstandings, the above \"Yes\" was given to the\nstatement, including the \"if there is a compelling reason\" part\n(and there isn't a compelling reason).\n\n> Reviewing the reftable coverity topic I was reminded of this\n> patch. I.e. in it we have this fix:\n> https://lore.kernel.org/git/xmqqtugl102l.fsf@gitster.g/\n\nI didn't give any \"fix\" in that message, though.\n\n> Which shows another advantage of using this sort of $(wildcard) pattern,\n> i.e. if we had this:\n> \t\n> \tdiff --git a/Makefile b/Makefile\n> \tindex 5580859afdb..48ea18afa53 100644\n> \t--- a/Makefile\n> \t+++ b/Makefile\n> \t@@ -2443,33 +2443,9 @@ XDIFF_OBJS += xdiff/xutils.o\n> \t .PHONY: xdiff-objs\n> \t xdiff-objs: $(XDIFF_OBJS)\n> \t \n> \t+REFTABLE_SOURCES = $(wildcard reftable/*.c)\n> \t+REFTABLE_OBJS += $(filter-out test,$(REFTABLE_SOURCES:%.c=%.o))\n> \t+REFTABLE_TEST_OBJS += $(filter test,$(REFTABLE_SOURCES:%.c=%.o))\n> \t \n> \t TEST_OBJS := $(patsubst %$X,%.o,$(TEST_PROGRAMS)) $(patsubst %,t/helper/%,$(TEST_BUILTINS_OBJS))\n>\n> We'd have a shorter Makefile, not need to manually maintain the list,\n\nBoth are not all that important.\n\n> and we'd have been getting linker errors all along on the dead code\n> (just showing one of many here):\n\nI am not sure if I follow.  You are forgetting to tell us something.\n\nAre you talking about an error you would see when you do what?\n\nPerhaps after you remove reftable/generic.c and have the definition\nof reftable_table_seek_ref() that used to be there in\nreftable/reftable.c?\n\nAssuming that is the scenario you have in mind, ...\n\n> \t$ make\n> \t[...]\n> \t/usr/bin/ld: reftable/libreftable.a(generic.o): in function `reftable_table_seek_ref':\n> \t/home/avar/g/git/reftable/generic.c:17: multiple definition of `reftable_table_seek_ref'; reftable/libreftable.a(reftable.o):/home/avar/g/git/reftable/reftable.c:17: first defined here\n\n... I do not think concrete list of filenames vs list of filenames\ncreated by $(wildcard) has any effect on that the fact that lib.a\nthat is incrementally updated by the \"ar r lib.a\" command does not\nlose a stale object file from it.\n\nIf we have a concrete filename list and removed generic.c, if we\nforget to remove it from the list, it will be noticed way before\n\"ld\" has the chance to complain.  We fail to produce generic.o,\nwhich may be a plus.  If we did not forget to also remove it from\nthe list when we removed the file, then $(wildcard) will give us the\nsame list of filenames, so you'd see the same error from your ld,\nno?\n"}]}