[PATCH 0/1] Add a test balloon for C99

[PATCH 0/1] Add a test balloon for C99

[brian m. carlson]

The C99 standard, more formally known as ISO/IEC 9899:1999, was, as its
name suggests, was ratified in 1999, more than 20 years ago[0].  It has
a variety of nice features, including loop variable initialization, that
are available to most modern programmers.

In fact, POSIX 1003.1-2001 and the Single Unix Specification version 3,
made its use mandatory through the c99 command.  As a practical matter,
all known Unix systems which receive security support have support for
these standards or their successors, POSIX 1003.1-2018 and the Single
Unix Specification version 4, and as such, we can safely assume support
for C99 there.

Unfortunately, Microsoft for many years refused[1] to support C99 in
MSVC, and still does not officially do so.  However, they recently added
support for C11 and C17, which are sufficient for modern programming.
These require a newer version of MSVC, including an updated SDK.  The
SDK update is available for download free of charge, and most public CI
systems have support for both the updated MSVC and the SDK update.

Even for users who would like to target an older version of Windows,
such as the no longer supported Windows 7, GCC and Clang are available.
The LLVM suite, including Clang, is available pre-compiled for download
free of charge.  Using a different compiler is specifically proposed by
Microsoft staff[1] as a solution for users who wish to build modern
programs for MSVC versions which do not support modern C.

As such, we can assume that Git can be safely compiled with C99 or C11
support on all operating systems which receive security support, and
even some which do not.  Our CI confirms that this series passes all
tests.  Let's introduce a test balloon which checks for this support and
fails with an error message if it is absent.

[0] The reader will note that there are people working professionally in
this industry who were not yet born at the time C99 was ratified.  Thus,
this occurred quite a long time ago indeed.
[1] https://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/

brian m. carlson (1):
  git-compat-util: add a test balloon for C99 support

 Makefile                            |  4 ++--
 contrib/buildsystems/CMakeLists.txt |  3 +--
 git-compat-util.h                   | 12 ++++++++++++
 3 files changed, 15 insertions(+), 4 deletions(-)

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

[brian m. carlson]

The C99 standard was released in January 1999, now 22 years ago.  It
provides a variety of useful features, including variadic arguments for
macros, declarations after statements, variable length arrays, and a
wide variety of other useful features, many of which we already use.

We'd like to take advantage of these features, but we want to be
cautious.  As far as we know, all major compilers now support C99 or a
later C standard, such as C11 or C17.  POSIX has required C99 support as
a requirement for the 2001 revision, so we can safely assume any POSIX
system which we are interested in supporting has C99.

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.

Let's add a test balloon to git-compat-util.h to see if anyone is using
an older compiler.  We'll add a comment telling people how to enable
this functionality on GCC and Clang, even though modern versions of both
will automatically do the right thing, and ask people still experiencing
a problem to report that to us on the list.

Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
use a well-known hack of using "- 0".  On compilers with this macro, it
doesn't change the value, and on C89 compilers, the macro will be
replaced with nothing, and our value will be 0.

Sparse is also updated with a reference to the gnu99 standard, without
which it defaults to C89.

Update the cmake configuration to require C11 for MSVC.  We do this
because this will make MSVC to use C11, since it does not explicitly
support C99.  We do this with a compiler options because setting the
C_STANDARD option does not work in our CI on MSVC and at the moment, we
don't want to require C11 for Unix compilers.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Makefile                            |  4 ++--
 contrib/buildsystems/CMakeLists.txt |  3 +--
 git-compat-util.h                   | 12 ++++++++++++
 3 files changed, 15 insertions(+), 4 deletions(-)

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
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
 BASIC_CFLAGS = -I.
@@ -1215,7 +1215,7 @@ ARFLAGS = rcs
 PTHREAD_CFLAGS =
 
 # For the 'sparse' target
-SPARSE_FLAGS ?=
+SPARSE_FLAGS ?= -std=gnu99
 SP_EXTRA_FLAGS = -Wno-universal-initializer
 
 # For informing GIT-BUILD-OPTIONS of the SANITIZE=leak target
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index fd1399c440..91e8525fa9 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -208,7 +208,7 @@ endif()
 if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
-	add_compile_options(/MP)
+	add_compile_options(/MP /std:c11)
 endif()
 
 #default behaviour
@@ -600,7 +600,6 @@ endif()
 list(REMOVE_DUPLICATES excluded_progs)
 list(REMOVE_DUPLICATES PROGRAMS_BUILT)
 
-
 foreach(p ${excluded_progs})
 	list(APPEND EXCLUSION_PROGS --exclude-program ${p} )
 endforeach()
diff --git a/git-compat-util.h b/git-compat-util.h
index d70ce14286..6d995bdc0f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1,6 +1,18 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#if __STDC_VERSION__ - 0 < 199901L
+/*
+ * Git is in a testing period for mandatory C99 support in the compiler.  If
+ * your compiler is reasonably recent, you can try to enable C99 support (or,
+ * for MSVC, C11 support).  If you encounter a problem and can't enable C99
+ * support with your compiler and don't have access to one with this support,
+ * such as GCC or Clang, you can remove this #if directive, but please report
+ * the details of your system to git@vger.kernel.org.
+ */
+#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 /*
  * For these to work they must appear very early in each

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

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 14 2021, brian m. carlson wrote:

> The C99 standard was released in January 1999, now 22 years ago.  It
> provides a variety of useful features, including variadic arguments for
> macros, declarations after statements, variable length arrays, and a
> wide variety of other useful features, many of which we already use.
>
> We'd like to take advantage of these features, but we want to be
> cautious.  As far as we know, all major compilers now support C99 or a
> later C standard, such as C11 or C17.  POSIX has required C99 support as
> a requirement for the 2001 revision, so we can safely assume any POSIX
> system which we are interested in supporting has C99.

I like this direction.

> Sparse is also updated with a reference to the gnu99 standard, without
> which it defaults to C89.

Do we really need it in SPARSE_FLAGS though...

> @@ -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
>  LDFLAGS =
>  CC_LD_DYNPATH = -Wl,-rpath,
>  BASIC_CFLAGS = -I.
> @@ -1215,7 +1215,7 @@ ARFLAGS = rcs
>  PTHREAD_CFLAGS =

Since $(CFLAGS) ends up in:

    ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)

...

>  # For the 'sparse' target
> -SPARSE_FLAGS ?=
> +SPARSE_FLAGS ?= -std=gnu99
>  SP_EXTRA_FLAGS = -Wno-universal-initializer

... and this will be used for this rule:

$(SP_OBJ): %.sp: %.c %.o
        $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
                -Wsparse-error \
                $(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< [...]

I.e. unless it needs to be later on the command-line the $(ALL_CFLAGS)
should put it there already.

Also (and this pre-dates this patch) it's unfortunate that CFLAGS is a
mixed bag of compiler tweaking and "mandatory" flags. I think the below
would be a better approach, particurly since our own config.mak.uname
will override CFLAGS in some cases, and probably everyone who works on
git to any degree has a local config.mak which sets it to something
already.

But why gnu99 and not c99?

diff --git a/Makefile b/Makefile
index 12be39ac497..7470d627165 100644
--- a/Makefile
+++ b/Makefile
@@ -1204,10 +1204,14 @@ 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.
+#
+# The MANDATORY_CFLAGS can be similarly overridden, but really
+# shouldn't.
 CFLAGS = -g -O2 -Wall
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
-BASIC_CFLAGS = -I.
+BASIC_CFLAGS =
+MANDATORY_CFLAGS = -I. -std=gnu99
 BASIC_LDFLAGS =
 
 # library flags
@@ -1249,7 +1253,7 @@ ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)
 ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)
 endif
 
-ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
+ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(MANDATORY_CFLAGS)
 ALL_LDFLAGS = $(LDFLAGS)
 
 comma := ,

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3380 bytes --]

On 2021-11-15 at 01:14:42, Ævar Arnfjörð Bjarmason wrote:
> 
> On Sun, Nov 14 2021, brian m. carlson wrote:
> 
> > The C99 standard was released in January 1999, now 22 years ago.  It
> > provides a variety of useful features, including variadic arguments for
> > macros, declarations after statements, variable length arrays, and a
> > wide variety of other useful features, many of which we already use.
> >
> > We'd like to take advantage of these features, but we want to be
> > cautious.  As far as we know, all major compilers now support C99 or a
> > later C standard, such as C11 or C17.  POSIX has required C99 support as
> > a requirement for the 2001 revision, so we can safely assume any POSIX
> > system which we are interested in supporting has C99.
> 
> I like this direction.

