On Mon, Nov 22 2021, Johannes Schindelin wrote: > Hi Junio & brian, > > On Wed, 17 Nov 2021, Junio C Hamano wrote: > >> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.
