diff mbox series

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

Message ID 20211116021241.1565740-2-sandals@crustytoothpaste.net (mailing list archive)
State Superseded
Headers show
Series Add a test balloon for C99 support | expand

Commit Message

brian m. carlson Nov. 16, 2021, 2:12 a.m. UTC
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(-)

Comments

Jeff King Nov. 16, 2021, 12:19 p.m. UTC | #1
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
Ævar Arnfjörð Bjarmason Nov. 16, 2021, 12:54 p.m. UTC | #2
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/
Jeff King Nov. 16, 2021, 2:54 p.m. UTC | #3
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
Phillip Wood Nov. 16, 2021, 7:44 p.m. UTC | #4
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
>
brian m. carlson Nov. 17, 2021, 1:44 a.m. UTC | #5
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 Nov. 17, 2021, 2:53 a.m. UTC | #6
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.
Jeff King Nov. 17, 2021, 2:58 a.m. UTC | #7
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
Jeff King Nov. 17, 2021, 3:01 a.m. UTC | #8
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
Junio C Hamano Nov. 17, 2021, 8:49 a.m. UTC | #9
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.
brian m. carlson Nov. 17, 2021, 11:18 p.m. UTC | #10
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).
Carlo Marcelo Arenas Belón Nov. 17, 2021, 11:45 p.m. UTC | #11
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/
Ævar Arnfjörð Bjarmason Nov. 18, 2021, 2:26 a.m. UTC | #12
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.
Junio C Hamano Nov. 18, 2021, 7:10 p.m. UTC | #13
"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);
diff mbox series

Patch

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