I felt like a test balloon would go over better than a wholesale
changeover.  I feel confident we can make this change, and we may even
in the not too distant future be able to switch to C11.

> > Sparse is also updated with a reference to the gnu99 standard, without
> > which it defaults to C89.
> 
> Do we really need it in SPARSE_FLAGS though...
> 
> > @@ -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
> >  LDFLAGS =
> >  CC_LD_DYNPATH = -Wl,-rpath,
> >  BASIC_CFLAGS = -I.
> > @@ -1215,7 +1215,7 @@ ARFLAGS = rcs
> >  PTHREAD_CFLAGS =
> 
> Since $(CFLAGS) ends up in:
> 
>     ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
> 
> ...
> 
> >  # For the 'sparse' target
> > -SPARSE_FLAGS ?=
> > +SPARSE_FLAGS ?= -std=gnu99
> >  SP_EXTRA_FLAGS = -Wno-universal-initializer
> 
> ... and this will be used for this rule:
> 
> $(SP_OBJ): %.sp: %.c %.o
>         $(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
>                 -Wsparse-error \
>                 $(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) $< [...]
> 
> I.e. unless it needs to be later on the command-line the $(ALL_CFLAGS)
> should put it there already.

I added it to SPARSE_FLAGS before adding it into CFLAGS, so I can look
into dropping it from the former.

> Also (and this pre-dates this patch) it's unfortunate that CFLAGS is a
> mixed bag of compiler tweaking and "mandatory" flags. I think the below
> would be a better approach, particurly since our own config.mak.uname
> will override CFLAGS in some cases, and probably everyone who works on
> git to any degree has a local config.mak which sets it to something
> already.

We don't want to do this, because some people are using other compilers
(i.e., neither GCC nor clang) that need different options.  We
definitely do want them to be able to override these values as
necessary.  I believe config.mak.uname does this in some cases for
certain Unix systems.

> But why gnu99 and not c99?

I'll explain in the commit message in a reroll, but essentially, because
we do in some case use GNUisms when we're working with GCC.  Right now
those are mostly limited to providing C99 features on C89, but I'd like
to leave the opportunity open for us to do this in the future.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

[Eric Sunshine]

On Sun, Nov 14, 2021 at 4:27 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> +#if __STDC_VERSION__ - 0 < 199901L
> +/*
> + * Git is in a testing period for mandatory C99 support in the compiler.  If
> + * your compiler is reasonably recent, you can try to enable C99 support (or,
> + * for MSVC, C11 support).  If you encounter a problem and can't enable C99
> + * support with your compiler and don't have access to one with this support,
> + * such as GCC or Clang, you can remove this #if directive, but please report
> + * the details of your system to git@vger.kernel.org.
> + */
> +#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."

You don't need to encapsulate the #error message in double quotes.

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

On 2021-11-15 at 03:16:02, Eric Sunshine wrote:
> On Sun, Nov 14, 2021 at 4:27 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > +#if __STDC_VERSION__ - 0 < 199901L
> > +/*
> > + * Git is in a testing period for mandatory C99 support in the compiler.  If
> > + * your compiler is reasonably recent, you can try to enable C99 support (or,
> > + * for MSVC, C11 support).  If you encounter a problem and can't enable C99
> > + * support with your compiler and don't have access to one with this support,
> > + * such as GCC or Clang, you can remove this #if directive, but please report
> > + * the details of your system to git@vger.kernel.org.
> > + */
> > +#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
> 
> You don't need to encapsulate the #error message in double quotes.

Technically, I believe in this case you are correct.  The C standard
specifies this as pp-tokens, which means one or more preprocessing
tokens, and from my brief overview of the draft standard, it appears
that this meets that definition.  (I could be wrong, though.)

_However_, there are some cases where quoting is required, such as when
apostrophes appear, and although we don't have that case here, there are
some compilers which are very strict about what they do or don't allow
in an #error statement, or which are just broken, and as such, in my
experience, it is safer and more portable to always quote it.  I
definitely don't want us to have the problem that we break an otherwise
functional compiler by causing it to choke on the #error directive it
doesn't need to execute.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

[Johannes Schindelin]

Hi brian,

On Sun, 14 Nov 2021, brian m. carlson wrote:

> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fd1399c440..91e8525fa9 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -208,7 +208,7 @@ endif()
>  if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
> -	add_compile_options(/MP)
> +	add_compile_options(/MP /std:c11)
>  endif()
>
>  #default behaviour

If we do this, we also need the following patch, to ensure that
`FLEX_ARRAY` is once again defined in a way that is acceptable for MS
Visual C:

-- snipsnap --
commit 9043470c19170643d1f74d6767a0df5d72e6c35c
Author: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date:   Mon Nov 22 12:39:40 2021 +0100

    fixup??? git-compat-util: add a test balloon for C99 support

    This seems to be required to define `FLEX_ARRAY` in a manner that MSVC
    in C11 mode accepts.

    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

diff --git a/git-compat-util.h b/git-compat-util.h
index cba8ad260043..fcde3588172f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -45,7 +45,7 @@
 /*
  * See if our compiler is known to support flexible array members.
  */
-#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && (!defined(__SUNPRO_C) || (__SUNPRO_C > 0x580))
+#if defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L) && !defined(_MSC_VER) && (!defined(__SUNPRO_C) || (__SUNPRO_C > 0x580))
 # define FLEX_ARRAY /* empty */
 #elif defined(__GNUC__)
 # if (__GNUC__ >= 3)

Re: [PATCH 0/1] Add a test balloon for C99

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

On 2021-11-14 at 21:24:36, brian m. carlson wrote:
> brian m. carlson (1):
>   git-compat-util: add a test balloon for C99 support
> 
>  Makefile                            |  4 ++--
>  contrib/buildsystems/CMakeLists.txt |  3 +--
>  git-compat-util.h                   | 12 ++++++++++++
>  3 files changed, 15 insertions(+), 4 deletions(-)

Note that vger has decided to firewall my old MX, so I've resent this
through a new one.  If you received a CC on this, it's a different
email.

This should be fixed for the future.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 0/1] Add a test balloon for C99

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Even for users who would like to target an older version of Windows,
> such as the no longer supported Windows 7, GCC and Clang are available.
> The LLVM suite, including Clang, is available pre-compiled for download
> free of charge.  Using a different compiler is specifically proposed by
> Microsoft staff[1] as a solution for users who wish to build modern
> programs for MSVC versions which do not support modern C.
>
> As such, we can assume that Git can be safely compiled with C99 or C11
> support on all operating systems which receive security support, and
> even some which do not.  Our CI confirms that this series passes all
> tests.  Let's introduce a test balloon which checks for this support and
> fails with an error message if it is absent.

I do not have much against adopting nicer C99 language features in
principle, but I hope that we are not becoming too Linux centric
with "other than Linux, as long as Windows is OK in some form,
everything is peachy" mentality.

If there is a rogue implementation that claims to do C99 with
STDC_VERSION without properly supporting some language constructs we
care about, that would be bad.  That is why I tend to prefer the
approach we have taken so far, validating selected features we care
about one by one with pointed weather balloon tests, over "the
compiler says it supports that version, so we trust them".

Perhaps we can do this, and a more pointed "break compilers without
variable decl in for() loop" change, at the same time.  After the
latter turns out to be noise free, we can update CodingGuidelines.

Even when we clear C99 as the whole, I'd be hesitant to allow
certain things from the language (not because compilers do not grok
them, but purely from style and readability point of view).  For
example, -Wdecl-after-statement is a good discipline to follow even
if your compiler allows them.

Thanks.


 Documentation/CodingGuidelines | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git c/Documentation/CodingGuidelines w/Documentation/CodingGuidelines
index 45465bc0c9..5c32b29949 100644
--- c/Documentation/CodingGuidelines
+++ w/Documentation/CodingGuidelines
@@ -205,15 +205,16 @@ For C programs:
    . since mid 2017 with 512f41cf, we have been using designated
      initializers for array (e.g. "int array[10] = { [5] = 2 }").
 
