Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
From: Junio C Hamano <hidden>
Date: 2023-05-16 19:12:11
Possibly related (same subject, not in this thread)
- 2023-05-16 · [PATCH] run-command.c: fix missing include under `NO_PTHREADS` · Taylor Blau <hidden>
Taylor Blau [off-list ref] 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.