Subject: Re: [PATCH] config.mak.dev: fix DEVELOPER=1 on FreeBSD with -std=gnu99

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Wed, 19 Jan 2022 01:26:19 +0100

On Tue, Jan 18 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <[email-masked]> writes:
>
>> The inherent problem is that __has_extension() is a way to ask
>> Clang (and GCC) whether the compiler supports that feature, but the
>> -Wc11-extensions warning will be issued on the basis of the selected
>> __STDC_VERSION__. With -std=gnu99 the __has_extension() built-in will
>> return true, but the warning will still fire.
>>
>> Let's narrowly work around this by checking whether:
>
> The end result, if and when a change along this line proves OK for
> all versions of FreeBSD we care about, may be narrow, but as a band
> aid to discuss just before the final release, I do not know if
> anybody can sensibly assess the ramifications of such a change,
> except for somebody whose primary development environment has been
> FreeBSD for the past 6 months or more.

libgen.h is a trivially sized header (~20 lines of non-comment
non-whitespace), and uses __generic for exactly one thing.

If you'd like to go with your patch instead that's fair enough, but I
don't see what using FreeBSD as a primary development environment for an
extended time would buy anyone in understanding this narrow issue
causing the C11 warning.

> This looks way more than we can take comfortably at this point in
> the release cycle, compared to "fixing" the developer build by
> kicking developers on FreeBSD out of the "we are C99 or below at
> this point" enforcement mechanism.
>
>>  A. We are under FreeBSD
>>  B. We are using a C standard older than C11
>>
>> In that case we'll include sys/cdefs.h, and undefine FreeBSD's
>> __generic() wrapper if it's been defined. We'll then load libgen.h,
>> and restore whatever __generic definition we temporarily undefined
>> earlier.
>>
>> An alternate solution would be to simply define NO_LIBGEN_H=Y in
>> config.mak.uname for FreeBSD, but this way we'll use its OS-provided
>> basename() and dirname(), and in the general case ensure that nothing
>> changes outside of DEVELOPER=1 builds.
>>
>> 1. https://github.com/freebsd/freebsd-src/commit/62b7f85d4749
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
>> ---
>>  git-compat-util.h | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 1229c8296b9..69d9b5f202f 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -318,7 +318,25 @@ static inline int setitimer(int which, const struct itimerval *value, struct iti
>>  #endif
>>  
>>  #ifndef NO_LIBGEN_H
>> +/*
>> + * FreeBSD's libgen.h inadvertently requires C11 features, due to its
>> + * sys/cdefs.h using _Generic() if
>> + * __has_extension(c_generic_selections) is true, regardless of
>> + * __STDC_VERSION__....
>> + */
>> +#if defined(__FreeBSD__) && __STDC_VERSION__ - 0 < 201112L
>> +#include <sys/cdefs.h>
>> +#ifdef __generic
>> +#define __fbsd_generic __generic
>> +#endif
>> +#undef __generic
>> +#endif
>>  #include <libgen.h>
>> +/* ...continue FreeBSD-specific hack above */
>> +#ifdef __fbsd_generic
>> +#define __generic __fbsd_generic
>> +#undef __fbsd_generic
>> +#endif
>>  #else
>>  #define basename gitbasename
>>  char *gitbasename(char *);