2012-05-04 07:41:42

by Artem Bityutskiy

[permalink] [raw]
Subject: [PATCH 1/2] Makefile: make sure KCFLAGS options are at the end

From: Artem Bityutskiy <[email protected]>

When I build the kernel like this:

$ make W=1 KCFLAGS="-Wno-sign-compare"

the "-Wno-sign-compare" option is ignore. The reason is that we append gcc
options associated with W=1 _after_ KCFLAGS, so W=1 overrides my KCFLAGS
options. This patch tries to fix the issue by moving the "W=[123]" stiff
from "scripts/Makefile.build" to the main Maikefile. This fixes the issue
as well as this should be a small improvent because "scripts/Makefile.build"
is parsed by make many times (I think for each file), bue we do not need
to re-assign all the warnings stuff so many times - it is enough to do it
only once.

Signed-off-by: Artem Bityutskiy <[email protected]>
---
Makefile | 51 ++++++++++++++++++++++++++++++++++++++++++++++++
scripts/Makefile.build | 51 ------------------------------------------------
2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/Makefile b/Makefile
index afc868e..2cba5db 100644
--- a/Makefile
+++ b/Makefile
@@ -640,6 +640,57 @@ ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC)), y)
KBUILD_CFLAGS += -DCC_HAVE_ASM_GOTO
endif

+#
+# make W=... settings
+#
+# W=1 - warnings that may be relevant and does not occur too often
+# W=2 - warnings that occur quite often but may still be relevant
+# W=3 - the more obscure warnings, can most likely be ignored
+#
+# $(call cc-option, -W...) handles gcc -W.. options which
+# are not supported by all versions of the compiler
+ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
+warning- := $(empty)
+
+warning-1 := -Wextra -Wunused -Wno-unused-parameter
+warning-1 += -Wmissing-declarations
+warning-1 += -Wmissing-format-attribute
+warning-1 += -Wmissing-prototypes
+warning-1 += -Wold-style-definition
+warning-1 += $(call cc-option, -Wmissing-include-dirs)
+warning-1 += $(call cc-option, -Wunused-but-set-variable)
+warning-1 += $(call cc-disable-warning, missing-field-initializers)
+
+warning-2 := -Waggregate-return
+warning-2 += -Wcast-align
+warning-2 += -Wdisabled-optimization
+warning-2 += -Wnested-externs
+warning-2 += -Wshadow
+warning-2 += $(call cc-option, -Wlogical-op)
+warning-2 += $(call cc-option, -Wmissing-field-initializers)
+
+warning-3 := -Wbad-function-cast
+warning-3 += -Wcast-qual
+warning-3 += -Wconversion
+warning-3 += -Wpacked
+warning-3 += -Wpadded
+warning-3 += -Wpointer-arith
+warning-3 += -Wredundant-decls
+warning-3 += -Wswitch-default
+warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
+warning-3 += $(call cc-option, -Wvla)
+
+warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
+warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
+warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
+
+ifeq ("$(strip $(warning))","")
+ $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
+endif
+
+KBUILD_CFLAGS += $(warning)
+endif
+
# Add user supplied CPPFLAGS, AFLAGS and CFLAGS as the last assignments
# But warn user when we do so
warn-assign = \
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ff1720d..f8681cc 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -50,57 +50,6 @@ ifeq ($(KBUILD_NOPEDANTIC),)
endif
endif