+   . since early 2022 with YYYYYYY, we have been using variable
+     declaration in the for loop (e.g. "for (int i = 0; i < 10;
+     i++)").
+
    These used to be forbidden, but we have not heard any breakage
    report, and they are assumed to be safe.
 
  - Variables have to be declared at the beginning of the block, before
    the first statement (i.e. -Wdeclaration-after-statement).
 
- - Declaring a variable in the for loop "for (int i = 0; i < 10; i++)"
-   is still not allowed in this codebase.
-
  - NULL pointers shall be written as NULL, not as 0.
 
  - When declaring pointers, the star sides with the variable

Re: [PATCH 0/1] Add a test balloon for C99

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 3924 bytes --]

On 2021-11-15 at 07:00:25, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > Even for users who would like to target an older version of Windows,
> > such as the no longer supported Windows 7, GCC and Clang are available.
> > The LLVM suite, including Clang, is available pre-compiled for download
> > free of charge.  Using a different compiler is specifically proposed by
> > Microsoft staff[1] as a solution for users who wish to build modern
> > programs for MSVC versions which do not support modern C.
> >
> > As such, we can assume that Git can be safely compiled with C99 or C11
> > support on all operating systems which receive security support, and
> > even some which do not.  Our CI confirms that this series passes all
> > tests.  Let's introduce a test balloon which checks for this support and
> > fails with an error message if it is absent.
> 
> I do not have much against adopting nicer C99 language features in
> principle, but I hope that we are not becoming too Linux centric
> with "other than Linux, as long as Windows is OK in some form,
> everything is peachy" mentality.

It's definitely not my goal to exclude Windows here.  I'm pretty sure
every major Unix platform will handle this fine, and an up to date
MSVC will also handle this fine.

Because compiling Git for Windows is a lot of work (not due to any
failing of that project or its members, just the fact that it requires a
lot of components to be assembled, including a full POSIX environment),
it's not very likely we're going to see a lot of people doing it, since
almost all Windows users are going to be using the regular releases.
It's also likely that they're going to be using some automated CI system
which will likely support a recent version of the compiler.

However, we have in the past heard screaming from people who want to
support old versions of Windows, so my point here is that there are
options if they can't use MSVC for any reason and those options are
easy, accessible, and free of charge.  I should point out that we
already require people on non-Linux Unix systems to install GNU make and
possibly also a shell (if theirs doesn't support the local keyword), so
installing suitable tooling to build Git isn't without precedent.

> If there is a rogue implementation that claims to do C99 with
> STDC_VERSION without properly supporting some language constructs we
> care about, that would be bad.  That is why I tend to prefer the
> approach we have taken so far, validating selected features we care
> about one by one with pointed weather balloon tests, over "the
> compiler says it supports that version, so we trust them".

I think this is going to be unlikely.  People can and do expect to rely
on __STDC_VERSION__ working properly, and it's likely any compiler which
lacked those features has long been fixed.

> Perhaps we can do this, and a more pointed "break compilers without
> variable decl in for() loop" change, at the same time.  After the
> latter turns out to be noise free, we can update CodingGuidelines.

As I mentioned in another thread, we'll need a patch like this first to
enable proper handling for older compilers, but I think that should be
fine on top.

> Even when we clear C99 as the whole, I'd be hesitant to allow
> certain things from the language (not because compilers do not grok
> them, but purely from style and readability point of view).  For
> example, -Wdecl-after-statement is a good discipline to follow even
> if your compiler allows them.

I think specifically -Wdecl-after-statement depends on the situation.
There are many cases where it's less error prone or easier to read if
the variable is declared immediately before it's used.

Regardless, I don't think we need to decide this now.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH 0/1] Add a test balloon for C99

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2021-11-15 at 07:00:25, Junio C Hamano wrote:
>> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>> 
>> > Even for users who would like to target an older version of Windows,
>> > such as the no longer supported Windows 7, GCC and Clang are available.
>> > The LLVM suite, including Clang, is available pre-compiled for download
>> > free of charge.  Using a different compiler is specifically proposed by
>> > Microsoft staff[1] as a solution for users who wish to build modern
>> > programs for MSVC versions which do not support modern C.
>> >
>> > As such, we can assume that Git can be safely compiled with C99 or C11
>> > support on all operating systems which receive security support, and
>> > even some which do not.  Our CI confirms that this series passes all
>> > tests.  Let's introduce a test balloon which checks for this support and
>> > fails with an error message if it is absent.
>> 
>> I do not have much against adopting nicer C99 language features in
>> principle, but I hope that we are not becoming too Linux centric
>> with "other than Linux, as long as Windows is OK in some form,
>> everything is peachy" mentality.
>
> It's definitely not my goal to exclude Windows here.  I'm pretty sure
> every major Unix platform will handle this fine, and an up to date
> MSVC will also handle this fine.
>
> Because compiling Git for Windows is a lot of work (not due to any
> failing of that project or its members, just the fact that it requires a
> lot of components to be assembled, including a full POSIX environment),
> it's not very likely we're going to see a lot of people doing it, since
> almost all Windows users are going to be using the regular releases.
> It's also likely that they're going to be using some automated CI system
> which will likely support a recent version of the compiler.
>
> However, we have in the past heard screaming from people who want to
> support old versions of Windows, so my point here is that there are
> options if they can't use MSVC for any reason and those options are
> easy, accessible, and free of charge.  I should point out that we
> already require people on non-Linux Unix systems to install GNU make and
> possibly also a shell (if theirs doesn't support the local keyword), so
> installing suitable tooling to build Git isn't without precedent.

Windows does not need me to worry about them---they can fend for
themselves.  I cannot tell if the original discussion behind the
patch considered the current situation in non-mac BSD land (which I
am not familiar with), or even less common platforms like NonStop.

Re: [PATCH 0/1] Add a test balloon for C99

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On 2021-11-16 at 19:02:08, Junio C Hamano wrote:
> Windows does not need me to worry about them---they can fend for
> themselves.  I cannot tell if the original discussion behind the
> patch considered the current situation in non-mac BSD land (which I
> am not familiar with), or even less common platforms like NonStop.

Oh, I did full well consider BSD land.  They are generally up on the
latest standards, and I would even say that most proprietary Unix
systems are also far enough along from what I know of them.  I do try
specifically to support BSD systems and consider their needs to the best
of my abilities.

As for NonStop, I checked, and it already forces C99 mode because we use
enough features, like inline, that C89 doesn't support and so it needs
to do that already.

I do appreciate you looking out for them, though.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[PATCH v2 0/1] Add a test balloon for C99 support

[brian m. carlson]

There seems to be a decent amount of interest in moving to C99 as our
base level of C support due to the many features it provides.  Let's add
a test balloon to make sure we haven't accidentally excluded any
systems.

Changes from v1:
* Don't modify SPARSE_FLAGS since it's unnecessary.
* Improve commit message to explain why we use -std=gnu99.

brian m. carlson (1):
  git-compat-util: add a test balloon for C99 support

 Makefile                            |  2 +-
 contrib/buildsystems/CMakeLists.txt |  2 +-
 git-compat-util.h                   | 12 ++++++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

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

[brian m. carlson]

The C99 standard was released in January 1999, now 22 years ago.  It
provides a variety of useful features, including variadic arguments for
macros, declarations after statements, variable length arrays, and a
wide variety of other useful features, many of which we already use.

We'd like to take advantage of these features, but we want to be
cautious.  As far as we know, all major compilers now support C99 or a
later C standard, such as C11 or C17.  POSIX has required C99 support as
a requirement for the 2001 revision, so we can safely assume any POSIX
system which we are interested in supporting has C99.

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.

Let's add a test balloon to git-compat-util.h to see if anyone is using
an older compiler.  We'll add a comment telling people how to enable
this functionality on GCC and Clang, even though modern versions of both
will automatically do the right thing, and ask people still experiencing
a problem to report that to us on the list.

Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
use a well-known hack of using "- 0".  On compilers with this macro, it
doesn't change the value, and on C89 compilers, the macro will be
replaced with nothing, and our value will be 0.

We explicitly request the gnu99 style because we've traditionally taken
advantage of some GCC- and clang-specific extensions when available and
we'd like to retain the ability to do that, especially now that we're
building with -pedantic in developer mode.  Older compilers may in fact
support C99 just fine, but default to C89 without an option, so the
option is necessary.  Allow it to be overridden by the user or
config.mak.uname to ensure that people using other compilers can make it
work.

Update the cmake configuration to require C11 for MSVC.  We do this
because this will make MSVC to use C11, since it does not explicitly
support C99.  We do this with a compiler options because setting the
C_STANDARD option does not work in our CI on MSVC and at the moment, we
don't want to require C11 for Unix compilers.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Makefile                            |  2 +-
 contrib/buildsystems/CMakeLists.txt |  2 +-
 git-compat-util.h                   | 12 ++++++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 12be39ac49..893d533d22 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
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
 BASIC_CFLAGS = -I.
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index fd1399c440..07b6c5494b 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -208,7 +208,7 @@ endif()
 if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
-	add_compile_options(/MP)
+	add_compile_options(/MP /std:c11)
 endif()
 
 #default behaviour
diff --git a/git-compat-util.h b/git-compat-util.h
index d70ce14286..6d995bdc0f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1,6 +1,18 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#if __STDC_VERSION__ - 0 < 199901L
+/*
+ * Git is in a testing period for mandatory C99 support in the compiler.  If
+ * your compiler is reasonably recent, you can try to enable C99 support (or,
+ * for MSVC, C11 support).  If you encounter a problem and can't enable C99
+ * support with your compiler and don't have access to one with this support,
+ * such as GCC or Clang, you can remove this #if directive, but please report
+ * the details of your system to git@vger.kernel.org.
+ */
+#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 /*
  * For these to work they must appear very early in each

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

[Jeff King]

On Tue, Nov 16, 2021 at 02:12:41AM +0000, brian m. carlson wrote:

> The C99 standard was released in January 1999, now 22 years ago.  It
> provides a variety of useful features, including variadic arguments for
> macros, declarations after statements, variable length arrays, and a
> wide variety of other useful features, many of which we already use.

I like the idea of being able to assume C99. And I know this list is
just "here are some things we could do". But I'd like to express caution
over variable length arrays. We've already had problems with alloca()
causing stack exhaustion, and VLAs are basically the same thing. And the
worst part is there's no way to recover; you just get a segfault.

> Let's add a test balloon to git-compat-util.h to see if anyone is using
> an older compiler.  We'll add a comment telling people how to enable
> this functionality on GCC and Clang, even though modern versions of both
> will automatically do the right thing, and ask people still experiencing
> a problem to report that to us on the list.
> 
> Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
> use a well-known hack of using "- 0".  On compilers with this macro, it
> doesn't change the value, and on C89 compilers, the macro will be
> replaced with nothing, and our value will be 0.

This part makes sense to me. The macro check will complain if any
compiler isn't C99. But this hunk seems like it is going to cause
headaches:

> diff --git a/Makefile b/Makefile
> index 12be39ac49..893d533d22 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
>  LDFLAGS =
>  CC_LD_DYNPATH = -Wl,-rpath,
>  BASIC_CFLAGS = -I.

Do most compilers understand -std=gnu99? It seems like we're breaking
the out-of-the-box build for everything that isn't gcc or clang.

I understand that older versions of gcc (prior to 5.1.0, from my
digging) default to gnu89, and so they would be broken _without_ this.
So it is a tradeoff one way or the other. But somehow this seems
backwards to me. We should assume that modern compilers support C99 out
of the box, and put the burden on older ones to trigger C99 support in
whatever non-portable way they need.

I also checked clang, and it looks like it has defaulted to gnu11 since
clang-7 (I'm not sure about clang-6; its documentation wasn't clear).

-Peff

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

[Ævar Arnfjörð Bjarmason]

On Tue, Nov 16 2021, Jeff King wrote:

> On Tue, Nov 16, 2021 at 02:12:41AM +0000, brian m. carlson wrote:
[...]
>> diff --git a/Makefile b/Makefile
>> index 12be39ac49..893d533d22 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
>>  LDFLAGS =
>>  CC_LD_DYNPATH = -Wl,-rpath,
>>  BASIC_CFLAGS = -I.
>
> Do most compilers understand -std=gnu99? It seems like we're breaking
> the out-of-the-box build for everything that isn't gcc or clang.
>
> I understand that older versions of gcc (prior to 5.1.0, from my
> digging) default to gnu89, and so they would be broken _without_ this.
> So it is a tradeoff one way or the other. But somehow this seems
> backwards to me. We should assume that modern compilers support C99 out
> of the box, and put the burden on older ones to trigger C99 support in
> whatever non-portable way they need.
>
> I also checked clang, and it looks like it has defaulted to gnu11 since
> clang-7 (I'm not sure about clang-6; its documentation wasn't clear).

Yes, this seems like something we'd really want to feed to
"detect-compiler" after extracting it out of config.mak.dev.

And as you note it's not only that older or non-gcc non-clang compilers
may not understand this at all, but are we getting worse behavior on
modern versions of those two because they're forced into some 20 year
old C99 standard mode, instead of the current preferred default?

If we do go for this there's also the additional breakage I mentioned
upthread of th [1], and which I think I wasn't clear enough about. I.e. if we do:

    CFLAGS = -O2 -g -std=whatever

And then in config.mak.uname do e.g.:

    CFLAGS = -g -O0 -Winline

We'll override the -std=whatever, but just wanted to overrride the -O2.

This came up in another context recently (Carlo had series to hack in
this area), but I think we shouldn't add anything new to CFLAGS. Let's
instead add a new variable like BASIC_CFLAGS, so in this case
CFLAGS_C_STANDARD or whatever, and then later in the file:

    -ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
    +ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_C_STANDARD)

But IMO this whole thing of trying to make this work on every compiler
etc. just isn't worth it.

Let's just start using C99 features, and if anyone's compiler breaks on
something like CentOS 6 document that they'll need to tweak a flag
somewhere. We already know that works for all the other C99 features we
have, there seems to just be this one exception of the ancient GCC
version in this particular case.

1. https://lore.kernel.org/git/YZG9wF56unj7eYhl@camp.crustytoothpaste.net/

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

[Jeff King]

On Tue, Nov 16, 2021 at 01:54:27PM +0100, Ævar Arnfjörð Bjarmason wrote:

> But IMO this whole thing of trying to make this work on every compiler
> etc. just isn't worth it.
> 
> Let's just start using C99 features, and if anyone's compiler breaks on
> something like CentOS 6 document that they'll need to tweak a flag
> somewhere. We already know that works for all the other C99 features we
> have, there seems to just be this one exception of the ancient GCC
> version in this particular case.

Yeah, I definitely agree with this sentiment.

-Peff

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

On 2021-11-16 at 14:54:32, Jeff King wrote:
> On Tue, Nov 16, 2021 at 01:54:27PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> > But IMO this whole thing of trying to make this work on every compiler
> > etc. just isn't worth it.
> > 
> > Let's just start using C99 features, and if anyone's compiler breaks on
> > something like CentOS 6 document that they'll need to tweak a flag
> > somewhere. We already know that works for all the other C99 features we
> > have, there seems to just be this one exception of the ancient GCC
> > version in this particular case.
> 
> Yeah, I definitely agree with this sentiment.

Unfortunately, we cannot do this without some sort of patch because our
CI will be broken.  I'm fine if we want to drop GCC 4.8, but we need to
be explicit about that or we need to add flags to make it work.  We also
need at least something to make Windows work.  Dscho will not be happy
if we just leave it broken.

So I think some version of this patch or something similar needs to be
adopted.  I'll try to get a v3 out relatively soon that fixes these
issues and drops -std=gnu99 from CFLAGS, which seems to be the approach
people are most desirous of.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

[Jeff King]

On Wed, Nov 17, 2021 at 02:53:33AM +0000, brian m. carlson wrote:

> On 2021-11-16 at 14:54:32, Jeff King wrote:
> > On Tue, Nov 16, 2021 at 01:54:27PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > 
> > > But IMO this whole thing of trying to make this work on every compiler
> > > etc. just isn't worth it.
> > > 
> > > Let's just start using C99 features, and if anyone's compiler breaks on
> > > something like CentOS 6 document that they'll need to tweak a flag
> > > somewhere. We already know that works for all the other C99 features we
> > > have, there seems to just be this one exception of the ancient GCC
> > > version in this particular case.
> > 
> > Yeah, I definitely agree with this sentiment.
> 
> Unfortunately, we cannot do this without some sort of patch because our
> CI will be broken.  I'm fine if we want to drop GCC 4.8, but we need to
> be explicit about that or we need to add flags to make it work.  We also
> need at least something to make Windows work.  Dscho will not be happy
> if we just leave it broken.

Yes, but I'm not at all worried about breaking our CI. That's just a
patch away from fixing. I'm much more worried about confused users
building from source, because helping them is more difficult to scale.

My thinking was that breaking older compilers was preferable to breaking
non-gnu ones, because at least old ones go away eventually. But your
other email makes me wonder if those non-GNU ones may already be
overriding CFLAGS.

Still, if we can come up with a solution that breaks neither (with some
light auto-detection or heuristics in the Makefile), that could be the
best of both worlds.

-Peff

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]

