Re: [PATCH 2/3] Makefile: avoid multiple -Wall in CFLAGS

From: Carlo Arenas <carenas@gmail.com>
Date: Sep 28, 2021, 23:22 UTC
Message-ID: <CAPUEspjvZHEb0X71tFRuFJoxK9eChajfC40OVw7iaup1R+1jZg@mail.gmail.com>
In-Reply-To: <xmqqbl4cjt3e.fsf@gitster.g>

On Tue, Sep 28, 2021 at 2:19 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 17 quoted lines

>
> Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:
>
> > 6163f3f1a4 (config.mak.dev: add -Wall, primarily for -Wformat, to help
> > autoconf users, 2018-10-12) adds a second -Wall in config.mak.dev to
> > workaround the lack of one from config.mak.autogen.
> >
> > Since 6d5d4b4e93 (Makefile: allow for combining DEVELOPER=1 and
> > CFLAGS="...", 2019-02-22), that variable is set instead as part of
> > DEVELOPER_FLAGS which won't be overriden by config.mak.autogen, so
> > it can be safely removed from config.mak.dev if set instead in the
> > Makefile.
>
> Hmph, don't this break non-developers, though?
>
> They now do not get -Wall that they used to?  Or am I reading the
> patch incorrectly?

It is subtle and apparently problematic as some users[1] rely on removing -Wall to support non GNU compilers, but my change kept the -Wall by default by making sure it got set in DEVELOPER_FLAGS instead (which as Ævar points out might be a layering violation and hence might need more planning).

Either way I'll be dropping this series for now, to make sure that the minimal change is put forward as part of js/win-lazyload-buildfix instead.

Carlo

[1] https://lore.kernel.org/git/CAPUEspj_hOXRc2d+c+DTvShgWX8NH+fXKD4Pk+_G=nj9Z97VnQ@mail.gmail.com/