From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org,
"Randall S. Becker" <randall.becker@nexbridge.ca>,
Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH] run-command.c: fix missing include under `NO_PTHREADS`
Date: Tue, 16 May 2023 12:12:02 -0700 [thread overview]
Message-ID: <xmqqh6sct7jx.fsf@gitster.g> (raw)
In-Reply-To: <db403de74da839084165f11dab05d71484457c6f.1684259780.git.me@ttaylorr.com> (Taylor Blau's message of "Tue, 16 May 2023 13:56:21 -0400")
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.
next prev parent reply other threads:[~2023-05-16 19:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2023-05-16 19:13 ` rsbecker
2023-05-16 21:33 ` Taylor Blau
2023-05-18 4:16 ` Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqh6sct7jx.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=newren@gmail.com \
--cc=randall.becker@nexbridge.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).