On 2021-11-17 at 03:01:57, Jeff King wrote:
> Yes, but I'm not at all worried about breaking our CI. That's just a
> patch away from fixing. I'm much more worried about confused users
> building from source, because helping them is more difficult to scale.

That's one of the reasons I had proposed the current patch, because it
pukes in a very noticeable way with directives on where to look to
continue.  Just using C99 features means that Git breaks in a very
subtle way where the user compiling may not be familiar with C and may
not know how to fix it otherwise.  For example, my previous employer
ships Git, but many of the folks who are doing the package updates are
not C programmers.

> My thinking was that breaking older compilers was preferable to breaking
> non-gnu ones, because at least old ones go away eventually. But your
> other email makes me wonder if those non-GNU ones may already be
> overriding CFLAGS.

Our only problem platform, as far as I can tell, is RHEL/CentOS 7.  That
uses GCC 4.8, and even Ubuntu 18.04 ships with GCC 7.

> Still, if we can come up with a solution that breaks neither (with some
> light auto-detection or heuristics in the Makefile), that could be the
> best of both worlds.

I can move COMPILER_FEATURES out of config.mak.dev and into Makefile so
that we can make use of it.  We'll need to depend on GCC 6 for this
because we lack a way to distinguish 5.1 (which should work) from 5.0
(which will not).
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

[Carlo Arenas]

On Wed, Nov 17, 2021 at 3:18 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> On 2021-11-17 at 03:01:57, Jeff King wrote:
> > My thinking was that breaking older compilers was preferable to breaking
> > non-gnu ones, because at least old ones go away eventually. But your
> > other email makes me wonder if those non-GNU ones may already be
> > overriding CFLAGS.
>
> Our only problem platform, as far as I can tell, is RHEL/CentOS 7.  That
> uses GCC 4.8, and even Ubuntu 18.04 ships with GCC 7.

There are several odd BSD platforms that are still stuck in pre-GPLv3
gcc (AKA gcc 4.2.1) like OpenBSD Alpha, hppa, landisk (and maybe also
SPARC64 which is tier1) and that will need the same, there is indeed
also luna88k that uses an even older gcc but hopefully will be able to
work if it understands enough C99 and can be told to use it by this
flag.

> > Still, if we can come up with a solution that breaks neither (with some
> > light auto-detection or heuristics in the Makefile), that could be the
> > best of both worlds.
>
> I can move COMPILER_FEATURES out of config.mak.dev and into Makefile so
> that we can make use of it.  We'll need to depend on GCC 6 for this
> because we lack a way to distinguish 5.1 (which should work) from 5.0
> (which will not).

5.0 works AFAIK, is anything older than 5 than does not as reported[1]
before, but it won't be still a good fit, since it only works for gcc
and clang AS-IS.

Carlo

[1] https://lore.kernel.org/git/CAPUEsphnCvK+RZ+h30ZarA1zo9yZ=ndEBrcAbKGf4W92j647vA@mail.gmail.com/

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

[Ævar Arnfjörð Bjarmason]

On Wed, Nov 17 2021, Carlo Arenas wrote:

> On Wed, Nov 17, 2021 at 3:18 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
>>
>> On 2021-11-17 at 03:01:57, Jeff King wrote:
>> > My thinking was that breaking older compilers was preferable to breaking
>> > non-gnu ones, because at least old ones go away eventually. But your
>> > other email makes me wonder if those non-GNU ones may already be
>> > overriding CFLAGS.
>>
>> Our only problem platform, as far as I can tell, is RHEL/CentOS 7.  That
>> uses GCC 4.8, and even Ubuntu 18.04 ships with GCC 7.
>
> There are several odd BSD platforms that are still stuck in pre-GPLv3
> gcc (AKA gcc 4.2.1) like OpenBSD Alpha, hppa, landisk (and maybe also
> SPARC64 which is tier1) and that will need the same, there is indeed
> also luna88k that uses an even older gcc but hopefully will be able to
> work if it understands enough C99 and can be told to use it by this
> flag.
>
>> > Still, if we can come up with a solution that breaks neither (with some
>> > light auto-detection or heuristics in the Makefile), that could be the
>> > best of both worlds.
>>
>> I can move COMPILER_FEATURES out of config.mak.dev and into Makefile so
>> that we can make use of it.  We'll need to depend on GCC 6 for this
>> because we lack a way to distinguish 5.1 (which should work) from 5.0
>> (which will not).
>
> 5.0 works AFAIK, is anything older than 5 than does not as reported[1]
> before, but it won't be still a good fit, since it only works for gcc
> and clang AS-IS.
>
> Carlo
>
> [1] https://lore.kernel.org/git/CAPUEsphnCvK+RZ+h30ZarA1zo9yZ=ndEBrcAbKGf4W92j647vA@mail.gmail.com/

Rather than moving around COMPILER_FEATURES etc. we can just compile a C
program as part of our Makefile auto-configuration. See the direction
suggested in:
https://lore.kernel.org/git/87bl6aypke.fsf@evledraar.gmail.com/

That example is ad-hoc, but the right way to do this is:

 1. Stick a C program somewhere, maybe git-autoconf/compiler.c 
 2. (Try to) Compile that unconditionally
 3. Emit its output to a generated file that we then "include", which
    likewise if it fails indicate that in something the Makefile can
    "include".

Since we set up that file-based dependency relationship we'll only do
that auto-detection on the first build.

This is really much simpler than fiddling with the version parsing
shellscript, i.e. we can just compile a program with -std=c99 or
whatever and see if it works, and if it does we stick that flag in
CFLAGS or equivalent.

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

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On 2021-11-17 at 03:01:57, Jeff King wrote:
>> Yes, but I'm not at all worried about breaking our CI. That's just a
>> patch away from fixing. I'm much more worried about confused users
>> building from source, because helping them is more difficult to scale.
>
> That's one of the reasons I had proposed the current patch, because it
> pukes in a very noticeable way with directives on where to look to
> continue.  Just using C99 features means that Git breaks in a very
> subtle way where the user compiling may not be familiar with C and may
> not know how to fix it otherwise.  For example, my previous employer
> ships Git, but many of the folks who are doing the package updates are
> not C programmers.

I wonder if this would work, then.

 Makefile   | 3 ++-
 revision.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git c/Makefile w/Makefile
index 201437e9d4..454118d86b 100644
--- c/Makefile
+++ w/Makefile
@@ -1218,7 +1218,8 @@ 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 -std=gnu99
+# Older versions of GCC may require adding "-std=gnu99" at the end.
+CFLAGS = -g -O2 -Wall
 LDFLAGS =
 CC_LD_DYNPATH = -Wl,-rpath,
 BASIC_CFLAGS = -I.
diff --git c/revision.c w/revision.c
index 78c1ceea7b..5390a479b3 100644
--- c/revision.c
+++ w/revision.c
@@ -49,7 +49,8 @@ void show_object_with_name(FILE *out, struct object *obj, const char *name)
 	 * This "for (const char *p = ..." is made as a first step towards
 	 * making use of such declarations elsewhere in our codebase.  If
 	 * it causes compilation problems on your platform, please report
