* [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
* [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: 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
* 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).