mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <>
To: Bagas Sanjaya <>
Cc: Junio C Hamano <>,,
	Fabrice Fontaine <>
Subject: Re: [PATCH] git-compat-util.h: Fix build without threads
Date: Mon, 28 Nov 2022 00:01:35 -0500	[thread overview]
Message-ID: <Y4RAr04vS/> (raw)
In-Reply-To: <>

On Fri, Nov 25, 2022 at 04:23:39PM +0700, Bagas Sanjaya wrote:

> From: Fabrice Fontaine <>
> Git build with toolchains without threads support is broken (as reported
> by Buildroot autobuilder [1]) since version 2.29.0, which traces back to
> 15b52a44e0 (compat-util: type-check parameters of no-op replacement
> functions, 2020-08-06):
> In file included from cache.h:4,
>                  from blame.c:1:
> git-compat-util.h:1238:20: error: static declaration of 'flockfile' follows non-static declaration
>  static inline void flockfile(FILE *fh)
>                     ^~~~~~~~~
> In file included from git-compat-util.h:168,
>                  from cache.h:4,
>                  from blame.c:1:
> /nvme/rc-buildroot-test/scripts/instance-0/output-1/host/arm-buildroot-linux-uclibcgnueabihf/sysroot/usr/include/stdio.h:806:13: note: previous declaration of 'flockfile' was here

We'll only compile our flockfile wrapper if _POSIX_THREAD_SAFE_FUNCTIONS
isn't set. So this is a platform where flockfile() is declared in a
header, but that flag is not defined.

If flockfile() really is available, I think a better fix here is to
figure out why the posix flag isn't set, or to set it manually, so that
we use the system flockfile(). That would give better performance.

From the filename, I'm assuming it's uclibc. And poking at the uclibc
source, it looks like the flag is sometimes set, but may be unset if
__UCLIBC_HAS_THREADS__ isn't set. And flockfile() is defined regardless.
So it may be correctly telling us that there's no thread support, but
still declares flockfile() anyway. Which is a little weird, but I think
not strictly violating POSIX.

If that's the case, then the patch here seems like the wrong thing.
We'll avoid the extra noop declaration of flockfile() in the header
which is blocking your compilation, but we'll still try to call
flockfile() in config.c. That will try to call the system version that
the headers told us should not be used. How will it behave? Maybe OK,
maybe not, depending on the platform.

All this points to 15b52a44e0 being a bit too loose with its
assumptions. It is assuming that if the posix flag is not defined, we
are free to use those system names. But here's an example where that is
not true. And the only way around it is with a macro, which is what we
had before that commit.

So I think we'd want to revert the flockfile() bits of that commit. And
I'd guess setitimer is in the same boat (the system may declare it, but
for whatever reason somebody may choose not to use it by feeding
NO_SETITIMER to make, at which point the compiler will complain about
the duplicate declaration.

There's more discussion in the original thread:

Junio said "we'll cross that bridge when we need to port to such a
system". I guess that time is now. ;)

In theory we should also #undef all of the macros we're replacing, in
case the platform implements them as macros. I'm OK to wait on that
until we see such a system, though (I was mildly surprised that the
compiler didn't also complain about getc_unlocked(), because I believe
that it can be a macro, but it looks like it isn't in uclibc).


  parent reply	other threads:[~2022-11-28  5:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-25  9:23 [PATCH] git-compat-util.h: Fix build without threads Bagas Sanjaya
2022-11-25 23:47 ` Ævar Arnfjörð Bjarmason
2022-11-28  5:04   ` Jeff King
2022-11-29  3:30   ` Bagas Sanjaya
2022-11-29  3:46     ` Bagas Sanjaya
2022-11-28  5:01 ` Jeff King [this message]
2022-11-30 21:15   ` [PATCH] git-compat-util: avoid redefining system function names Jeff King
2022-12-02 10:05     ` Bagas Sanjaya
2022-12-02 11:05       ` Jeff King
2022-12-03  8:02         ` Bagas Sanjaya
2022-12-07  8:33           ` Bagas Sanjaya
2022-12-07 13:00             ` Jeff King
2022-12-02 11:31     ` Ævar Arnfjörð Bjarmason
2022-12-02 22:50       ` Jeff King

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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y4RAr04vS/ \ \ \ \ \ \

* 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.
Code repositories for project(s) associated with this public inbox

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).