-	 * it to the Git mailing list at git@vger.kernel.org.
+	 * it to the Git mailing list at git@vger.kernel.org. In the meantime,
+	 * adding -std=gnu99 to CFLAGS may help if you are with older GCC.
 	 */
 	for (const char *p = name; *p && *p != '\n'; p++)
 		fputc(*p, out);

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

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Nov 16, 2021 at 01:54:27PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> But IMO this whole thing of trying to make this work on every compiler
>> etc. just isn't worth it.
>> 
>> Let's just start using C99 features, and if anyone's compiler breaks on
>> something like CentOS 6 document that they'll need to tweak a flag
>> somewhere. We already know that works for all the other C99 features we
>> have, there seems to just be this one exception of the ancient GCC
>> version in this particular case.
>
> Yeah, I definitely agree with this sentiment.

+1, provided that we are not saying that "we will start using any
random C99 features" without limit.  I do appreciate the way the
previous efforts were conducted, to start with small test balloons
that can easily be backed out and leaving it there for sufficiently
long time.

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

[Phillip Wood]

On 16/11/2021 12:19, Jeff King wrote:
> On Tue, Nov 16, 2021 at 02:12:41AM +0000, brian m. carlson wrote:
> 
>> The C99 standard was released in January 1999, now 22 years ago.  It
>> provides a variety of useful features, including variadic arguments for
>> macros, declarations after statements, variable length arrays, and a
>> wide variety of other useful features, many of which we already use.
> 
> I like the idea of being able to assume C99. And I know this list is
> just "here are some things we could do". But I'd like to express caution
> over variable length arrays. We've already had problems with alloca()
> causing stack exhaustion, and VLAs are basically the same thing. And the
> worst part is there's no way to recover; you just get a segfault.

I agree with this, also C11 and later make variable length array support 
an optional compiler feature which is another reason not to rely on them.

Best Wishes

Phillip

>> Let's add a test balloon to git-compat-util.h to see if anyone is using
>> an older compiler.  We'll add a comment telling people how to enable
>> this functionality on GCC and Clang, even though modern versions of both
>> will automatically do the right thing, and ask people still experiencing
>> a problem to report that to us on the list.
>>
>> Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
>> use a well-known hack of using "- 0".  On compilers with this macro, it
>> doesn't change the value, and on C89 compilers, the macro will be
>> replaced with nothing, and our value will be 0.
> 
> This part makes sense to me. The macro check will complain if any
> compiler isn't C99. But this hunk seems like it is going to cause
> headaches:
> 
>> diff --git a/Makefile b/Makefile
>> index 12be39ac49..893d533d22 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
>>   LDFLAGS =
>>   CC_LD_DYNPATH = -Wl,-rpath,
>>   BASIC_CFLAGS = -I.
> 
> Do most compilers understand -std=gnu99? It seems like we're breaking
> the out-of-the-box build for everything that isn't gcc or clang.
> 
> I understand that older versions of gcc (prior to 5.1.0, from my
> digging) default to gnu89, and so they would be broken _without_ this.
> So it is a tradeoff one way or the other. But somehow this seems
> backwards to me. We should assume that modern compilers support C99 out
> of the box, and put the burden on older ones to trigger C99 support in
> whatever non-portable way they need.
> 
> I also checked clang, and it looks like it has defaulted to gnu11 since
> clang-7 (I'm not sure about clang-6; its documentation wasn't clear).
> 
> -Peff
> 

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

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2501 bytes --]

On 2021-11-16 at 12:19:43, Jeff King wrote:
> On Tue, Nov 16, 2021 at 02:12:41AM +0000, brian m. carlson wrote:
> 
> > The C99 standard was released in January 1999, now 22 years ago.  It
> > provides a variety of useful features, including variadic arguments for
> > macros, declarations after statements, variable length arrays, and a
> > wide variety of other useful features, many of which we already use.
> 
> I like the idea of being able to assume C99. And I know this list is
> just "here are some things we could do". But I'd like to express caution
> over variable length arrays. We've already had problems with alloca()
> causing stack exhaustion, and VLAs are basically the same thing. And the
> worst part is there's no way to recover; you just get a segfault.

Since it looks like I'll be doing a v3, I'll reroll without that.

> > diff --git a/Makefile b/Makefile
> > index 12be39ac49..893d533d22 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
> >  LDFLAGS =
> >  CC_LD_DYNPATH = -Wl,-rpath,
> >  BASIC_CFLAGS = -I.
> 
> Do most compilers understand -std=gnu99? It seems like we're breaking
> the out-of-the-box build for everything that isn't gcc or clang.

I'm pretty sure -Wall is GCC- and clang-specific, as is -Wl,-rpath, so I
think we've already crossed that bridge.  There are places in
config.mak.uname where they're specifically overridden for that reason.

-std=gnu99 (or -std=c99) is absolutely required for sparse, though,
since it defaults to C89 (at least in CI).

> I understand that older versions of gcc (prior to 5.1.0, from my
> digging) default to gnu89, and so they would be broken _without_ this.
> So it is a tradeoff one way or the other. But somehow this seems
> backwards to me. We should assume that modern compilers support C99 out
> of the box, and put the burden on older ones to trigger C99 support in
> whatever non-portable way they need.

We'll have to adjust the CI job that builds with GCC 4.8, but I can do
that.  I just am not eager to hear complaints from people that it
doesn't work out of the box, especially since CentOS 7 is going to hit
this case.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

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

[Jeff King]

On Wed, Nov 17, 2021 at 01:44:23AM +0000, brian m. carlson wrote:

> > Do most compilers understand -std=gnu99? It seems like we're breaking
> > the out-of-the-box build for everything that isn't gcc or clang.
> 
> I'm pretty sure -Wall is GCC- and clang-specific, as is -Wl,-rpath, so I
> think we've already crossed that bridge.  There are places in
> config.mak.uname where they're specifically overridden for that reason.

That's a good point. I guess we don't really have good data on how many
people will be (newly) affected either way.

> > I understand that older versions of gcc (prior to 5.1.0, from my
> > digging) default to gnu89, and so they would be broken _without_ this.
> > So it is a tradeoff one way or the other. But somehow this seems
> > backwards to me. We should assume that modern compilers support C99 out
> > of the box, and put the burden on older ones to trigger C99 support in
> > whatever non-portable way they need.
> 
> We'll have to adjust the CI job that builds with GCC 4.8, but I can do
> that.  I just am not eager to hear complaints from people that it
> doesn't work out of the box, especially since CentOS 7 is going to hit
> this case.

Nor am I. I wonder if there's a way we can make it work out of the box
everywhere. Using detect-compiler more widely would be one way to do
that, though that's a bigger change to the Makefile in general. I kind
of wonder if we could just assume in config.mak.uname that Linux will
always have a compiler that understands -std=gnu99.

-Peff

Microsoft's C language policy (was: [PATCH 0/1] Add a test balloon for C99)

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 14 2021, brian m. carlson wrote:

