* [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW @ 2023-05-16 16:58 rsbecker 2023-05-16 17:06 ` Taylor Blau 0 siblings, 1 reply; 15+ messages in thread From: rsbecker @ 2023-05-16 16:58 UTC (permalink / raw) To: git Hi Git Team, I am getting a compile error on rc0: ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); ^ "/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c ", line 1103: error(114): identifier "ALLOC_GROW" is undefined Any help o nthis? Thanks, Randall -- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400) NonStop(211288444200000000) -- In real life, I talk too much. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW 2023-05-16 16:58 [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW rsbecker @ 2023-05-16 17:06 ` Taylor Blau 2023-05-16 17:19 ` Junio C Hamano 2023-05-16 17:56 ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau 0 siblings, 2 replies; 15+ messages in thread From: Taylor Blau @ 2023-05-16 17:06 UTC (permalink / raw) To: rsbecker; +Cc: git, Elijah Newren On Tue, May 16, 2023 at 12:58:15PM -0400, rsbecker@nexbridge.com wrote: > Hi Git Team, > > I am getting a compile error on rc0: > > ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, > git_atexit_hdlrs.alloc); > ^ > "/home/ituglib/randall/jenkins/.jenkins/workspace/Git_Pipeline/run-command.c > ", line 1103: error(114): identifier "ALLOC_GROW" is undefined > > Any help o nthis? None of the code in run-command.c has changed much recently, but this bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from cache.h, 2023-02-24). Note that it only affects when building with NO_PTHREADS=1 (which I suspect is the case on NonStop). Here's the bisection: $ git bisect start $ git bisect bad $ git bisect good v2.40.0 $ git bisect run sh -c 'make DEVELOPER=1 NO_PTHREADS=1 run-command.o' I'll attach a patch in a separate message. Thanks, Taylor ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW 2023-05-16 17:06 ` Taylor Blau @ 2023-05-16 17:19 ` Junio C Hamano 2023-05-16 17:24 ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano 2023-05-16 17:56 ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau 1 sibling, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2023-05-16 17:19 UTC (permalink / raw) To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren Taylor Blau <me@ttaylorr.com> writes: Ah, alloc.h was introduced and everybody who took ALLOC_GROW() from git-compat-util.h (or was it cache.h?) now includes it, but apparently Elijah forgot run-command.c and nobody caught this. I wonder if there are other leftover ones that we haven't caught? $ git grep -l '[^_]ALLOC_GROW(' \*.c | sort >/var/tmp/1 $ git grep -l 'alloc\.h' \*.c | sort >/var/tmp/2 $ comm -23 /var/tmp/[12] compat/simple-ipc/ipc-unix-socket.c run-command.c But the former one is only in a comment, so it probably is OK. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 17:19 ` Junio C Hamano @ 2023-05-16 17:24 ` Junio C Hamano 2023-05-16 17:33 ` rsbecker 2023-05-16 17:57 ` Taylor Blau 0 siblings, 2 replies; 15+ messages in thread From: Junio C Hamano @ 2023-05-16 17:24 UTC (permalink / raw) To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren Recent header file shuffling missed this old user of ALLOC_GROW() that was inside "#ifdef NO_PTHREADS' section and forgot to include the new file, alloc.h, that defines the macro. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- run-command.c | 1 + 1 file changed, 1 insertion(+) diff --git c/run-command.c w/run-command.c index d4247d5fcc..1affea48af 100644 --- c/run-command.c +++ w/run-command.c @@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code) } #else +#include <alloc.h> static struct { void (**handlers)(void); ^ permalink raw reply related [flat|nested] 15+ messages in thread
* RE: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 17:24 ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano @ 2023-05-16 17:33 ` rsbecker 2023-05-16 17:57 ` Taylor Blau 1 sibling, 0 replies; 15+ messages in thread From: rsbecker @ 2023-05-16 17:33 UTC (permalink / raw) To: 'Junio C Hamano', 'Taylor Blau' Cc: git, 'Elijah Newren' On Tuesday, May 16, 2023 1:24 PM, Junio C Hamano wrote: >Recent header file shuffling missed this old user of ALLOC_GROW() that was inside >"#ifdef NO_PTHREADS' section and forgot to include the new file, alloc.h, that defines >the macro. > >Signed-off-by: Junio C Hamano <gitster@pobox.com> > >--- > run-command.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git c/run-command.c w/run-command.c index d4247d5fcc..1affea48af 100644 >--- c/run-command.c >+++ w/run-command.c >@@ -1073,6 +1073,7 @@ static void NORETURN async_exit(int code) } > > #else >+#include <alloc.h> > > static struct { > void (**handlers)(void); Thanks gents. Regards, Randall ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 17:24 ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano 2023-05-16 17:33 ` rsbecker @ 2023-05-16 17:57 ` Taylor Blau 2023-05-16 18:01 ` Taylor Blau 1 sibling, 1 reply; 15+ messages in thread From: Taylor Blau @ 2023-05-16 17:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote: > Recent header file shuffling missed this old user of ALLOC_GROW() > that was inside "#ifdef NO_PTHREADS' section and forgot to include > the new file, alloc.h, that defines the macro. Heh. I wrote an identical patch before lunch (which I just came back from). Feel free to queue either, I honestly do not care which as long as the bug is fixed :-). https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/ Thanks, Taylor ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 17:57 ` Taylor Blau @ 2023-05-16 18:01 ` Taylor Blau 2023-05-16 18:44 ` Junio C Hamano 0 siblings, 1 reply; 15+ messages in thread From: Taylor Blau @ 2023-05-16 18:01 UTC (permalink / raw) To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren On Tue, May 16, 2023 at 01:57:50PM -0400, Taylor Blau wrote: > On Tue, May 16, 2023 at 10:24:24AM -0700, Junio C Hamano wrote: > > Recent header file shuffling missed this old user of ALLOC_GROW() > > that was inside "#ifdef NO_PTHREADS' section and forgot to include > > the new file, alloc.h, that defines the macro. > > Heh. I wrote an identical patch before lunch (which I just came back > from). Feel free to queue either, I honestly do not care which as long > as the bug is fixed :-). > > https://lore.kernel.org/git/db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com/ OK, having now read both completely, I would say I have a vague preference for my version since it keeps the include at the top and unconditional, and has slightly more information in the patch message. But I do not prefer it so much over yours that I would be sad if you had already queued yours and didn't want to bother shuffling it around. Thanks, Taylor ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 18:01 ` Taylor Blau @ 2023-05-16 18:44 ` Junio C Hamano 2023-05-16 18:47 ` Taylor Blau 0 siblings, 1 reply; 15+ messages in thread From: Junio C Hamano @ 2023-05-16 18:44 UTC (permalink / raw) To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren Taylor Blau <me@ttaylorr.com> writes: > OK, having now read both completely, I would say I have a vague > preference for my version since it keeps the include at the top and > unconditional, and has slightly more information in the patch message. > > But I do not prefer it so much over yours that I would be sad if you had > already queued yours and didn't want to bother shuffling it around. I do not have much preference between the two, either. Both lack one important description that we are reasonably confident that this breakage is limited to run-command.c and no other files. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 18:44 ` Junio C Hamano @ 2023-05-16 18:47 ` Taylor Blau 2023-05-16 18:53 ` Junio C Hamano 0 siblings, 1 reply; 15+ messages in thread From: Taylor Blau @ 2023-05-16 18:47 UTC (permalink / raw) To: Junio C Hamano; +Cc: rsbecker, git, Elijah Newren On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > OK, having now read both completely, I would say I have a vague > > preference for my version since it keeps the include at the top and > > unconditional, and has slightly more information in the patch message. > > > > But I do not prefer it so much over yours that I would be sad if you had > > already queued yours and didn't want to bother shuffling it around. > > I do not have much preference between the two, either. Both lack > one important description that we are reasonably confident that this > breakage is limited to run-command.c and no other files. I believe that mine does: (Everything else compiles fine when NO_PTHREADS is defined, so this is the only spot that needs fixing). Thanks, Taylor ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation 2023-05-16 18:47 ` Taylor Blau @ 2023-05-16 18:53 ` Junio C Hamano 0 siblings, 0 replies; 15+ messages in thread From: Junio C Hamano @ 2023-05-16 18:53 UTC (permalink / raw) To: Taylor Blau; +Cc: rsbecker, git, Elijah Newren Taylor Blau <me@ttaylorr.com> writes: > On Tue, May 16, 2023 at 11:44:36AM -0700, Junio C Hamano wrote: >> Taylor Blau <me@ttaylorr.com> writes: >> >> > OK, having now read both completely, I would say I have a vague >> > preference for my version since it keeps the include at the top and >> > unconditional, and has slightly more information in the patch message. >> > >> > But I do not prefer it so much over yours that I would be sad if you had >> > already queued yours and didn't want to bother shuffling it around. >> >> I do not have much preference between the two, either. Both lack >> one important description that we are reasonably confident that this >> breakage is limited to run-command.c and no other files. > > I believe that mine does: > > (Everything else compiles fine when NO_PTHREADS is defined, so this is > the only spot that needs fixing). Not quite. Who says NO_PTHREADS is the only conditional that may hide use of ALLOC_GROW()? ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] run-command.c: fix missing include under `NO_PTHREADS` 2023-05-16 17:06 ` Taylor Blau 2023-05-16 17:19 ` Junio C Hamano @ 2023-05-16 17:56 ` Taylor Blau 2023-05-16 19:12 ` Junio C Hamano 1 sibling, 1 reply; 15+ messages in thread From: Taylor Blau @ 2023-05-16 17:56 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Randall S. Becker, Elijah Newren When building git with `NO_PTHREADS=YesPlease`, we fail to build run-command.o since we don't have a definition for ALLOC_GROW: $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty CC run-command.o run-command.c: In function ‘git_atexit’: run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration] 1103 | ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); | ^~~~~~~~~~ cc1: all warnings being treated as errors make: *** [Makefile:2715: run-command.o] Error 1 This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from cache.h, 2023-02-24), which replaced includes of "cache.h" with "alloc.h", which is the new home of `ALLOC_GROW()` (and `ALLOC_GROW_BY()`). run-command.c compiles fine when `NO_PTHREADS` is undefined, since its use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything else compiles fine when NO_PTHREADS is defined, so this is the only spot that needs fixing). We could fix this by conditionally including "alloc.h" when `NO_PTHREADS` is defined. But we have relatively few examples of conditional includes throughout the tree[^1]. Instead, include "alloc.h" unconditionally in run-command.c to allow it to successfully compile even when NO_PTHREADS is defined. [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`. Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- run-command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/run-command.c b/run-command.c index d4247d5fcc..60c9419866 100644 --- a/run-command.c +++ b/run-command.c @@ -16,6 +16,7 @@ #include "packfile.h" #include "hook.h" #include "compat/nonblock.h" +#include "alloc.h" void child_process_init(struct child_process *child) { -- 2.41.0.rc0.dirty ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` 2023-05-16 17:56 ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau @ 2023-05-16 19:12 ` Junio C Hamano 2023-05-16 19:13 ` rsbecker 2023-05-16 21:33 ` Taylor Blau 0 siblings, 2 replies; 15+ messages in thread From: Junio C Hamano @ 2023-05-16 19:12 UTC (permalink / raw) To: Taylor Blau; +Cc: git, Randall S. Becker, Elijah Newren Taylor Blau <me@ttaylorr.com> writes: > When building git with `NO_PTHREADS=YesPlease`, we fail to build > run-command.o since we don't have a definition for ALLOC_GROW: > > $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o > GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty > CC run-command.o > run-command.c: In function ‘git_atexit’: > run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration] > 1103 | ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); > | ^~~~~~~~~~ > cc1: all warnings being treated as errors > make: *** [Makefile:2715: run-command.o] Error 1 I am OK to give a reproduction recipe, i.e. the "$ make" command line, to make it easy for people, who cannot remember how to define make variables from the command line, to try out themselves, but I hate the "build transcript" in the log message, when a few lines of prose suffices. How about this? Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS defined, i.e. $ make NO_PTHREADS=1 run-command.o as ALLOC_GROW() macro is used in the atexit() emulation but the macro definition is not available. > This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from > cache.h, 2023-02-24), which replaced includes of "cache.h" with > "alloc.h", which is the new home of `ALLOC_GROW()` (and > `ALLOC_GROW_BY()`). Good. Insert something like: We can see that run-command.c is the only one that try to use these macros without including <alloc.h>. $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1 $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2 $ comm -23 /tmp/[12] compat/simple-ipc/ipc-unix-socket.c run-command.c The "compat/" file only talks about the macro in the comment, and is not broken. here. What I am aiming for is to tell readers what they do not have to spend their time on. As we encourage people to make sure to look for other errors that come from the same root cause when making a fix, avoiding duplicated work by telling what they do not have to look at is rather important. > run-command.c compiles fine when `NO_PTHREADS` is undefined, since its > use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything > else compiles fine when NO_PTHREADS is defined, so this is the only spot > that needs fixing). I think we can say that, but is probably coered by the three-line summary I gave above. > We could fix this by conditionally including "alloc.h" when > `NO_PTHREADS` is defined. But we have relatively few examples of > conditional includes throughout the tree[^1]. > > Instead, include "alloc.h" unconditionally in run-command.c to allow it > to successfully compile even when NO_PTHREADS is defined. Good. > [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`. Good. ^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` 2023-05-16 19:12 ` Junio C Hamano @ 2023-05-16 19:13 ` rsbecker 2023-05-16 21:33 ` Taylor Blau 1 sibling, 0 replies; 15+ messages in thread From: rsbecker @ 2023-05-16 19:13 UTC (permalink / raw) To: 'Junio C Hamano', 'Taylor Blau' Cc: git, 'Randall S. Becker', 'Elijah Newren' On Tuesday, May 16, 2023 3:12 PM, Junio C Hamano wrote: >Taylor Blau <me@ttaylorr.com> writes: > >> When building git with `NO_PTHREADS=YesPlease`, we fail to build >> run-command.o since we don't have a definition for ALLOC_GROW: >> >> $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o >> GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty >> CC run-command.o >> run-command.c: In function ‘git_atexit’: >> run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [- >Werror=implicit-function-declaration] >> 1103 | ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, >git_atexit_hdlrs.alloc); >> | ^~~~~~~~~~ >> cc1: all warnings being treated as errors >> make: *** [Makefile:2715: run-command.o] Error 1 > >I am OK to give a reproduction recipe, i.e. the "$ make" command line, to make it >easy for people, who cannot remember how to define make variables from the >command line, to try out themselves, but I hate the "build transcript" in the log >message, when a few lines of prose suffices. > >How about this? > > Git 2.41-rc0 fails to compile run-command.c with NO_PTHREADS > defined, i.e. > > $ make NO_PTHREADS=1 run-command.o > > as ALLOC_GROW() macro is used in the atexit() emulation but the > macro definition is not available. > >> This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from >> cache.h, 2023-02-24), which replaced includes of "cache.h" with >> "alloc.h", which is the new home of `ALLOC_GROW()` (and >> `ALLOC_GROW_BY()`). > >Good. >Insert something like: > > We can see that run-command.c is the only one that try to use > these macros without including <alloc.h>. > > $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1 > $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2 > $ comm -23 /tmp/[12] > compat/simple-ipc/ipc-unix-socket.c > run-command.c > > The "compat/" file only talks about the macro in the comment, > and is not broken. > >here. What I am aiming for is to tell readers what they do not have to spend their >time on. As we encourage people to make sure to look for other errors that come >from the same root cause when making a fix, avoiding duplicated work by telling >what they do not have to look at is rather important. > >> run-command.c compiles fine when `NO_PTHREADS` is undefined, since its >> use of `ALLOC_GROW()` is behind a `#ifndef NO_PTHREADS`. (Everything >> else compiles fine when NO_PTHREADS is defined, so this is the only >> spot that needs fixing). > >I think we can say that, but is probably coered by the three-line summary I gave >above. > >> We could fix this by conditionally including "alloc.h" when >> `NO_PTHREADS` is defined. But we have relatively few examples of >> conditional includes throughout the tree[^1]. >> >> Instead, include "alloc.h" unconditionally in run-command.c to allow >> it to successfully compile even when NO_PTHREADS is defined. > >Good. > >> [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`. > >Good. This all makes sense to me. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` 2023-05-16 19:12 ` Junio C Hamano 2023-05-16 19:13 ` rsbecker @ 2023-05-16 21:33 ` Taylor Blau 2023-05-18 4:16 ` Elijah Newren 1 sibling, 1 reply; 15+ messages in thread From: Taylor Blau @ 2023-05-16 21:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Randall S. Becker, Elijah Newren On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > When building git with `NO_PTHREADS=YesPlease`, we fail to build > > run-command.o since we don't have a definition for ALLOC_GROW: > > > > $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o > > GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty > > CC run-command.o > > run-command.c: In function ‘git_atexit’: > > run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration] > > 1103 | ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); > > | ^~~~~~~~~~ > > cc1: all warnings being treated as errors > > make: *** [Makefile:2715: run-command.o] Error 1 > > I am OK to give a reproduction recipe, i.e. the "$ make" command > line, to make it easy for people, who cannot remember how to define > make variables from the command line, to try out themselves, but I > hate the "build transcript" in the log message, when a few lines of > prose suffices. Much appreciated. Here's a version with your changes include that is suitable for queueing: --- 8< --- Subject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined, i.e. $ make NO_PTHREADS=1 run-command.o as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the macro definition is not available. This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from cache.h, 2023-02-24), which replaced includes of <cache.h> with <alloc.h>, which is the new home of `ALLOC_GROW()` (and `ALLOC_GROW_BY()`). We can see that run-command.c is the only one that try to use these macros without including <alloc.h>. $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1 $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2 $ comm -23 /tmp/[12] compat/simple-ipc/ipc-unix-socket.c run-command.c The "compat/" file only talks about the macro in the comment, and is not broken. We could fix this by conditionally including "alloc.h" when `NO_PTHREADS` is defined. But we have relatively few examples of conditional includes throughout the tree[^1]. Instead, include "alloc.h" unconditionally in run-command.c to allow it to successfully compile even when NO_PTHREADS is defined. [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`. Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> Co-authored-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- run-command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/run-command.c b/run-command.c index d4247d5fcc..60c9419866 100644 --- a/run-command.c +++ b/run-command.c @@ -16,6 +16,7 @@ #include "packfile.h" #include "hook.h" #include "compat/nonblock.h" +#include "alloc.h" void child_process_init(struct child_process *child) { -- 2.41.0.rc0.dirty ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` 2023-05-16 21:33 ` Taylor Blau @ 2023-05-18 4:16 ` Elijah Newren 0 siblings, 0 replies; 15+ messages in thread From: Elijah Newren @ 2023-05-18 4:16 UTC (permalink / raw) To: Taylor Blau; +Cc: Junio C Hamano, git, Randall S. Becker On Tue, May 16, 2023 at 2:33 PM Taylor Blau <me@ttaylorr.com> wrote: > > On Tue, May 16, 2023 at 12:12:02PM -0700, Junio C Hamano wrote: > > Taylor Blau <me@ttaylorr.com> writes: > > > > > When building git with `NO_PTHREADS=YesPlease`, we fail to build > > > run-command.o since we don't have a definition for ALLOC_GROW: > > > > > > $ make NO_PTHREADS=1 DEVELOPER=1 run-command.o > > > GIT_VERSION = 2.41.0.rc0.1.g787eb3beae.dirty > > > CC run-command.o > > > run-command.c: In function ‘git_atexit’: > > > run-command.c:1103:9: error: implicit declaration of function ‘ALLOC_GROW’ [-Werror=implicit-function-declaration] > > > 1103 | ALLOC_GROW(git_atexit_hdlrs.handlers, git_atexit_hdlrs.nr + 1, git_atexit_hdlrs.alloc); > > > | ^~~~~~~~~~ > > > cc1: all warnings being treated as errors > > > make: *** [Makefile:2715: run-command.o] Error 1 > > > > I am OK to give a reproduction recipe, i.e. the "$ make" command > > line, to make it easy for people, who cannot remember how to define > > make variables from the command line, to try out themselves, but I > > hate the "build transcript" in the log message, when a few lines of > > prose suffices. > > Much appreciated. Here's a version with your changes include that is > suitable for queueing: > > --- 8< --- > > Subject: [PATCH] run-command.c: fix missing include under `NO_PTHREADS` > > Git 2.41-rc0 fails to compile run-command.c with `NO_PTHREADS` defined, > i.e. > > $ make NO_PTHREADS=1 run-command.o > > as `ALLOC_GROW()` macro is used in the `atexit()` emulation but the > macro definition is not available. > > This bisects to 36bf195890 (alloc.h: move ALLOC_GROW() functions from > cache.h, 2023-02-24), which replaced includes of <cache.h> with > <alloc.h>, which is the new home of `ALLOC_GROW()` (and > `ALLOC_GROW_BY()`). > > We can see that run-command.c is the only one that try to use these > macros without including <alloc.h>. > > $ git grep -l -e '[^_]ALLOC_GROW(' -e 'ALLOC_GROW_BY(' \*.c | sort >/tmp/1 > $ git grep -l 'alloc\.h' \*.c | sort >/tmp/2 > $ comm -23 /tmp/[12] > compat/simple-ipc/ipc-unix-socket.c > run-command.c > > The "compat/" file only talks about the macro in the comment, > and is not broken. > > We could fix this by conditionally including "alloc.h" when > `NO_PTHREADS` is defined. But we have relatively few examples of > conditional includes throughout the tree[^1]. > > Instead, include "alloc.h" unconditionally in run-command.c to allow it > to successfully compile even when NO_PTHREADS is defined. > > [^1]: With `git grep -E -A1 '#if(n)?def' -- **/*.c | grep '#include' -B1`. > > Reported-by: Randall S. Becker <randall.becker@nexbridge.ca> > Co-authored-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Junio C Hamano <gitster@pobox.com> > Signed-off-by: Taylor Blau <me@ttaylorr.com> > --- > run-command.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/run-command.c b/run-command.c > index d4247d5fcc..60c9419866 100644 > --- a/run-command.c > +++ b/run-command.c > @@ -16,6 +16,7 @@ > #include "packfile.h" > #include "hook.h" > #include "compat/nonblock.h" > +#include "alloc.h" > > void child_process_init(struct child_process *child) > { > -- > 2.41.0.rc0.dirty Thanks everyone for fixing up my issue, and my other issue in the imap-send/strbuf.h thread. Sorry for any headaches. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-05-18 4:17 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-16 16:58 [BUG] Git 2.41.0-rc0 - Compile Error ALLOC_GROW rsbecker 2023-05-16 17:06 ` Taylor Blau 2023-05-16 17:19 ` Junio C Hamano 2023-05-16 17:24 ` [PATCH] run-command.c: need alloc.h for our own at-exit handler emulation Junio C Hamano 2023-05-16 17:33 ` rsbecker 2023-05-16 17:57 ` Taylor Blau 2023-05-16 18:01 ` Taylor Blau 2023-05-16 18:44 ` Junio C Hamano 2023-05-16 18:47 ` Taylor Blau 2023-05-16 18:53 ` Junio C Hamano 2023-05-16 17:56 ` [PATCH] run-command.c: fix missing include under `NO_PTHREADS` Taylor Blau 2023-05-16 19:12 ` Junio C Hamano 2023-05-16 19:13 ` rsbecker 2023-05-16 21:33 ` Taylor Blau 2023-05-18 4:16 ` Elijah Newren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).