-#
-# make W=... settings
-#
-# W=1 - warnings that may be relevant and does not occur too often
-# W=2 - warnings that occur quite often but may still be relevant
-# W=3 - the more obscure warnings, can most likely be ignored
-#
-# $(call cc-option, -W...) handles gcc -W.. options which
-# are not supported by all versions of the compiler
-ifdef KBUILD_ENABLE_EXTRA_GCC_CHECKS
-warning- := $(empty)
-
-warning-1 := -Wextra -Wunused -Wno-unused-parameter
-warning-1 += -Wmissing-declarations
-warning-1 += -Wmissing-format-attribute
-warning-1 += -Wmissing-prototypes
-warning-1 += -Wold-style-definition
-warning-1 += $(call cc-option, -Wmissing-include-dirs)
-warning-1 += $(call cc-option, -Wunused-but-set-variable)
-warning-1 += $(call cc-disable-warning, missing-field-initializers)
-
-warning-2 := -Waggregate-return
-warning-2 += -Wcast-align
-warning-2 += -Wdisabled-optimization
-warning-2 += -Wnested-externs
-warning-2 += -Wshadow
-warning-2 += $(call cc-option, -Wlogical-op)
-warning-2 += $(call cc-option, -Wmissing-field-initializers)
-
-warning-3 := -Wbad-function-cast
-warning-3 += -Wcast-qual
-warning-3 += -Wconversion
-warning-3 += -Wpacked
-warning-3 += -Wpadded
-warning-3 += -Wpointer-arith
-warning-3 += -Wredundant-decls
-warning-3 += -Wswitch-default
-warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
-warning-3 += $(call cc-option, -Wvla)
-
-warning := $(warning-$(findstring 1, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-warning += $(warning-$(findstring 2, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-warning += $(warning-$(findstring 3, $(KBUILD_ENABLE_EXTRA_GCC_CHECKS)))
-
-ifeq ("$(strip $(warning))","")
- $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
-endif
-
-KBUILD_CFLAGS += $(warning)
-endif
-
include scripts/Makefile.lib

ifdef host-progs
--
1.7.9.1


2012-05-04 07:41:44

by Artem Bityutskiy

[permalink] [raw]
Subject: [PATCH 2/2] Makefile: remove useless warning

From: Artem Bityutskiy <[email protected]>

This patch removes annoying warning:

Makefile:708: "WARNING: Appending $KCFLAGS (-Wno-sign-compare) from command line to kernel $CFLAGS"

which is printed every time I use KCFLAFS. The commit which introduced the
warning:

69ee0b3 kbuild: do not pick up CFLAGS from the environment

tells about the problems when people have CFLAGS in their environment,
then switches to KCFLAFS which should be enough to solve the issue, but
it anyway introduces a warning. I think it is too much and I find this
warning very annoying - why should we warn users when they use the
build system properly?

Signed-off-by: Artem Bityutskiy <[email protected]>
---
Makefile | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 2cba5db..53e6b7d 100644
--- a/Makefile
+++ b/Makefile
@@ -692,20 +692,14 @@ KBUILD_CFLAGS += $(warning)
endif

# Add user supplied CPPFLAGS, AFLAGS and CFLAGS as the last assignments
-# But warn user when we do so
-warn-assign = \
-$(warning "WARNING: Appending $$K$(1) ($(K$(1))) from $(origin K$(1)) to kernel $$$(1)")

ifneq ($(KCPPFLAGS),)
- $(call warn-assign,CPPFLAGS)
KBUILD_CPPFLAGS += $(KCPPFLAGS)
endif
ifneq ($(KAFLAGS),)
- $(call warn-assign,AFLAGS)
KBUILD_AFLAGS += $(KAFLAGS)
endif
ifneq ($(KCFLAGS),)
- $(call warn-assign,CFLAGS)
KBUILD_CFLAGS += $(KCFLAGS)
endif

--
1.7.9.1

2012-05-04 08:56:39

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 1/2] Makefile: make sure KCFLAGS options are at the end

On Fri, May 04, 2012 at 10:42:34AM +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <[email protected]>
>
> When I build the kernel like this:
>
> $ make W=1 KCFLAGS="-Wno-sign-compare"
>
> the "-Wno-sign-compare" option is ignore. The reason is that we append gcc
> options associated with W=1 _after_ KCFLAGS, so W=1 overrides my KCFLAGS
> options. This patch tries to fix the issue by moving the "W=[123]" stiff
> from "scripts/Makefile.build" to the main Maikefile. This fixes the issue
> as well as this should be a small improvent because "scripts/Makefile.build"
> is parsed by make many times (I think for each file), bue we do not need
> to re-assign all the warnings stuff so many times - it is enough to do it
> only once.

Moving all the W=... support out of Makefile.build is a good thing,
as we really should do the evaluation only once.

But I detest that this is added to the top-level Makefile.
The top-level Makefile is full of all sort of strange stuff
already and very hard to navigate / update for this reason.

How about introducing a new file : scripts/Kbuild.config
This file should be included only _once_ from the top-level Makefile.
And it should contains all the trivial assignments to
CC, CFLAGS, etc...

Including exporting of variables so they a visible in sub-makes.

First step could be to move the W=... support, and then it could be
gradually extended.

Michal - any comments on this?

Sam