git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/1] git-compat-util: add a test balloon for C99 support

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 22, 2021, 13:05 UTC
Message-ID
<211122.86pmqsz66b.gmgdl@evledraar.gmail.com>
In-Reply-To
<nycvar.QRO.7.76.6.2111221242320.63@tvgsbejvaqbjf.bet>
On Mon, Nov 22 2021, Johannes Schindelin wrote:
Show 52 quoted lines
> Hi Junio & brian,
>
> On Wed, 17 Nov 2021, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> >> Even MSVC, long a holdout against modern C, now supports both C11 and
>> >> C17 with an appropriate update.  Moreover, even if people are using an
>> >> older version of MSVC on these systems, they will generally need some
>> >> implementation of the standard Unix utilities for the testsuite, and GNU
>> >> coreutils, the most common option, has required C99 since 2009.
>> >> Therefore, we can safely assume that a suitable version of GCC or clang
>> >> is available to users even if their version of MSVC is not sufficiently
>> >> capable.
>> >
>> > I am all in favor of this patch!
>>
>> I like the direction, but ...
>>
>> >> diff --git a/Makefile b/Makefile
>> >> index 12be39ac49..22d9e67542 100644
>> >> --- a/Makefile
>> >> +++ b/Makefile
>> >> @@ -1204,7 +1204,7 @@ endif
>> >>  # Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be
>> >>  # tweaked by config.* below as well as the command-line, both of
>> >>  # which'll override these defaults.
>> >> -CFLAGS = -g -O2 -Wall
>> >> +CFLAGS = -g -O2 -Wall -std=gnu99
>>
>> ... as has been already pointed out, this part probably should not
>> be there.  It is not our intention to require gcc/clang, or to
>> constrain newer systems to gnu99.
>
> Another data point in favor of dropping this: our FreeBSD CI build reports
> a compile error with this:
>
> 	[...]
> 	archive.c:337:35: error: '_Generic' is a C11 extension
> 	[-Werror,-Wc11-extensions]
> 			strbuf_addstr(&path_in_archive, basename(path));
> 							^
> 	/usr/include/libgen.h:61:21: note: expanded from macro 'basename'
> 	#define basename(x)     __generic(x, const char *, __old_basename, basename)(x)
> 				^
> 	/usr/include/sys/cdefs.h:329:2: note: expanded from macro '__generic'
> 		_Generic(expr, t: yes, default: no)
> 		^
> 	1 error generated.
>
> I verified in https://github.com/gitgitgadget/git/pull/1082 that this
> patch is indeed the cause of this compile error.

As noted in another reply I don't think this -std=* thing is worth it, but this isn't so much a case of breakage with this patch in particular, but revealing an existing issue of us implicitly requiring C11 on FreeBSD.

Whether that's worth pursuing is another matter, but it's not some inherent issue in this approach, but just a platform-specific nit we could fix. Either by saying -std=c11 on that platform, or presumably defining NO_LIBGEN_H if we wanted to proceed in lockstep with C99-but-not-C11 everyhere.

Previous: Johannes SchindelinNext: Junio C Hamano
Message 4 of 14 in “Re: [PATCH 1/1] git-compat-util: add a test balloon for C99 support”
  1. Johannes SchindelinNov 16, 2021
  2. Junio C HamanoNov 17, 2021
  3. Johannes SchindelinNov 22, 2021
  4. Ævar Arnfjörð BjarmasonNov 22, 2021
  5. Junio C HamanoNov 22, 2021
  6. Carlo ArenasNov 22, 2021
  7. Junio C HamanoNov 22, 2021
  8. Junio C HamanoNov 22, 2021
  9. Johannes SchindelinNov 22, 2021
  10. Junio C HamanoNov 22, 2021
  11. Carlo ArenasNov 22, 2021
  12. Johannes SchindelinNov 23, 2021
  13. Junio C HamanoNov 23, 2021
  14. brian m. carlsonNov 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.