{"thread":{"id":"56921","subject":"[PATCH] Makefile: fix parallel build race","startedAt":"2021-11-17T01:26:08Z","lastAt":"2021-11-19T17:07:19Z","messageCount":12,"participants":["Đoàn Trần Công Danh","Jeff King","Mike Hommey","Ævar Arnfjörð Bjarmason","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441384","messageId":"7d82342089a80b19e54ac8997d5765a33951499f.1637112066.git.congdanhqx@gmail.com","threadId":"56921","inReplyTo":null,"subject":"[PATCH] Makefile: fix parallel build race","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-11-17T01:25:55Z","receivedAt":"2021-11-17T01:26:08Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"* builtin/bugreport.c includes hook-list.h, hence generated files from\nit must depend on hook-list.h\n\nSigned-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>\n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 241dc322c0..413503b488 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2222,6 +2222,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \n help.sp help.s help.o: command-list.h\n hook.sp hook.s hook.o: hook-list.h\n+builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h\n \n builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n-- \n2.34.0.rc1\n\n"},{"id":"441401","messageId":"YZR0djZbRUicXcQm@coredump.intra.peff.net","threadId":"56921","inReplyTo":"7d82342089a80b19e54ac8997d5765a33951499f.1637112066.git.congdanhqx@gmail.com","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-17T03:18:14Z","receivedAt":"2021-11-17T03:18:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 17, 2021 at 08:25:55AM +0700, Đoàn Trần Công Danh wrote:\n\n> * builtin/bugreport.c includes hook-list.h, hence generated files from\n> it must depend on hook-list.h\n\nGood catch. This is trivially reproducible with:\n\n  make clean\n  make builtin/bugreport.o\n\nThe problem comes from cfe853e66b (hook-list.h: add a generated list of\nhooks, like config-list.h, 2021-09-26), as you might expect.\n\n> diff --git a/Makefile b/Makefile\n> index 241dc322c0..413503b488 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -2222,6 +2222,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n>  \n>  help.sp help.s help.o: command-list.h\n>  hook.sp hook.s hook.o: hook-list.h\n> +builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h\n\nThis fix looks correct. I grepped for other similar cases, but this is\nthe only file that needs it.\n\nCuriously, the existing hook.c does not seem to include hook-list.h,\neven though you can see a dependency in the context above. Nor does\nhelp.c, which gained a similar dependency in cfe853e66b. Those seem\nsuperfluous, but maybe I'm missing something.\n\nI wondered if contrib/buildsystems/CMakeLists would need a similar\nfixup, but it doesn't have any generated header dependencies at all (not\nfor hook-list.h, but not for the existing command-list.h). So I'll\nassume it's fine (as did cfe853e66b).\n\n-Peff\n"},{"id":"441403","messageId":"20211117033938.r3wsv3znxva7smgy@glandium.org","threadId":"56921","inReplyTo":"YZR0djZbRUicXcQm@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2021-11-17T03:39:38Z","receivedAt":"2021-11-17T03:39:48Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Tue, Nov 16, 2021 at 10:18:14PM -0500, Jeff King wrote:\n> On Wed, Nov 17, 2021 at 08:25:55AM +0700, Đoàn Trần Công Danh wrote:\n> \n> > * builtin/bugreport.c includes hook-list.h, hence generated files from\n> > it must depend on hook-list.h\n> \n> Good catch. This is trivially reproducible with:\n> \n>   make clean\n>   make builtin/bugreport.o\n> \n> The problem comes from cfe853e66b (hook-list.h: add a generated list of\n> hooks, like config-list.h, 2021-09-26), as you might expect.\n> \n> > diff --git a/Makefile b/Makefile\n> > index 241dc322c0..413503b488 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -2222,6 +2222,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n> >  \n> >  help.sp help.s help.o: command-list.h\n> >  hook.sp hook.s hook.o: hook-list.h\n> > +builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h\n> \n> This fix looks correct. I grepped for other similar cases, but this is\n> the only file that needs it.\n> \n> Curiously, the existing hook.c does not seem to include hook-list.h,\n> even though you can see a dependency in the context above. Nor does\n> help.c, which gained a similar dependency in cfe853e66b. Those seem\n> superfluous, but maybe I'm missing something.\n\nNeither does builtin/help.c. This was discussed in the subthread\nstarting at https://lore.kernel.org/all/20211115220455.xse7mhbwabrheej4@glandium.org/\nand is covered by https://lore.kernel.org/all/patch-v3-19.23-234b4eb613c-20211116T114334Z-avarab@gmail.com/\n(to which I responded that the line for hook.o can be removed too)\n\nMike\n"},{"id":"441467","messageId":"211117.86o86j6q0s.gmgdl@evledraar.gmail.com","threadId":"56921","inReplyTo":"20211117033938.r3wsv3znxva7smgy@glandium.org","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-17T10:21:02Z","receivedAt":"2021-11-17T10:22:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 17 2021, Mike Hommey wrote:\n\n> On Tue, Nov 16, 2021 at 10:18:14PM -0500, Jeff King wrote:\n>> On Wed, Nov 17, 2021 at 08:25:55AM +0700, Đoàn Trần Công Danh wrote:\n>> \n>> > * builtin/bugreport.c includes hook-list.h, hence generated files from\n>> > it must depend on hook-list.h\n>> \n>> Good catch. This is trivially reproducible with:\n>> \n>>   make clean\n>>   make builtin/bugreport.o\n>> \n>> The problem comes from cfe853e66b (hook-list.h: add a generated list of\n>> hooks, like config-list.h, 2021-09-26), as you might expect.\n>> \n>> > diff --git a/Makefile b/Makefile\n>> > index 241dc322c0..413503b488 100644\n>> > --- a/Makefile\n>> > +++ b/Makefile\n>> > @@ -2222,6 +2222,7 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n>> >  \n>> >  help.sp help.s help.o: command-list.h\n>> >  hook.sp hook.s hook.o: hook-list.h\n>> > +builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h\n>> \n>> This fix looks correct. I grepped for other similar cases, but this is\n>> the only file that needs it.\n>> \n>> Curiously, the existing hook.c does not seem to include hook-list.h,\n>> even though you can see a dependency in the context above. Nor does\n>> help.c, which gained a similar dependency in cfe853e66b. Those seem\n>> superfluous, but maybe I'm missing something.\n>\n> Neither does builtin/help.c. This was discussed in the subthread\n> starting at https://lore.kernel.org/all/20211115220455.xse7mhbwabrheej4@glandium.org/\n> and is covered by https://lore.kernel.org/all/patch-v3-19.23-234b4eb613c-20211116T114334Z-avarab@gmail.com/\n> (to which I responded that the line for hook.o can be removed too)\n\nI've got an updated patch in my just-re-rolled Makefile dependency fixes\nseries for this isuse, which also addresses the needless \"hook.{sp,s,o}\n-> hook-list.h\" dependency issue:\nhttps://lore.kernel.org/git/patch-v4-19.23-2710f8af6cd-20211117T101807Z-avarab@gmail.com/\n\nThanks again for looking all of this over & helping to make the Makefile\nbetter.\n"},{"id":"441531","messageId":"nycvar.QRO.7.76.6.2111180012470.21127@tvgsbejvaqbjf.bet","threadId":"56921","inReplyTo":"YZR0djZbRUicXcQm@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-17T23:14:31Z","receivedAt":"2021-11-17T23:14:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 16 Nov 2021, Jeff King wrote:\n\n> I wondered if contrib/buildsystems/CMakeLists would need a similar\n> fixup, but it doesn't have any generated header dependencies at all (not\n> for hook-list.h, but not for the existing command-list.h). So I'll\n> assume it's fine (as did cfe853e66b).\n\nThe strategy we take in our CMake-based configuration is for files like\nhook-list.h to be generated at _configure_ time, i.e. before the build\ndefinition file is written, i.e. well before the build. That's why there\nis no explicit dependency, it's not necessary.\n\nCiao,\nDscho\n"},{"id":"441539","messageId":"211118.86tuga5o68.gmgdl@evledraar.gmail.com","threadId":"56921","inReplyTo":"nycvar.QRO.7.76.6.2111180012470.21127@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-17T23:56:35Z","receivedAt":"2021-11-17T23:59:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 18 2021, Johannes Schindelin wrote:\n\n> On Tue, 16 Nov 2021, Jeff King wrote:\n>\n>> I wondered if contrib/buildsystems/CMakeLists would need a similar\n>> fixup, but it doesn't have any generated header dependencies at all (not\n>> for hook-list.h, but not for the existing command-list.h). So I'll\n>> assume it's fine (as did cfe853e66b).\n>\n> The strategy we take in our CMake-based configuration is for files like\n> hook-list.h to be generated at _configure_ time, i.e. before the build\n> definition file is written, i.e. well before the build. That's why there\n> is no explicit dependency, it's not necessary.\n\nIt is necessary, otherwise how will it know to re-generate the\nhook-list.h if its source of truth changes? I.e. if we add a new\nhook. Ditto for a new built-in, config variable etc.\n\nI understand that the answer is that cmake (or at least our use of it)\ndoesn't even try to solve the same problem as the Makefile does, i.e. to\ndeclare dependencies and to be capable of incremental builds.\n\nIt's more of a one-shot command where you'll need to run its equivalent\nof \"make clean\" before you recompile.\n\nCorrect?\n"},{"id":"441552","messageId":"YZWqK38NRjD7aPOG@danh.dev","threadId":"56921","inReplyTo":"211118.86tuga5o68.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2021-11-18T01:19:39Z","receivedAt":"2021-11-18T01:19:44Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2021-11-18 00:56:35+0100, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> On Thu, Nov 18 2021, Johannes Schindelin wrote:\n> \n> > On Tue, 16 Nov 2021, Jeff King wrote:\n> >\n> >> I wondered if contrib/buildsystems/CMakeLists would need a similar\n> >> fixup, but it doesn't have any generated header dependencies at all (not\n> >> for hook-list.h, but not for the existing command-list.h). So I'll\n> >> assume it's fine (as did cfe853e66b).\n> >\n> > The strategy we take in our CMake-based configuration is for files like\n> > hook-list.h to be generated at _configure_ time, i.e. before the build\n> > definition file is written, i.e. well before the build. That's why there\n> > is no explicit dependency, it's not necessary.\n> \n> It is necessary, otherwise how will it know to re-generate the\n> hook-list.h if its source of truth changes? I.e. if we add a new\n> hook. Ditto for a new built-in, config variable etc.\n> \n> I understand that the answer is that cmake (or at least our use of it)\n> doesn't even try to solve the same problem as the Makefile does, i.e. to\n> declare dependencies and to be capable of incremental builds.\n\nIf used correctly, with correct dependencies link, cmake is fully\ncapable to regenerate hook-list.h upon its source mtime changed.\n\n> It's more of a one-shot command where you'll need to run its equivalent\n> of \"make clean\" before you recompile.\n\nHowever, the current CMakeLists.txt has a bigger problem: it won't\nre-run itself when a source file has been added or removed.\nIt couldn't be configured on Linux system, except with this diff\napplied (because CMake documentation mandated <docstring> in\n(set CACHE FORCE) [1]):\n\n----- 8< ----\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 6d7bc16d05..a612217dd9 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -52,9 +52,10 @@ cmake_minimum_required(VERSION 3.14)\n #set the source directory to root of git\n set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)\n \n+if(WIN32)\n option(USE_VCPKG \"Whether or not to use vcpkg for obtaining dependencies.  Only applicable to Windows platforms\" ON)\n-if(NOT WIN32)\n-\tset(USE_VCPKG OFF CACHE BOOL FORCE)\n+else()\n+\tset(USE_VCPKG OFF)\n endif()\n \n if(NOT DEFINED CMAKE_EXPORT_COMPILE_COMMANDS)\n----- >8 ----\n\nEven after it's applied, the linking step is failing.\n(seems to not link with compat/linux/procinfo.o, I didn't dig further)\n\nThe traditional method to list source files in CMake (and meson)\nis listing them all in the CMakeLists.txt (or meson.build).\nWith manual listing like that, we can avoid the current complicated\nlogic to parse Makefile. The bigger benefit from listing manually is:\nCMake will generate an implicit dependency to CMakeLists.txt,\nhence, whenever a source/header files was added/removed,\ncmake will told to re-run configuring steps.\n\nIf you're interested on moving on that direction, I can provide\nsome patches to make the cmake buildsystem a bit less messy,\nI'm not a fan of CMake, don't count too much on me, though.\n\n[1]: https://cmake.org/cmake/help/v3.16/command/set.html#set-cache-entry\n\n-- \nDanh\n"},{"id":"441608","messageId":"nycvar.QRO.7.76.6.2111181529380.11028@tvgsbejvaqbjf.bet","threadId":"56921","inReplyTo":"YZWqK38NRjD7aPOG@danh.dev","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-18T14:36:05Z","receivedAt":"2021-11-18T14:36:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 18 Nov 2021, Đoàn Trần Công Danh wrote:\n\n> [Git's CMake-based build] couldn't be configured on Linux system,\n\nThat was an explicit decision in\nhttps://lore.kernel.org/git/xmqq1rmcm6md.fsf@gitster.c.googlers.com/:\n\n\tLet's not worry about cross-platform and instead stick to Windows\n\tand nothing else for now to expedite the process.  As long as it\n\tis advertised as such, nobody would complain that it does not work\n\ton Linux or macOS.\n\nCiao,\nDscho\n"},{"id":"441700","messageId":"211119.86ilwo4o8c.gmgdl@evledraar.gmail.com","threadId":"56921","inReplyTo":"YZWqK38NRjD7aPOG@danh.dev","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-19T07:06:04Z","receivedAt":"2021-11-19T07:08:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 18 2021, Đoàn Trần Công Danh wrote:\n\n> On 2021-11-18 00:56:35+0100, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> \n>> On Thu, Nov 18 2021, Johannes Schindelin wrote:\n>> \n>> > On Tue, 16 Nov 2021, Jeff King wrote:\n>> >\n>> >> I wondered if contrib/buildsystems/CMakeLists would need a similar\n>> >> fixup, but it doesn't have any generated header dependencies at all (not\n>> >> for hook-list.h, but not for the existing command-list.h). So I'll\n>> >> assume it's fine (as did cfe853e66b).\n>> >\n>> > The strategy we take in our CMake-based configuration is for files like\n>> > hook-list.h to be generated at _configure_ time, i.e. before the build\n>> > definition file is written, i.e. well before the build. That's why there\n>> > is no explicit dependency, it's not necessary.\n>> \n>> It is necessary, otherwise how will it know to re-generate the\n>> hook-list.h if its source of truth changes? I.e. if we add a new\n>> hook. Ditto for a new built-in, config variable etc.\n>> \n>> I understand that the answer is that cmake (or at least our use of it)\n>> doesn't even try to solve the same problem as the Makefile does, i.e. to\n>> declare dependencies and to be capable of incremental builds.\n>\n> If used correctly, with correct dependencies link, cmake is fully\n> capable to regenerate hook-list.h upon its source mtime changed.\n>\n>> It's more of a one-shot command where you'll need to run its equivalent\n>> of \"make clean\" before you recompile.\n>\n> However, the current CMakeLists.txt has a bigger problem: it won't\n> re-run itself when a source file has been added or removed.\n> It couldn't be configured on Linux system, except with this diff\n> applied (because CMake documentation mandated <docstring> in\n> (set CACHE FORCE) [1]):\n>\n> ----- 8< ----\n> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\n> index 6d7bc16d05..a612217dd9 100644\n> --- a/contrib/buildsystems/CMakeLists.txt\n> +++ b/contrib/buildsystems/CMakeLists.txt\n> @@ -52,9 +52,10 @@ cmake_minimum_required(VERSION 3.14)\n>  #set the source directory to root of git\n>  set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)\n>  \n> +if(WIN32)\n>  option(USE_VCPKG \"Whether or not to use vcpkg for obtaining dependencies.  Only applicable to Windows platforms\" ON)\n> -if(NOT WIN32)\n> -\tset(USE_VCPKG OFF CACHE BOOL FORCE)\n> +else()\n> +\tset(USE_VCPKG OFF)\n>  endif()\n>  \n>  if(NOT DEFINED CMAKE_EXPORT_COMPILE_COMMANDS)\n> ----- >8 ----\n>\n> Even after it's applied, the linking step is failing.\n> (seems to not link with compat/linux/procinfo.o, I didn't dig further)\n>\n> The traditional method to list source files in CMake (and meson)\n> is listing them all in the CMakeLists.txt (or meson.build).\n> With manual listing like that, we can avoid the current complicated\n> logic to parse Makefile. The bigger benefit from listing manually is:\n> CMake will generate an implicit dependency to CMakeLists.txt,\n> hence, whenever a source/header files was added/removed,\n> cmake will told to re-run configuring steps.\n>\n> If you're interested on moving on that direction, I can provide\n> some patches to make the cmake buildsystem a bit less messy,\n> I'm not a fan of CMake, don't count too much on me, though.\n>\n> [1]: https://cmake.org/cmake/help/v3.16/command/set.html#set-cache-entry\n\nI think getting it working on non-Windows if we're going to keep it\n(which looks to be the case) would be very useful.\n\nI think you should look at the WIP patches from & coordinate with\nPhillip Wood, who has WIP patches in that direction. See:\nhttps://lore.kernel.org/git/24482f96-7d87-1570-a171-95ec182f6091@gmail.com/\n"},{"id":"441703","messageId":"211119.86ee7c4n8r.gmgdl@evledraar.gmail.com","threadId":"56921","inReplyTo":"nycvar.QRO.7.76.6.2111181529380.11028@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-19T07:08:11Z","receivedAt":"2021-11-19T07:29:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Nov 18 2021, Johannes Schindelin wrote:\n\n> On Thu, 18 Nov 2021, Đoàn Trần Công Danh wrote:\n>\n>> [Git's CMake-based build] couldn't be configured on Linux system,\n>\n> That was an explicit decision in\n> https://lore.kernel.org/git/xmqq1rmcm6md.fsf@gitster.c.googlers.com/:\n>\n> \tLet's not worry about cross-platform and instead stick to Windows\n> \tand nothing else for now to expedite the process.  As long as it\n> \tis advertised as such, nobody would complain that it does not work\n> \ton Linux or macOS.\n\nThat was said at a time when the CI didn't have a hard dependency on\nthis cmake integration, which as noted in the recent discussion\ndownthread of [1] made it so that for changes in this area you need to\nmaintain both the Makefile and contrib/buildsystems/CMakeLists.txt in\nlockstep, least that CI is broken.\n\nSo we've created a scenario where in order to make certain changes to\ngit.git, you need to either go through a very painfully slow\nedit/push/test cycle via the CI, with each test taking anywhere between\n~5m-60m, depending on what step it might fail at. Or, install your own\ncopy of that proprietary OS locally.\n\nIn other areas we often don't have any hard line separating such\nplatform-specific code from the rest of the codebase. But for anything\nelse I can think of it's in its own files or ifdefs, and usually doesn't\nrequire maintaining dual-implementations in lockstep, except if the\ninterface itself is changing.\n\nOr that code is at least in a common language like C, Sh, Perl etc., and\ndoesn't require you to learn a new tool/language just for maintaining\nthe propriterary-specific portability code.\n\nSo I think it would be most welcome to get patches to\ncontrib/buildsystems/CMakeLists.txt to make it portable, cmake itself\nis, and AFAICT the only inherently Windows-specific part of it is some\nsmall part dealing with generating a file for VS to consume, which\npresumably can be in some if/else construct within that file (I haven't\nchecked out Phillip Wood's portability patches).\n\n1. https://lore.kernel.org/git/patch-1.1-bbacbed5c95-20211030T223011Z-avarab@gmail.com/\n"},{"id":"441750","messageId":"nycvar.QRO.7.76.6.2111191625002.63@tvgsbejvaqbjf.bet","threadId":"56921","inReplyTo":"211119.86ilwo4o8c.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-19T15:44:46Z","receivedAt":"2021-11-19T15:44:56Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Fri, 19 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> I think getting it working on non-Windows if we're going to keep it\n> (which looks to be the case) would be very useful.\n\nThe idea to extend the CMake to more than just Windows is contrary to what\nJunio said in\nhttps://lore.kernel.org/git/xmqq1rmcm6md.fsf@gitster.c.googlers.com/:\n\n\tLet's not worry about cross-platform and instead stick to Windows\n\tand nothing else for now to expedite the process.  As long as it\n\tis advertised as such, nobody would complain that it does not work\n\ton Linux or macOS.\n\nIf that is not enough to tone down opposing opinions (the opinion of the\nGit maintainer is more important, after all, it's his maintenance burden\nso he gets to decide), you can also look at this statement from\nhttps://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com/:\n\n\tI already said that I feel that engineering burden to divert\n\tresources for CMake support would be unacceptably high.\n\nThe only reason we have CMake in addition to the Makefile (and the\nautoconf-based) setup is that CMake makes it possible to build Git on\nWindows in the development environment with which the majority of the\ndevelopers on Windows are familiar: Visual Studio.\n\nIf it weren't for those developers, for who it would be a ridiculous\nsuggestion to \"just go download GNU make\", we would not have the CMake\nbased build at all.\n\nAnd I am still agreeing with what Junio further said in the second mail I\nlinked above:\n\n\t[...] it is unclear why it would be beneficial to slow our\n\texisting developers down by forcing them to become familiar with\n\tCMake.\n\nSo now we are discussing to extend the CMake build to allow Linux and\nmacOS developers to use it, to, for little to no benefit. We are very much\nin the situation where we are slowed down by discussing something as\nnon-essential as extending our CMake support beyond Windows, while patches\nthat are provably much more beneficial to a lot more people are left\nunder-reviewed.\n\nEven worse: reviewers who _could_ provide high-quality reviews for those\npatches (which takes a lot of time and diligence), but are as much pressed\nfor time as I am and therefore have to choose wisely how to spend their\ntime, are _actively_ distracted from spending their time more wisely.\n\nCan't we please focus on more relevant things again? Pretty please?\n\nCiao,\nJohannes\n"},{"id":"441763","messageId":"211119.86h7c82hx8.gmgdl@evledraar.gmail.com","threadId":"56921","inReplyTo":"nycvar.QRO.7.76.6.2111191625002.63@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] Makefile: fix parallel build race","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-19T16:48:32Z","receivedAt":"2021-11-19T17:07:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Nov 19 2021, Johannes Schindelin wrote:\n\n> On Fri, 19 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n>\n>> I think getting it working on non-Windows if we're going to keep it\n>> (which looks to be the case) would be very useful.\n>\n> The idea to extend the CMake to more than just Windows is contrary to what\n> Junio said in\n> https://lore.kernel.org/git/xmqq1rmcm6md.fsf@gitster.c.googlers.com/:\n>\n> \tLet's not worry about cross-platform and instead stick to Windows\n> \tand nothing else for now to expedite the process.  As long as it\n> \tis advertised as such, nobody would complain that it does not work\n> \ton Linux or macOS.\n\nThat's quoted in the <211119.86ee7c4n8r.gmgdl@evledraar.gmail.com>\nyou're replying to. No need to repeat it.\n\n> If that is not enough to tone down opposing opinions (the opinion of the\n> Git maintainer is more important, after all, it's his maintenance burden\n> so he gets to decide), you can also look at this statement from\n> https://lore.kernel.org/git/xmqq8sikblv2.fsf@gitster.c.googlers.com/:\n>\n> \tI already said that I feel that engineering burden to divert\n> \tresources for CMake support would be unacceptably high.\n\nThere's some back & forth in that thread, I do think a fair summary of\nit is that the proponents of CMakeLists.txt, including yourself, assured\nreviewers that having the cmake component wouldn't place a maintenance\nburden on anyone not using it.\n\nIncluding yourself at:\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.2004251354390.18039@tvgsbejvaqbjf.bet/:\n\n    When it comes to new Makefile knobs, I do agree that it would place an\n    unacceptable burden on contributors if we expected them to add the same\n    knob to CMakeLists.txt. But we already don't do that for our autoconf\n    support, so why would we expect it for CMake?\n    \n    When it comes to adding new, and/or removing, files, I fail to see the\n    problem. It is dead easy to keep the Makefile and CMakeLists.txt in sync\n    when it comes to lists of files.\n\nThe \"that it\" here refers to \"slow our existing developers down by\nforcing them to become familiar with CMake\", which you quote below.\n\n> The only reason we have CMake in addition to the Makefile (and the\n> autoconf-based) setup is that CMake makes it possible to build Git on\n> Windows in the development environment with which the majority of the\n> developers on Windows are familiar: Visual Studio.\n>\n> If it weren't for those developers, for who it would be a ridiculous\n> suggestion to \"just go download GNU make\", we would not have the CMake\n> based build at all.\n\nI'm happy it works for those developers, and don't mind at all that\nthey're choosing to run a propriterary stack while developing a free\nsoftware project, I'd just prefer not to be forced to do that because\nour CI has a hard dependency on it.\n\nWe can argue about the trade-offs here, but I think it's clearly\nhyperbole to say that would be a ridiculous suggestion when it was the\nstatus quo until 2020.\n\nI'm specifically pointing out that the issue with the hard dependency of\nCI on this \"contrib\" component.\n\nI'd think of all people you'd be in vehement agreement with me on that\npoint, after all our back & forth on the contrib/scalar topic, and that\nthings \"contrib\" must be decoupled and \"optional\" in a way that this\ncmake integration clearly isn't.\n\n> And I am still agreeing with what Junio further said in the second mail I\n> linked above:\n>\n> \t[...] it is unclear why it would be beneficial to slow our\n> \texisting developers down by forcing them to become familiar with\n> \tCMake.\n>\n> So now we are discussing to extend the CMake build to allow Linux and\n> macOS developers to use it, to, for little to no benefit. We are very much\n> in the situation where we are slowed down by discussing something as\n> non-essential as extending our CMake support beyond Windows, while patches\n> that are provably much more beneficial to a lot more people are left\n> under-reviewed.\n>\n> Even worse: reviewers who _could_ provide high-quality reviews for those\n> patches (which takes a lot of time and diligence), but are as much pressed\n> for time as I am and therefore have to choose wisely how to spend their\n> time, are _actively_ distracted from spending their time more wisely.\n>\n> Can't we please focus on more relevant things again? Pretty please?\n\nWe have out-of-tree patches to make this thing work outside of Windows\nauthored by Phillip, and it sonuds like Đoàn would find it useful too.\n\nAs for myself I really don't care to interact with this cmake component\nat all, but if we're not going to drop the CI hard dependency on it I'd\nfind being able to test it on a free OS locally to be vastly\npreferrable.\n\nIn any case, I'm not submitting patches in that direction. But I don't\nsee a reason to discourage other people from collaborating on topics\nthey may find useful, as you're doing here.\n"}]}