[dpdk-dev,v3] mk: stop on warning only in developer build

Message ID 1456928543-23329-1-git-send-email-thomas.monjalon@6wind.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Thomas Monjalon March 2, 2016, 2:22 p.m. UTC
  From: Panu Matilainen <pmatilai@redhat.com>

Add RTE_DEVEL_BUILD make-variable which can be used to do things
differently when doing development vs building a release,
autodetected from source root .git presence and overridable via
commandline. It is used it to enable -Werror compiler flag and may
be extended to other checks.

Failing build on warnings is a useful developer tool but its bad
for release tarballs which can and do get built with newer
compilers than what was used/available during development. Compilers
routinely add new warnings so code which built silently with cc X
might no longer do so with X+1. This doesn't make the existing code
any more buggier and failing the build in this case does not help
to improve the quality of an already released version either.

This change the default flags which can be tuned with EXTRA_CFLAGS.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 doc/build-sdk-quick.txt                        | 1 +
 doc/guides/prog_guide/dev_kit_build_system.rst | 2 ++
 mk/rte.vars.mk                                 | 5 +++++
 mk/toolchain/clang/rte.vars.mk                 | 6 +++++-
 mk/toolchain/gcc/rte.vars.mk                   | 6 +++++-
 mk/toolchain/icc/rte.vars.mk                   | 6 +++++-
 6 files changed, 23 insertions(+), 3 deletions(-)

v3:
- not only for SDK build (-Werror for examples, apps)
- update doc
  

Comments

Bruce Richardson March 2, 2016, 10:04 p.m. UTC | #1
On Wed, Mar 02, 2016 at 03:22:23PM +0100, Thomas Monjalon wrote:
> From: Panu Matilainen <pmatilai@redhat.com>
> 
> Add RTE_DEVEL_BUILD make-variable which can be used to do things
> differently when doing development vs building a release,
> autodetected from source root .git presence and overridable via
> commandline. It is used it to enable -Werror compiler flag and may
> be extended to other checks.
> 
> Failing build on warnings is a useful developer tool but its bad
> for release tarballs which can and do get built with newer
> compilers than what was used/available during development. Compilers
> routinely add new warnings so code which built silently with cc X
> might no longer do so with X+1. This doesn't make the existing code
> any more buggier and failing the build in this case does not help
> to improve the quality of an already released version either.
> 
> This change the default flags which can be tuned with EXTRA_CFLAGS.
> 
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  
Thomas Monjalon March 3, 2016, 10:36 a.m. UTC | #2
2016-03-02 22:04, Bruce Richardson:
> On Wed, Mar 02, 2016 at 03:22:23PM +0100, Thomas Monjalon wrote:
> > From: Panu Matilainen <pmatilai@redhat.com>
> > 
> > Add RTE_DEVEL_BUILD make-variable which can be used to do things
> > differently when doing development vs building a release,
> > autodetected from source root .git presence and overridable via
> > commandline. It is used it to enable -Werror compiler flag and may
> > be extended to other checks.
> > 
> > Failing build on warnings is a useful developer tool but its bad
> > for release tarballs which can and do get built with newer
> > compilers than what was used/available during development. Compilers
> > routinely add new warnings so code which built silently with cc X
> > might no longer do so with X+1. This doesn't make the existing code
> > any more buggier and failing the build in this case does not help
> > to improve the quality of an already released version either.
> > 
> > This change the default flags which can be tuned with EXTRA_CFLAGS.
> > 
> > Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks

If people have problems compiling previous releases,
please remind to use EXTRA_CFLAGS=-Wno-error
  
Panu Matilainen March 3, 2016, 10:53 a.m. UTC | #3
On 03/03/2016 12:36 PM, Thomas Monjalon wrote:
> 2016-03-02 22:04, Bruce Richardson:
>> On Wed, Mar 02, 2016 at 03:22:23PM +0100, Thomas Monjalon wrote:
>>> From: Panu Matilainen <pmatilai@redhat.com>
>>>
>>> Add RTE_DEVEL_BUILD make-variable which can be used to do things
>>> differently when doing development vs building a release,
>>> autodetected from source root .git presence and overridable via
>>> commandline. It is used it to enable -Werror compiler flag and may
>>> be extended to other checks.
>>>
>>> Failing build on warnings is a useful developer tool but its bad
>>> for release tarballs which can and do get built with newer
>>> compilers than what was used/available during development. Compilers
>>> routinely add new warnings so code which built silently with cc X
>>> might no longer do so with X+1. This doesn't make the existing code
>>> any more buggier and failing the build in this case does not help
>>> to improve the quality of an already released version either.
>>>
>>> This change the default flags which can be tuned with EXTRA_CFLAGS.
>>>
>>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>>> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
>>
>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
> Applied, thanks

Thanks for dusting this up, I'd pretty much forgotten the whole thing.

	- Panu -
  

Patch

diff --git a/doc/build-sdk-quick.txt b/doc/build-sdk-quick.txt
index acd1bfe..967ff09 100644
--- a/doc/build-sdk-quick.txt
+++ b/doc/build-sdk-quick.txt
@@ -15,6 +15,7 @@  Build variables
 	EXTRA_LDFLAGS    linker options
 	EXTRA_LDLIBS     linker library options
 	RTE_KERNELDIR    linux headers path
