Use append attribution to CFLAGS in wcsmbs
Commit Message
CFLAGS for several files in wcsmbs/Makefile are set without the append
operator ("+="), thus ignoring other attributions. This patch changes
that so Makefiles in sysdeps can set extra compilation flags. This is
being done in preparation for future float128 patches.
Tested for powerpc64le and x86_64.
2017-03-07 Gabriel F. T. Gomes <gftg@linux.vnet.ibm.com>
* wcsmbs/Makefile (CFLAGS-wcstol.c, CFLAGS-wcstoul.c)
(CFLAGS-wcstoll.c, CFLAGS-wcstoull.c, CFLAGS-wcstod.c)
(CFLAGS-wcstold.c, CFLAGS-wcstof.c, CFLAGS-wcstol_l.c)
(CFLAGS-wcstoul_l.c, CFLAGS-wcstoll_l.c, CFLAGS-wcstoull_l.c)
(CFLAGS-wcstod_l.c, CFLAGS-wcstold_l.c, CFLAGS-wcstof_l.c)
(CPPFLAGS-tst-wchar-h.c): Use the append operator.
---
wcsmbs/Makefile | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
Comments
On 08 Mar 2017 13:47, Gabriel F. T. Gomes wrote:
> CFLAGS for several files in wcsmbs/Makefile are set without the append
> operator ("+="), thus ignoring other attributions. This patch changes
> that so Makefiles in sysdeps can set extra compilation flags. This is
> being done in preparation for future float128 patches.
i'm not against this, but it seems like we should be doing this
everywhere ? when i grep for CFLAGS-xxx.c, there's about 650
using = and 250 using +=.
even in this file, you've only changed one block ... there's
more entries in it still using =.
-mike
On Sat, 11 Mar 2017, Mike Frysinger wrote:
> On 08 Mar 2017 13:47, Gabriel F. T. Gomes wrote:
> > CFLAGS for several files in wcsmbs/Makefile are set without the append
> > operator ("+="), thus ignoring other attributions. This patch changes
> > that so Makefiles in sysdeps can set extra compilation flags. This is
> > being done in preparation for future float128 patches.
>
> i'm not against this, but it seems like we should be doing this
> everywhere ? when i grep for CFLAGS-xxx.c, there's about 650
> using = and 250 using +=.
See the discussion starting at
<https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html> (and
<https://sourceware.org/ml/libc-alpha/2013-01/msg00247.html>). Given that
CFLAGS are the right way of handling a particular file (rather than
diagnostic pragmas, #define, etc., directly in the source code), I think
appending makes sense unless there is a specific reason for not doing so
in a particular case (and working out if any existing cases are
deliberately not appending may be tricky).
On Mon, 13 Mar 2017 16:37:43 +0000
Joseph Myers <joseph@codesourcery.com> wrote:
> On Sat, 11 Mar 2017, Mike Frysinger wrote:
>
> > On 08 Mar 2017 13:47, Gabriel F. T. Gomes wrote:
> > > CFLAGS for several files in wcsmbs/Makefile are set without the append
> > > operator ("+="), thus ignoring other attributions. This patch changes
> > > that so Makefiles in sysdeps can set extra compilation flags. This is
> > > being done in preparation for future float128 patches.
> >
> > i'm not against this, but it seems like we should be doing this
> > everywhere ? when i grep for CFLAGS-xxx.c, there's about 650
> > using = and 250 using +=.
>
> See the discussion starting at
> <https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html> (and
> <https://sourceware.org/ml/libc-alpha/2013-01/msg00247.html>). Given that
> CFLAGS are the right way of handling a particular file (rather than
> diagnostic pragmas, #define, etc., directly in the source code), I think
> appending makes sense unless there is a specific reason for not doing so
> in a particular case (and working out if any existing cases are
> deliberately not appending may be tricky).
>
In the discussion, there seems to be some consensus as to using
sysdep-CFLAGS-<filename> (and sysdep-CFLAGS += $(sysdep-CFLAGS-$(<F))),
instead of CFLAGS (as mentioned in the third item of [1]). As well
as there seems to be consensus on using the append operator (as
mentioned in [2])
(Please, let me know if my perception of consensus is wrong here)
[1] https://sourceware.org/ml/libc-alpha/2013-01/msg00337.html
[2] https://sourceware.org/ml/libc-alpha/2013-01/msg00338.html
I wasn't aware of sysdep-CFLAGS when I sent this patch.
With the use of sysdep-CFLAGS, there is no need to change the =
operator to += in wcsmbs/Makefile.
So, may I drop this patch, or is it still useful?
On 13 Mar 2017 16:03, Gabriel F. T. Gomes wrote:
> On Mon, 13 Mar 2017 16:37:43 +0000 Joseph Myers wrote:
> > On Sat, 11 Mar 2017, Mike Frysinger wrote:
> > > On 08 Mar 2017 13:47, Gabriel F. T. Gomes wrote:
> > > > CFLAGS for several files in wcsmbs/Makefile are set without the append
> > > > operator ("+="), thus ignoring other attributions. This patch changes
> > > > that so Makefiles in sysdeps can set extra compilation flags. This is
> > > > being done in preparation for future float128 patches.
> > >
> > > i'm not against this, but it seems like we should be doing this
> > > everywhere ? when i grep for CFLAGS-xxx.c, there's about 650
> > > using = and 250 using +=.
> >
> > See the discussion starting at
> > <https://sourceware.org/ml/libc-alpha/2012-11/msg00798.html> (and
> > <https://sourceware.org/ml/libc-alpha/2013-01/msg00247.html>). Given that
> > CFLAGS are the right way of handling a particular file (rather than
> > diagnostic pragmas, #define, etc., directly in the source code), I think
> > appending makes sense unless there is a specific reason for not doing so
> > in a particular case (and working out if any existing cases are
> > deliberately not appending may be tricky).
> >
>
> In the discussion, there seems to be some consensus as to using
> sysdep-CFLAGS-<filename> (and sysdep-CFLAGS += $(sysdep-CFLAGS-$(<F))),
> instead of CFLAGS (as mentioned in the third item of [1]). As well
> as there seems to be consensus on using the append operator (as
> mentioned in [2])
>
> (Please, let me know if my perception of consensus is wrong here)
rereading the threads again, it looks like we're all in agreement.
let's land a commit to change all of the "=" to "+=", and let's add
support for $(sysdep-CFLAGS-$(<F)).
-mike
@@ -75,21 +75,21 @@ CFLAGS-wcwidth.c = -I../wctype
CFLAGS-wcswidth.c = -I../wctype
strtox-CFLAGS = -I../include
-CFLAGS-wcstol.c = $(strtox-CFLAGS)
-CFLAGS-wcstoul.c = $(strtox-CFLAGS)
-CFLAGS-wcstoll.c = $(strtox-CFLAGS)
-CFLAGS-wcstoull.c = $(strtox-CFLAGS)
-CFLAGS-wcstod.c = $(strtox-CFLAGS)
-CFLAGS-wcstold.c = $(strtox-CFLAGS)
-CFLAGS-wcstof.c = $(strtox-CFLAGS)
-CFLAGS-wcstol_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstoul_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstoll_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstoull_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstod_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstold_l.c = $(strtox-CFLAGS)
-CFLAGS-wcstof_l.c = $(strtox-CFLAGS)
-CPPFLAGS-tst-wchar-h.c = -D_FORTIFY_SOURCE=2
+CFLAGS-wcstol.c += $(strtox-CFLAGS)
+CFLAGS-wcstoul.c += $(strtox-CFLAGS)
+CFLAGS-wcstoll.c += $(strtox-CFLAGS)
+CFLAGS-wcstoull.c += $(strtox-CFLAGS)
+CFLAGS-wcstod.c += $(strtox-CFLAGS)
+CFLAGS-wcstold.c += $(strtox-CFLAGS)
+CFLAGS-wcstof.c += $(strtox-CFLAGS)
+CFLAGS-wcstol_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstoul_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstoll_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstoull_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstod_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstold_l.c += $(strtox-CFLAGS)
+CFLAGS-wcstof_l.c += $(strtox-CFLAGS)
+CPPFLAGS-tst-wchar-h.c += -D_FORTIFY_SOURCE=2
CFLAGS-isoc99_wscanf.c += -fexceptions
CFLAGS-isoc99_fwscanf.c += -fexceptions