[Replying to this tidbit in the v1 CL, which isn't in the V2 one]

> Unfortunately, Microsoft for many years refused[1] to support C99 in
> MSVC, and still does not officially do so.  However, they recently added
> support for C11 and C17, which are sufficient for modern programming.
> These require a newer version of MSVC, including an updated SDK.  The
> SDK update is available for download free of charge, and most public CI
> systems have support for both the updated MSVC and the SDK update.
>
> Even for users who would like to target an older version of Windows,
> such as the no longer supported Windows 7, GCC and Clang are available.
> The LLVM suite, including Clang, is available pre-compiled for download
> free of charge.  Using a different compiler is specifically proposed by
> Microsoft staff[1] as a solution for users who wish to build modern
> programs for MSVC versions which do not support modern C.
>
> As such, we can assume that Git can be safely compiled with C99 or C11
> support on all operating systems which receive security support, and
> even some which do not.  Our CI confirms that this series passes all
> tests.  Let's introduce a test balloon which checks for this support and
> fails with an error message if it is absent.
>
> [0] The reader will note that there are people working professionally in
> this industry who were not yet born at the time C99 was ratified.  Thus,
> this occurred quite a long time ago indeed.
> [1] https://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/

I hadn't seen that blog post before I'd read it when going over your
series. I'd assumed that Microsoft was just dragging their feed on C
language support, but that post indicates (from what seems to be as
official of a source as anyone's going to get) that they're intending to
not support C at all, but just some pseudo-C that happens to be a
convenient subset of C++.

However that post is from 2012, and you indicate that they've since
added (full?) support for C11 and C17.

So is this "policy" of supporting some arbitrary subset of C++ at all
current as far as MSVC today goes?

Aside: I just remembered we had a (seemingly abandoned) effort to get git's sources to
compile with a C++ compiler[1], which would be another possible way
forward to using more modern C features that happened to overlap with
C++. Another one would be some sort of coccinelle-powered refactoring or
other source pre-processing...

1. https://lore.kernel.org/git/20180129223728.30569-1-bmwill@google.com/

Re: Microsoft's C language policy (was: [PATCH 0/1] Add a test balloon for C99)

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

On 2021-11-30 at 20:43:15, Ævar Arnfjörð Bjarmason wrote:
> I hadn't seen that blog post before I'd read it when going over your
> series. I'd assumed that Microsoft was just dragging their feed on C
> language support, but that post indicates (from what seems to be as
> official of a source as anyone's going to get) that they're intending to
> not support C at all, but just some pseudo-C that happens to be a
> convenient subset of C++.
> 
> However that post is from 2012, and you indicate that they've since
> added (full?) support for C11 and C17.
>
> So is this "policy" of supporting some arbitrary subset of C++ at all
> current as far as MSVC today goes?

No, it is not.  MSVC should have full C11 and C17 support, at least
according to the documentation I've seen.  They seem to have changed
their mind (maybe the Windows kernel developers complained).
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[PATCH v3 0/1] Add a test balloon for C99 support

[brian m. carlson]

There seems to be a decent amount of interest in moving to C99 as our
base level of C support due to the many features it provides.  Let's add
a test balloon to make sure we haven't accidentally excluded any
systems.

Changes from v2:
* Avoid setting the main CFLAGS to avoid breaking FreeBSD.
* Set SPARSE_FLAGS again since it's now necessary with the main CFLAGS
  not being set.
* Add a mention of "-std=gnu99" to the comment.
Changes from v1:
* Don't modify SPARSE_FLAGS since it's unnecessary.
* Improve commit message to explain why we use -std=gnu99.

brian m. carlson (1):
  git-compat-util: add a test balloon for C99 support

 Makefile                            |  2 +-
 contrib/buildsystems/CMakeLists.txt |  2 +-
 git-compat-util.h                   | 13 +++++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

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

[brian m. carlson]

The C99 standard was released in January 1999, now 22 years ago.  It
provides a variety of useful features, including variadic arguments for
macros, declarations after statements, designated initializers, and a
wide variety of other useful features, many of which we already use.

We'd like to take advantage of these features, but we want to be
cautious.  As far as we know, all major compilers now support C99 or a
later C standard, such as C11 or C17.  POSIX has required C99 support as
a requirement for the 2001 revision, so we can safely assume any POSIX
system which we are interested in supporting has C99.

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.

Let's add a test balloon to git-compat-util.h to see if anyone is using
an older compiler.  We'll add a comment telling people how to enable
this functionality on GCC and Clang, even though modern versions of both
will automatically do the right thing, and ask people still experiencing
a problem to report that to us on the list.

Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
use a well-known hack of using "- 0".  On compilers with this macro, it
doesn't change the value, and on C89 compilers, the macro will be
replaced with nothing, and our value will be 0.

For sparse, we explicitly request the gnu99 style because we've
traditionally taken advantage of some GCC- and clang-specific extensions
when available and we'd like to retain the ability to do that.  sparse
also defaults to C89 without it, so things will fail for us if we don't.

Update the cmake configuration to require C11 for MSVC.  We do this
because this will make MSVC to use C11, since it does not explicitly
support C99.  We do this with a compiler options because setting the
C_STANDARD option does not work in our CI on MSVC and at the moment, we
don't want to require C11 for Unix compilers.

In the Makefile, don't set any compiler flags for the compiler itself,
since on some systems, such as FreeBSD, we actually need C11, and asking
for C99 causes things to fail to compile.  The error message should make
it obvious what's going wrong and allow a user to set the appropriate
option when building in the event they're using a Unix compiler that
doesn't support it by default.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Makefile                            |  2 +-
 contrib/buildsystems/CMakeLists.txt |  2 +-
 git-compat-util.h                   | 13 +++++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 12be39ac49..3d0ce6ddf6 100644
--- a/Makefile
+++ b/Makefile
@@ -1215,7 +1215,7 @@ ARFLAGS = rcs
 PTHREAD_CFLAGS =
 
 # For the 'sparse' target
-SPARSE_FLAGS ?=
+SPARSE_FLAGS ?= -std=gnu99
 SP_EXTRA_FLAGS = -Wno-universal-initializer
 
 # For informing GIT-BUILD-OPTIONS of the SANITIZE=leak target
diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index fd1399c440..07b6c5494b 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -208,7 +208,7 @@ endif()
 if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
 	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
-	add_compile_options(/MP)
+	add_compile_options(/MP /std:c11)
 endif()
 
 #default behaviour
diff --git a/git-compat-util.h b/git-compat-util.h
index d70ce14286..ffe70b570f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1,6 +1,19 @@
 #ifndef GIT_COMPAT_UTIL_H
 #define GIT_COMPAT_UTIL_H
 
+#if __STDC_VERSION__ - 0 < 199901L
+/*
+ * Git is in a testing period for mandatory C99 support in the compiler.  If
+ * your compiler is reasonably recent, you can try to enable C99 support (or,
+ * for MSVC, C11 support).  If you encounter a problem and can't enable C99
+ * support with your compiler (such as with "-std=gnu99") and don't have access
+ * to one with this support, such as GCC or Clang, you can remove this #if
+ * directive, but please report the details of your system to
+ * git@vger.kernel.org.
+ */
+#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
+#endif
+
 #ifdef USE_MSVC_CRTDBG
 /*
  * For these to work they must appear very early in each

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

[Johannes Schindelin]

Hi brian,

On Wed, 1 Dec 2021, brian m. carlson wrote:

> The C99 standard was released in January 1999, now 22 years ago.  It
> provides a variety of useful features, including variadic arguments for
> macros, declarations after statements, designated initializers, and a
> wide variety of other useful features, many of which we already use.
>
> We'd like to take advantage of these features, but we want to be
> cautious.  As far as we know, all major compilers now support C99 or a
> later C standard, such as C11 or C17.  POSIX has required C99 support as
> a requirement for the 2001 revision, so we can safely assume any POSIX
> system which we are interested in supporting has C99.
>
> 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.
>
> Let's add a test balloon to git-compat-util.h to see if anyone is using
> an older compiler.  We'll add a comment telling people how to enable
> this functionality on GCC and Clang, even though modern versions of both
> will automatically do the right thing, and ask people still experiencing
> a problem to report that to us on the list.
>
> Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
> use a well-known hack of using "- 0".  On compilers with this macro, it
> doesn't change the value, and on C89 compilers, the macro will be
> replaced with nothing, and our value will be 0.
>
> For sparse, we explicitly request the gnu99 style because we've
> traditionally taken advantage of some GCC- and clang-specific extensions
> when available and we'd like to retain the ability to do that.  sparse
> also defaults to C89 without it, so things will fail for us if we don't.

I agree with this revision of the patch entirely, especially with this
paragraph.

If there should crop up any Windows-specific issues, I will be glad to
have a look at them.

Thank you,
Dscho

>
> Update the cmake configuration to require C11 for MSVC.  We do this
> because this will make MSVC to use C11, since it does not explicitly
> support C99.  We do this with a compiler options because setting the
> C_STANDARD option does not work in our CI on MSVC and at the moment, we
> don't want to require C11 for Unix compilers.
>
> In the Makefile, don't set any compiler flags for the compiler itself,
> since on some systems, such as FreeBSD, we actually need C11, and asking
> for C99 causes things to fail to compile.  The error message should make
> it obvious what's going wrong and allow a user to set the appropriate
> option when building in the event they're using a Unix compiler that
> doesn't support it by default.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Makefile                            |  2 +-
>  contrib/buildsystems/CMakeLists.txt |  2 +-
>  git-compat-util.h                   | 13 +++++++++++++
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 12be39ac49..3d0ce6ddf6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1215,7 +1215,7 @@ ARFLAGS = rcs
>  PTHREAD_CFLAGS =
>
>  # For the 'sparse' target
> -SPARSE_FLAGS ?=
> +SPARSE_FLAGS ?= -std=gnu99
>  SP_EXTRA_FLAGS = -Wno-universal-initializer
>
>  # For informing GIT-BUILD-OPTIONS of the SANITIZE=leak target
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fd1399c440..07b6c5494b 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -208,7 +208,7 @@ endif()
>  if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
> -	add_compile_options(/MP)
> +	add_compile_options(/MP /std:c11)
>  endif()
>
>  #default behaviour
> diff --git a/git-compat-util.h b/git-compat-util.h
> index d70ce14286..ffe70b570f 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -1,6 +1,19 @@
>  #ifndef GIT_COMPAT_UTIL_H
>  #define GIT_COMPAT_UTIL_H
>
> +#if __STDC_VERSION__ - 0 < 199901L
> +/*
> + * Git is in a testing period for mandatory C99 support in the compiler.  If
> + * your compiler is reasonably recent, you can try to enable C99 support (or,
> + * for MSVC, C11 support).  If you encounter a problem and can't enable C99
> + * support with your compiler (such as with "-std=gnu99") and don't have access
> + * to one with this support, such as GCC or Clang, you can remove this #if
> + * directive, but please report the details of your system to
> + * git@vger.kernel.org.
> + */
> +#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
> +#endif
> +
>  #ifdef USE_MSVC_CRTDBG
>  /*
>   * For these to work they must appear very early in each
>

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

[Johannes Schindelin]

Hi brian,

On Sun, 14 Nov 2021, brian m. carlson wrote:

> The C99 standard was released in January 1999, now 22 years ago.  It
> provides a variety of useful features, including variadic arguments for
> macros, declarations after statements, variable length arrays, and a
> wide variety of other useful features, many of which we already use.
>
> We'd like to take advantage of these features, but we want to be
> cautious.  As far as we know, all major compilers now support C99 or a
> later C standard, such as C11 or C17.  POSIX has required C99 support as
> a requirement for the 2001 revision, so we can safely assume any POSIX
> system which we are interested in supporting has C99.
>
> 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.
>
> Let's add a test balloon to git-compat-util.h to see if anyone is using
> an older compiler.  We'll add a comment telling people how to enable
> this functionality on GCC and Clang, even though modern versions of both
> will automatically do the right thing, and ask people still experiencing
> a problem to report that to us on the list.
>
> Note that C89 compilers don't provide the __STDC_VERSION__ macro, so we
> use a well-known hack of using "- 0".  On compilers with this macro, it
> doesn't change the value, and on C89 compilers, the macro will be
> replaced with nothing, and our value will be 0.
>
> Sparse is also updated with a reference to the gnu99 standard, without
> which it defaults to C89.
>
> Update the cmake configuration to require C11 for MSVC.  We do this
> because this will make MSVC to use C11, since it does not explicitly
> support C99.  We do this with a compiler options because setting the
> C_STANDARD option does not work in our CI on MSVC and at the moment, we
> don't want to require C11 for Unix compilers.

I am all in favor of this patch!

Thank you,
Dscho

>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  Makefile                            |  4 ++--
>  contrib/buildsystems/CMakeLists.txt |  3 +--
>  git-compat-util.h                   | 12 ++++++++++++
>  3 files changed, 15 insertions(+), 4 deletions(-)
>
> 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
>  LDFLAGS =
>  CC_LD_DYNPATH = -Wl,-rpath,
>  BASIC_CFLAGS = -I.
> @@ -1215,7 +1215,7 @@ ARFLAGS = rcs
>  PTHREAD_CFLAGS =
>
>  # For the 'sparse' target
> -SPARSE_FLAGS ?=
> +SPARSE_FLAGS ?= -std=gnu99
>  SP_EXTRA_FLAGS = -Wno-universal-initializer
>
>  # For informing GIT-BUILD-OPTIONS of the SANITIZE=leak target
> diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
> index fd1399c440..91e8525fa9 100644
> --- a/contrib/buildsystems/CMakeLists.txt
> +++ b/contrib/buildsystems/CMakeLists.txt
> @@ -208,7 +208,7 @@ endif()
>  if(CMAKE_C_COMPILER_ID STREQUAL "MSVC")
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR})
>  	set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR})
> -	add_compile_options(/MP)
> +	add_compile_options(/MP /std:c11)
>  endif()
>
>  #default behaviour
> @@ -600,7 +600,6 @@ endif()
>  list(REMOVE_DUPLICATES excluded_progs)
>  list(REMOVE_DUPLICATES PROGRAMS_BUILT)
>
> -
>  foreach(p ${excluded_progs})
>  	list(APPEND EXCLUSION_PROGS --exclude-program ${p} )
>  endforeach()
> diff --git a/git-compat-util.h b/git-compat-util.h
> index d70ce14286..6d995bdc0f 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -1,6 +1,18 @@
>  #ifndef GIT_COMPAT_UTIL_H
>  #define GIT_COMPAT_UTIL_H
>
> +#if __STDC_VERSION__ - 0 < 199901L
> +/*
> + * Git is in a testing period for mandatory C99 support in the compiler.  If
> + * your compiler is reasonably recent, you can try to enable C99 support (or,
> + * for MSVC, C11 support).  If you encounter a problem and can't enable C99
> + * support with your compiler and don't have access to one with this support,
> + * such as GCC or Clang, you can remove this #if directive, but please report
> + * the details of your system to git@vger.kernel.org.
> + */
> +#error "Required C99 support is in a test phase.  Please see git-compat-util.h for more details."
> +#endif
> +
>  #ifdef USE_MSVC_CRTDBG
>  /*
>   * For these to work they must appear very early in each
>

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

[Junio C Hamano]

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.

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

[Johannes Schindelin]

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.

Ciao,
Dscho

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

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 22 2021, Johannes Schindelin wrote:

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

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

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

Thanks.

I took a look at #1082, which reverted this one (and nothing else)
to see a test succeds, and then re-reverted it to see it fail (but
apparently only on FreeBSD/Cirrus).  

I had an impression that it was claimed that without this, the other
weatherbaloon for "for (type var=..." would not fly in some of the
jobs we have at CI?

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

[Carlo Arenas]

On Mon, Nov 22, 2021 at 9:27 AM Junio C Hamano <gitster@pobox.com> wrote:

> I had an impression that it was claimed that without this, the other
> weatherbaloon for "for (type var=..." would not fly in some of the
> jobs we have at CI?

It wouldn't if we have a CI job that tests with gcc < 5 but the last
version of that job died with travis-ci.org

Carlo

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

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> On Mon, Nov 22, 2021 at 9:27 AM Junio C Hamano <gitster@pobox.com> wrote:
>
>> I had an impression that it was claimed that without this, the other
>> weatherbaloon for "for (type var=..." would not fly in some of the
>> jobs we have at CI?
>
> It wouldn't if we have a CI job that tests with gcc < 5 but the last
> version of that job died with travis-ci.org

I was wondering how Dscho's test was not failing, and that is an
easy answer to that question ;-)

If we wanted to resurrect that CI job, we can always add it in the
CI definition anyway, so I am OK with that, too.  

Thanks.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Carlo Arenas <carenas@gmail.com> writes:
>
>> On Mon, Nov 22, 2021 at 9:27 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>>> I had an impression that it was claimed that without this, the other
>>> weatherbaloon for "for (type var=..." would not fly in some of the
>>> jobs we have at CI?
>>
>> It wouldn't if we have a CI job that tests with gcc < 5 but the last
>> version of that job died with travis-ci.org
>
> I was wondering how Dscho's test was not failing, and that is an
> easy answer to that question ;-)
>
> If we wanted to resurrect that CI job, we can always add it in the
> CI definition anyway, so I am OK with that, too.  

But if we were to do so, perhaps we'd want something like what I
gave at https://lore.kernel.org/git/xmqqy25lwa86.fsf@gitster.g/ in
its place to avoid confusing people.

Let me update that topic before dropping the one under discussion.