+	RTE_DEVEL_BUILD  stricter options (default: y in git tree)
 	CROSS     toolchain prefix
 	V         verbose
 	D         debug dependencies
diff --git a/doc/guides/prog_guide/dev_kit_build_system.rst b/doc/guides/prog_guide/dev_kit_build_system.rst
index dd3e3d0..3e89eae 100644
--- a/doc/guides/prog_guide/dev_kit_build_system.rst
+++ b/doc/guides/prog_guide/dev_kit_build_system.rst
@@ -343,6 +343,8 @@  Useful Variables Provided by the Build System
     By default, the variable is set to /lib/modules/$(shell uname -r)/build,
     which is correct when the target machine is also the build machine.
 
+*   RTE_DEVEL_BUILD: Stricter options (stop on warning). It defaults to y in a git tree.
+
 Variables that Can be Set/Overridden in a Makefile Only
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
index 2d734bd..28982a5 100644
--- a/mk/rte.vars.mk
+++ b/mk/rte.vars.mk
@@ -102,6 +102,11 @@  export RTE_MACHINE
 export RTE_EXEC_ENV
 export RTE_TOOLCHAIN
 
+# developer build automatically enabled in a git tree
+ifneq ($(wildcard $(RTE_SDK)/.git),)
+RTE_DEVEL_BUILD := y
+endif
+
 # SRCDIR is the current source directory
 ifdef S
 SRCDIR := $(abspath $(RTE_SRCDIR)/$(S))
diff --git a/mk/toolchain/clang/rte.vars.mk b/mk/toolchain/clang/rte.vars.mk
index 245ea7e..7749b99 100644
--- a/mk/toolchain/clang/rte.vars.mk
+++ b/mk/toolchain/clang/rte.vars.mk
@@ -63,12 +63,16 @@  TOOLCHAIN_ASFLAGS =
 TOOLCHAIN_CFLAGS =
 TOOLCHAIN_LDFLAGS =
 
-WERROR_FLAGS := -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
+WERROR_FLAGS := -W -Wall -Wstrict-prototypes -Wmissing-prototypes
 WERROR_FLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
 WERROR_FLAGS += -Wnested-externs -Wcast-qual
 WERROR_FLAGS += -Wformat-nonliteral -Wformat-security
 WERROR_FLAGS += -Wundef -Wwrite-strings
 
+ifeq ($(RTE_DEVEL_BUILD),y)
+WERROR_FLAGS += -Werror
+endif
+
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 
diff --git a/mk/toolchain/gcc/rte.vars.mk b/mk/toolchain/gcc/rte.vars.mk
index c2c5255..ff70f3d 100644
--- a/mk/toolchain/gcc/rte.vars.mk
+++ b/mk/toolchain/gcc/rte.vars.mk
@@ -71,12 +71,16 @@  ifeq (,$(findstring -O0,$(EXTRA_CFLAGS)))
 endif
 endif
 
-WERROR_FLAGS := -W -Wall -Werror -Wstrict-prototypes -Wmissing-prototypes
+WERROR_FLAGS := -W -Wall -Wstrict-prototypes -Wmissing-prototypes
 WERROR_FLAGS += -Wmissing-declarations -Wold-style-definition -Wpointer-arith
 WERROR_FLAGS += -Wcast-align -Wnested-externs -Wcast-qual
 WERROR_FLAGS += -Wformat-nonliteral -Wformat-security
 WERROR_FLAGS += -Wundef -Wwrite-strings
 
+ifeq ($(RTE_DEVEL_BUILD),y)
+WERROR_FLAGS += -Werror
+endif
+
 # There are many issues reported for ARMv7 architecture
 # which are not necessarily fatal. Report as warnings.
 ifeq ($(CONFIG_RTE_ARCH_ARMv7),y)
diff --git a/mk/toolchain/icc/rte.vars.mk b/mk/toolchain/icc/rte.vars.mk
index 9b6b34b..ba69f1f 100644
--- a/mk/toolchain/icc/rte.vars.mk
+++ b/mk/toolchain/icc/rte.vars.mk
@@ -69,9 +69,13 @@  TOOLCHAIN_ASFLAGS =
 #   error #13368: loop was not vectorized with "vector always assert"
 #   error #15527: loop was not vectorized: function call to fprintf cannot be vectorize
 #                   was declared "deprecated"
-WERROR_FLAGS := -Wall -Werror-all -w2 -diag-disable 271 -diag-warning 1478
+WERROR_FLAGS := -Wall -w2 -diag-disable 271 -diag-warning 1478
 WERROR_FLAGS += -diag-disable 13368 -diag-disable 15527
 
+ifeq ($(RTE_DEVEL_BUILD),y)
+WERROR_FLAGS += -Werror-all
+endif
+
 # process cpu flags
 include $(RTE_SDK)/mk/toolchain/$(RTE_TOOLCHAIN)/rte.toolchain-compat.mk
 # disable max-inline params boundaries for ICC compiler for version 15 and greater