Re: v2.35.0 DEVELOPER=1 regression

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 14, 2022, 23:56 UTC
Message-ID: <220115.86tue5rhlf.gmgdl@evledraar.gmail.com>
In-Reply-To: <xmqqzgny7xo7.fsf@gitster.g>

On Fri, Jan 14 2022, Junio C Hamano wrote:

Show 30 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> I think we had this discussion about FreeBSD before and that's why I
>>> specifically dropped that option from the main makefile.  We can either
>>> drop that patch, or we can set it to -std=gnu11 and tell folks setting
>>> DEVELOPER to use a system released in the last five years.  I think we
>>> can be a little stricter with what we require in the case of DEVELOPER
>>> than we might be otherwise.
>>
>> But that is not being stricter, but looser, no?  I thought that the
>> point of -std=gnu99 was to allow us to use C99 features while catching
>> use of language features newer than that, and use of -std=gnu11 will
>> defeat half the point, wouldn't it?
>
> If FreeBSD (or any other platform) cannot do "reject features beyond
> C99", I am perfectly OK to drop -std=gnu99 on such a place.
>
> DEVELOPER=YesPlease ought to be a collection of settings that helps
> the developers the most.  So on platforms that *can* do "reject
> features beyond C99", I prefer to have it enabled when
> DEVELOPER=YesPlease is given.
>
> It seems that -std=gnu99 is only added conditionally even in today's
> config.mak.dev, so it is fine if we dropped -std=gnu99 from there.
> Which means that developers on FreeBSD cannot participate in vetting
> use of features beyond C99, but there are developers on other
> platforms who will, so it's not too bad, I would say.
>
> As config.mak.dev is included after config.mak.uname, something like
> this may be sufficient, perhaps?

Wasn't the initial goal here to check whether we'd accidentally include
C99? Just checking on GCC/Clang somewhere seems sufficient enough,
I.e. something like:
    
    diff --git a/ci/lib.sh b/ci/lib.sh
    index 9d28ab50fb4..94d0d4127b9 100755
    --- a/ci/lib.sh
    +++ b/ci/lib.sh
    @@ -209,6 +209,9 @@ linux-leaks)
            export SANITIZE=leak
            export GIT_TEST_PASSING_SANITIZE_LEAK=true
            ;;
    +linux-gcc|linux-clang)
    +       MAKEFLAGS="$MAKEFLAGS CFLAGS=-std=gnu99"
    +       ;;
     esac
     
     MAKEFLAGS="$MAKEFLAGS CC=${CC:-cc}"
    diff --git a/config.mak.dev b/config.mak.dev
    index d4afac6b51f..216f92493fc 100644
    --- a/config.mak.dev
    +++ b/config.mak.dev
    @@ -20,10 +20,6 @@ endif
     endif
     endif
     
    -ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang7,$(COMPILER_FEATURES))),)
    -DEVELOPER_CFLAGS += -std=gnu99
    -endif
    -
     DEVELOPER_CFLAGS += -Wdeclaration-after-statement
     DEVELOPER_CFLAGS += -Wformat-security
     DEVELOPER_CFLAGS += -Wold-style-definition
    
We're just starting to get into the whack-a-mole of hardcoding OS's and
compiler versions, it's not all versions of FreeBSD, probably not just
that OS etc.

I've also wondered why we can't just ship our own super-light version of autoconf to run some real probes, and as a result eventually get rid of configure.ac, ./detect-compiler, the version hardcoding in config.mak.dev etc.

Just something as simple as extending this:
    
    diff --git a/Makefile b/Makefile
    index 5580859afdb..f197a07c100 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -2557,6 +2557,15 @@ else
     $(OBJECTS): $(LIB_H) $(GENERATED_H)
     endif
     
    +ifdef DEVELOPER
    +probe/std.mak:
    +       @mkdir -p $(@D)
    +       $(QUIET_GEN)if $(CC) -E -std=gnu99 - </dev/null >/dev/null; then \
    +               echo 'DEVELOPER_CFLAGS += -std=gnu99'; \
    +       fi >$@
    +include probe/std.mak
    +endif
    +
     ifeq ($(GENERATE_COMPILATION_DATABASE),yes)
     all:: compile_commands.json
     compile_commands.json: