{"thread":{"id":"59751","subject":"[BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW","startedAt":"2023-05-16T16:58:30Z","lastAt":"2023-05-18T04:17:08Z","messageCount":15,"participants":["rsbecker@nexbridge.com","Taylor Blau","Junio C Hamano","Elijah Newren"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"477385","messageId":"009501d98817$9eb44560$dc1cd020$@nexbridge.com","threadId":"59751","inReplyTo":null,"subject":"[BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-05-16T16:58:15Z","receivedAt":"2023-05-16T16:58:30Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"Hi Git Team,\n\nI am getting a compile error on rc0:\n\n  \tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,\ngit_atexit_hdlrs.alloc);\n  \t^\n\"/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c\n\", line 1103: error(114): identifier \"ALLOC_GROW\" is undefined\n\nAny help o nthis?\n\nThanks,\nRandall\n\n--\nBrief whoami: NonStop&UNIX developer since approximately\nUNIX(421664400)\nNonStop(211288444200000000)\n-- In real life, I talk too much.\n\n\n\n"},{"id":"477387","messageId":"ZGO4LesPe4A2ftdm@nand.local","threadId":"59751","inReplyTo":"009501d98817$9eb44560$dc1cd020$@nexbridge.com","subject":"Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T17:06:53Z","receivedAt":"2023-05-16T17:06:59Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, May 16, 2023 at 12:58:15PM -0400, rsbecker@nexbridge.com wrote:\n> Hi Git Team,\n>\n> I am getting a compile error on rc0:\n>\n>   \tALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,\n> git_atexit_hdlrs.alloc);\n>   \t^\n> \"/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c\n> \", line 1103: error(114): identifier \"ALLOC_GROW\" is undefined\n>\n> Any help o nthis?\n\nNone of the code in run-command.c has changed much recently, but this\nbisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\ncache.h, 2023-02-24).\n\nNote that it only affects when building with NO_PTHREADS=1 (which I\nsuspect is the case on NonStop). Here's the bisection:\n\n    $ git bisect start\n    $ git bisect bad\n    $ git bisect good v2.40.0\n    $ git bisect run sh -c 'make DEVELOPER=1 NO_PTHREADS=1 run-command.o'\n\nI'll attach a patch in a separate message.\n\nThanks,\nTaylor\n"},{"id":"477395","messageId":"xmqqlehourbf.fsf@gitster.g","threadId":"59751","inReplyTo":"ZGO4LesPe4A2ftdm@nand.local","subject":"Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T17:19:48Z","receivedAt":"2023-05-16T17:19:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\nAh, alloc.h was introduced and everybody who took ALLOC_GROW() from\ngit-compat-util.h (or was it cache.h?) now includes it, but\napparently Elijah forgot  run-command.c and nobody caught this.\n\nI wonder if there are other leftover ones that we haven't caught?\n\n    $ git grep -l '[^_]ALLOC_GROW(' \\*.c | sort >/var/tmp/1\n    $ git grep -l 'alloc\\.h' \\*.c | sort >/var/tmp/2\n    $ comm -23 /var/tmp/[12]\n    compat/simple-ipc/ipc-unix-socket.c\n    run-command.c\n\nBut the former one is only in a comment, so it probably is OK.\n\n"},{"id":"477396","messageId":"xmqqh6scur3r.fsf_-_@gitster.g","threadId":"59751","inReplyTo":"xmqqlehourbf.fsf@gitster.g","subject":"[PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T17:24:24Z","receivedAt":"2023-05-16T17:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Recent header file shuffling missed this old user of ALLOC_GROW()\nthat was inside \"#ifdef NO_PTHREADS' section and forgot to include\nthe new file, alloc.h, that defines the macro.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n\n---\n run-command.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git c/run-command.c w/run-command.c\nindex d4247d5fcc..1affea48af 100644\n--- c/run-command.c\n+++ w/run-command.c\n@@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code)\n }\n \n #else\n+#include <alloc.h>\n \n static struct {\n \tvoid (**handlers)(void);\n"},{"id":"477397","messageId":"009a01d9881c$8dc57490$a9505db0$@nexbridge.com","threadId":"59751","inReplyTo":"xmqqh6scur3r.fsf_-_@gitster.g","subject":"RE: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-05-16T17:33:34Z","receivedAt":"2023-05-16T17:33:47Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, May 16, 2023 1:24 PM, Junio C Hamano wrote:\n>Recent header file shuffling missed this old user of ALLOC_GROW() that was\ninside\n>\"#ifdef NO_PTHREADS' section and forgot to include the new file, alloc.h,\nthat defines\n>the macro.\n>\n>Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>\n>---\n> run-command.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n>diff --git c/run-command.c w/run-command.c index d4247d5fcc..1affea48af\n100644\n>--- c/run-command.c\n>+++ w/run-command.c\n>@@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code)  }\n>\n> #else\n>+#include <alloc.h>\n>\n> static struct {\n> \tvoid (**handlers)(void);\n\nThanks gents.\n\nRegards,\nRandall\n\n"},{"id":"477401","messageId":"db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com","threadId":"59751","inReplyTo":"ZGO4LesPe4A2ftdm@nand.local","subject":"[PATCH] run-command.c: fix missing include under `NO_PTHREADS`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T17:56:21Z","receivedAt":"2023-05-16T17:56:40Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"When building git with `NO_PTHREADS=YesPlease`, we fail to build\nrun-command.o since we don't have a definition for ALLOC_GROW:\n\n    $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o\n    GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty\n        CC run-command.o\n    run-command.c: In function ‘git_atexit’:\n    run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]\n     1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n          |         ^~~~~~~~~~\n    cc1: all warnings being treated as errors\n    make: *** [Makefile:2715: run-command.o] Error 1\n\nThis bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\ncache.h, 2023-02-24), which replaced includes of \"cache.h\" with\n\"alloc.h\", which is the new home of `ALLOC_GROW()` (and\n`ALLOC_GROW_BY()`).\n\nrun-command.c compiles fine when `NO_PTHREADS` is undefined, since its\nuse of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything\nelse compiles fine when NO_PTHREADS is defined, so this is the only spot\nthat needs fixing).\n\nWe could fix this by conditionally including \"alloc.h\" when\n`NO_PTHREADS` is defined.  But we have relatively few examples of\nconditional includes throughout the tree[^1].\n\nInstead, include \"alloc.h\" unconditionally in run-command.c to allow it\nto successfully compile even when NO_PTHREADS is defined.\n\n[^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.\n\nReported-by: Randall S. Becker <randall.becker@nexbridge.ca>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n run-command.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex d4247d5fcc..60c9419866 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -16,6 +16,7 @@\n #include \"packfile.h\"\n #include \"hook.h\"\n #include \"compat/nonblock.h\"\n+#include \"alloc.h\"\n \n void child_process_init(struct child_process *child)\n {\n-- \n2.41.0.rc0.dirty\n"},{"id":"477405","messageId":"ZGPEHu2pCYfdNI0h@nand.local","threadId":"59751","inReplyTo":"xmqqh6scur3r.fsf_-_@gitster.g","subject":"Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T17:57:50Z","receivedAt":"2023-05-16T17:58:00Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote:\n> Recent header file shuffling missed this old user of ALLOC_GROW()\n> that was inside \"#ifdef NO_PTHREADS' section and forgot to include\n> the new file, alloc.h, that defines the macro.\n\nHeh. I wrote an identical patch before lunch (which I just came back\nfrom). Feel free to queue either, I honestly do not care which as long\nas the bug is fixed :-).\n\n  https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/\n\nThanks,\nTaylor\n"},{"id":"477406","messageId":"ZGPE9ozRChUJ3NGV@nand.local","threadId":"59751","inReplyTo":"ZGPEHu2pCYfdNI0h@nand.local","subject":"Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T18:01:26Z","receivedAt":"2023-05-16T18:01:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, May 16, 2023 at 01:57:50PM -0400, Taylor Blau wrote:\n> On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote:\n> > Recent header file shuffling missed this old user of ALLOC_GROW()\n> > that was inside \"#ifdef NO_PTHREADS' section and forgot to include\n> > the new file, alloc.h, that defines the macro.\n>\n> Heh. I wrote an identical patch before lunch (which I just came back\n> from). Feel free to queue either, I honestly do not care which as long\n> as the bug is fixed :-).\n>\n>   https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/\n\nOK, having now read both completely, I would say I have a vague\npreference for my version since it keeps the include at the top and\nunconditional, and has slightly more information in the patch message.\n\nBut I do not prefer it so much over yours that I would be sad if you had\nalready queued yours and didn't want to bother shuffling it around.\n\nThanks,\nTaylor\n"},{"id":"477408","messageId":"xmqq353wune3.fsf@gitster.g","threadId":"59751","inReplyTo":"ZGPE9ozRChUJ3NGV@nand.local","subject":"Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T18:44:36Z","receivedAt":"2023-05-16T18:44:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> OK, having now read both completely, I would say I have a vague\n> preference for my version since it keeps the include at the top and\n> unconditional, and has slightly more information in the patch message.\n>\n> But I do not prefer it so much over yours that I would be sad if you had\n> already queued yours and didn't want to bother shuffling it around.\n\nI do not have much preference between the two, either.  Both lack\none important description that we are reasonably confident that this\nbreakage is limited to run-command.c and no other files.\n\nThanks.\n"},{"id":"477409","messageId":"ZGPP02zYlSM9iRYt@nand.local","threadId":"59751","inReplyTo":"xmqq353wune3.fsf@gitster.g","subject":"Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T18:47:47Z","receivedAt":"2023-05-16T18:47:59Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > OK, having now read both completely, I would say I have a vague\n> > preference for my version since it keeps the include at the top and\n> > unconditional, and has slightly more information in the patch message.\n> >\n> > But I do not prefer it so much over yours that I would be sad if you had\n> > already queued yours and didn't want to bother shuffling it around.\n>\n> I do not have much preference between the two, either.  Both lack\n> one important description that we are reasonably confident that this\n> breakage is limited to run-command.c and no other files.\n\nI believe that mine does:\n\n  (Everything else compiles fine when NO_PTHREADS is defined, so this is\n  the only spot that needs fixing).\n\nThanks,\nTaylor\n"},{"id":"477410","messageId":"xmqqy1lot8en.fsf@gitster.g","threadId":"59751","inReplyTo":"ZGPP02zYlSM9iRYt@nand.local","subject":"Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T18:53:36Z","receivedAt":"2023-05-16T18:53:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote:\n>> Taylor Blau <me@ttaylorr.com> writes:\n>>\n>> > OK, having now read both completely, I would say I have a vague\n>> > preference for my version since it keeps the include at the top and\n>> > unconditional, and has slightly more information in the patch message.\n>> >\n>> > But I do not prefer it so much over yours that I would be sad if you had\n>> > already queued yours and didn't want to bother shuffling it around.\n>>\n>> I do not have much preference between the two, either.  Both lack\n>> one important description that we are reasonably confident that this\n>> breakage is limited to run-command.c and no other files.\n>\n> I believe that mine does:\n>\n>   (Everything else compiles fine when NO_PTHREADS is defined, so this is\n>   the only spot that needs fixing).\n\nNot quite.  Who says NO_PTHREADS is the only conditional that may\nhide use of ALLOC_GROW()?\n"},{"id":"477412","messageId":"xmqqh6sct7jx.fsf@gitster.g","threadId":"59751","inReplyTo":"db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com","subject":"Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-16T19:12:02Z","receivedAt":"2023-05-16T19:12:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> When building git with `NO_PTHREADS=YesPlease`, we fail to build\n> run-command.o since we don't have a definition for ALLOC_GROW:\n>\n>     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o\n>     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty\n>         CC run-command.o\n>     run-command.c: In function ‘git_atexit’:\n>     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]\n>      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n>           |         ^~~~~~~~~~\n>     cc1: all warnings being treated as errors\n>     make: *** [Makefile:2715: run-command.o] Error 1\n\nI am OK to give a reproduction recipe, i.e. the \"$ make\" command\nline, to make it easy for people, who cannot remember how to define\nmake variables from the command line, to try out themselves, but I\nhate the \"build transcript\" in the log message, when a few lines of\nprose suffices.\n\nHow about this?\n\n    Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS\n    defined, i.e.\n\n      $ make NO_PTHREADS=1 run-command.o\n\n    as ALLOC_GROW() macro is used in the atexit() emulation but the\n    macro definition is not available.\n\n> This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\n> cache.h, 2023-02-24), which replaced includes of \"cache.h\" with\n> \"alloc.h\", which is the new home of `ALLOC_GROW()` (and\n> `ALLOC_GROW_BY()`).\n\nGood.\nInsert something like:\n\n    We can see that run-command.c is the only one that try to use\n    these macros without including <alloc.h>.\n\n      $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \\*.c | sort >/tmp/1\n      $ git grep -l 'alloc\\.h' \\*.c | sort >/tmp/2\n      $ comm -23 /tmp/[12]\n      compat/simple-ipc/ipc-unix-socket.c\n      run-command.c\n\n    The \"compat/\" file only talks about the macro in the comment,\n    and is not broken.\n\nhere.  What I am aiming for is to tell readers what they do not have\nto spend their time on.  As we encourage people to make sure to look\nfor other errors that come from the same root cause when making a\nfix, avoiding duplicated work by telling what they do not have to\nlook at is rather important.\n\n> run-command.c compiles fine when `NO_PTHREADS` is undefined, since its\n> use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything\n> else compiles fine when NO_PTHREADS is defined, so this is the only spot\n> that needs fixing).\n\nI think we can say that, but is probably coered by the three-line\nsummary I gave above.\n\n> We could fix this by conditionally including \"alloc.h\" when\n> `NO_PTHREADS` is defined.  But we have relatively few examples of\n> conditional includes throughout the tree[^1].\n>\n> Instead, include \"alloc.h\" unconditionally in run-command.c to allow it\n> to successfully compile even when NO_PTHREADS is defined.\n\nGood.\n\n> [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.\n\nGood.\n"},{"id":"477413","messageId":"00a601d9882a$93c7a940$bb56fbc0$@nexbridge.com","threadId":"59751","inReplyTo":"xmqqh6sct7jx.fsf@gitster.g","subject":"RE: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2023-05-16T19:13:57Z","receivedAt":"2023-05-16T19:14:13Z","isPatch":true,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On Tuesday, May 16, 2023 3:12 PM, Junio C Hamano wrote:\n>Taylor Blau <me@ttaylorr.com> writes:\n>\n>> When building git with `NO_PTHREADS=YesPlease`, we fail to build\n>> run-command.o since we don't have a definition for ALLOC_GROW:\n>>\n>>     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o\n>>     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty\n>>         CC run-command.o\n>>     run-command.c: In function ‘git_atexit’:\n>>     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-\n>Werror=implicit-function-declaration]\n>>      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1,\n>git_atexit_hdlrs.alloc);\n>>           |         ^~~~~~~~~~\n>>     cc1: all warnings being treated as errors\n>>     make: *** [Makefile:2715: run-command.o] Error 1\n>\n>I am OK to give a reproduction recipe, i.e. the \"$ make\" command line, to make it\n>easy for people, who cannot remember how to define make variables from the\n>command line, to try out themselves, but I hate the \"build transcript\" in the log\n>message, when a few lines of prose suffices.\n>\n>How about this?\n>\n>    Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS\n>    defined, i.e.\n>\n>      $ make NO_PTHREADS=1 run-command.o\n>\n>    as ALLOC_GROW() macro is used in the atexit() emulation but the\n>    macro definition is not available.\n>\n>> This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\n>> cache.h, 2023-02-24), which replaced includes of \"cache.h\" with\n>> \"alloc.h\", which is the new home of `ALLOC_GROW()` (and\n>> `ALLOC_GROW_BY()`).\n>\n>Good.\n>Insert something like:\n>\n>    We can see that run-command.c is the only one that try to use\n>    these macros without including <alloc.h>.\n>\n>      $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \\*.c | sort >/tmp/1\n>      $ git grep -l 'alloc\\.h' \\*.c | sort >/tmp/2\n>      $ comm -23 /tmp/[12]\n>      compat/simple-ipc/ipc-unix-socket.c\n>      run-command.c\n>\n>    The \"compat/\" file only talks about the macro in the comment,\n>    and is not broken.\n>\n>here.  What I am aiming for is to tell readers what they do not have to spend their\n>time on.  As we encourage people to make sure to look for other errors that come\n>from the same root cause when making a fix, avoiding duplicated work by telling\n>what they do not have to look at is rather important.\n>\n>> run-command.c compiles fine when `NO_PTHREADS` is undefined, since its\n>> use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything\n>> else compiles fine when NO_PTHREADS is defined, so this is the only\n>> spot that needs fixing).\n>\n>I think we can say that, but is probably coered by the three-line summary I gave\n>above.\n>\n>> We could fix this by conditionally including \"alloc.h\" when\n>> `NO_PTHREADS` is defined.  But we have relatively few examples of\n>> conditional includes throughout the tree[^1].\n>>\n>> Instead, include \"alloc.h\" unconditionally in run-command.c to allow\n>> it to successfully compile even when NO_PTHREADS is defined.\n>\n>Good.\n>\n>> [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.\n>\n>Good.\n\nThis all makes sense to me.\n\n"},{"id":"477417","messageId":"ZGP2tw0USsj9oecZ@nand.local","threadId":"59751","inReplyTo":"xmqqh6sct7jx.fsf@gitster.g","subject":"Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-05-16T21:33:43Z","receivedAt":"2023-05-16T21:34:11Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote:\n> Taylor Blau <me@ttaylorr.com> writes:\n>\n> > When building git with `NO_PTHREADS=YesPlease`, we fail to build\n> > run-command.o since we don't have a definition for ALLOC_GROW:\n> >\n> >     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o\n> >     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty\n> >         CC run-command.o\n> >     run-command.c: In function ‘git_atexit’:\n> >     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]\n> >      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n> >           |         ^~~~~~~~~~\n> >     cc1: all warnings being treated as errors\n> >     make: *** [Makefile:2715: run-command.o] Error 1\n>\n> I am OK to give a reproduction recipe, i.e. the \"$ make\" command\n> line, to make it easy for people, who cannot remember how to define\n> make variables from the command line, to try out themselves, but I\n> hate the \"build transcript\" in the log message, when a few lines of\n> prose suffices.\n\nMuch appreciated. Here's a version with your changes include that is\nsuitable for queueing:\n\n--- 8< ---\n\nSubject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`\n\nGit 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined,\ni.e.\n\n  $ make NO_PTHREADS=1 run-command.o\n\nas `ALLOC_GROW()` macro is used in the `atexit()` emulation but the\nmacro definition is not available.\n\nThis bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\ncache.h, 2023-02-24), which replaced includes of <cache.h> with\n<alloc.h>, which is the new home of `ALLOC_GROW()` (and\n`ALLOC_GROW_BY()`).\n\nWe can see that run-command.c is the only one that try to use these\nmacros without including <alloc.h>.\n\n  $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \\*.c | sort >/tmp/1\n  $ git grep -l 'alloc\\.h' \\*.c | sort >/tmp/2\n  $ comm -23 /tmp/[12]\n  compat/simple-ipc/ipc-unix-socket.c\n  run-command.c\n\nThe \"compat/\" file only talks about the macro in the comment,\nand is not broken.\n\nWe could fix this by conditionally including \"alloc.h\" when\n`NO_PTHREADS` is defined.  But we have relatively few examples of\nconditional includes throughout the tree[^1].\n\nInstead, include \"alloc.h\" unconditionally in run-command.c to allow it\nto successfully compile even when NO_PTHREADS is defined.\n\n[^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.\n\nReported-by: Randall S. Becker <randall.becker@nexbridge.ca>\nCo-authored-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n run-command.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/run-command.c b/run-command.c\nindex d4247d5fcc..60c9419866 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -16,6 +16,7 @@\n #include \"packfile.h\"\n #include \"hook.h\"\n #include \"compat/nonblock.h\"\n+#include \"alloc.h\"\n\n void child_process_init(struct child_process *child)\n {\n--\n2.41.0.rc0.dirty\n\n"},{"id":"477508","messageId":"CABPp-BFvCQKspFhq-NKnjjAx4V3jFpcV60N-WRD2HZHy4BQtHg@mail.gmail.com","threadId":"59751","inReplyTo":"ZGP2tw0USsj9oecZ@nand.local","subject":"Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2023-05-18T04:16:48Z","receivedAt":"2023-05-18T04:17:08Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, May 16, 2023 at 2:33 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote:\n> > Taylor Blau <me@ttaylorr.com> writes:\n> >\n> > > When building git with `NO_PTHREADS=YesPlease`, we fail to build\n> > > run-command.o since we don't have a definition for ALLOC_GROW:\n> > >\n> > >     $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o\n> > >     GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty\n> > >         CC run-command.o\n> > >     run-command.c: In function ‘git_atexit’:\n> > >     run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration]\n> > >      1103 |         ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc);\n> > >           |         ^~~~~~~~~~\n> > >     cc1: all warnings being treated as errors\n> > >     make: *** [Makefile:2715: run-command.o] Error 1\n> >\n> > I am OK to give a reproduction recipe, i.e. the \"$ make\" command\n> > line, to make it easy for people, who cannot remember how to define\n> > make variables from the command line, to try out themselves, but I\n> > hate the \"build transcript\" in the log message, when a few lines of\n> > prose suffices.\n>\n> Much appreciated. Here's a version with your changes include that is\n> suitable for queueing:\n>\n> --- 8< ---\n>\n> Subject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`\n>\n> Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined,\n> i.e.\n>\n>   $ make NO_PTHREADS=1 run-command.o\n>\n> as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the\n> macro definition is not available.\n>\n> This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from\n> cache.h, 2023-02-24), which replaced includes of <cache.h> with\n> <alloc.h>, which is the new home of `ALLOC_GROW()` (and\n> `ALLOC_GROW_BY()`).\n>\n> We can see that run-command.c is the only one that try to use these\n> macros without including <alloc.h>.\n>\n>   $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \\*.c | sort >/tmp/1\n>   $ git grep -l 'alloc\\.h' \\*.c | sort >/tmp/2\n>   $ comm -23 /tmp/[12]\n>   compat/simple-ipc/ipc-unix-socket.c\n>   run-command.c\n>\n> The \"compat/\" file only talks about the macro in the comment,\n> and is not broken.\n>\n> We could fix this by conditionally including \"alloc.h\" when\n> `NO_PTHREADS` is defined.  But we have relatively few examples of\n> conditional includes throughout the tree[^1].\n>\n> Instead, include \"alloc.h\" unconditionally in run-command.c to allow it\n> to successfully compile even when NO_PTHREADS is defined.\n>\n> [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`.\n>\n> Reported-by: Randall S. Becker <randall.becker@nexbridge.ca>\n> Co-authored-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> Signed-off-by: Taylor Blau <me@ttaylorr.com>\n> ---\n>  run-command.c | 1 +\n>  1 file changed, 1 insertion(+)\n>\n> diff --git a/run-command.c b/run-command.c\n> index d4247d5fcc..60c9419866 100644\n> --- a/run-command.c\n> +++ b/run-command.c\n> @@ -16,6 +16,7 @@\n>  #include \"packfile.h\"\n>  #include \"hook.h\"\n>  #include \"compat/nonblock.h\"\n> +#include \"alloc.h\"\n>\n>  void child_process_init(struct child_process *child)\n>  {\n> --\n> 2.41.0.rc0.dirty\n\nThanks everyone for fixing up my issue, and my other issue in the\nimap-send/strbuf.h thread.  Sorry for any headaches.\n"}]}