Skip to content

make: Clean up WERROR handling #9357

Merged
cladmi merged 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/WERROR
Jun 26, 2018
Merged

make: Clean up WERROR handling #9357
cladmi merged 1 commit intoRIOT-OS:masterfrom
jnohlgard:pr/WERROR

Conversation

@jnohlgard
Copy link
Copy Markdown
Member

Contribution description

Clean up of the WERROR handling in Makefile.include to let the WERROR variable only control the actual -Werror compiler flag. All other warning related flags are set regardless of WERROR=1 or not.

Issues/PRs references

@jnohlgard jnohlgard added Area: build system Area: Build system Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jun 14, 2018
@jnohlgard jnohlgard added this to the Release 2018.07 milestone Jun 14, 2018
@jnohlgard jnohlgard requested a review from cladmi June 14, 2018 21:50
Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I globally agree, now that -Wextra can be set by default without warnings to always enable it.

As the same time for cleaning, I think -pedantic-errors can be removed as explained inline.

Makefile.include Outdated
CFLAGS += -Wpedantic -pedantic-errors
CFLAGS += -Wpedantic
ifeq ($(WERROR),1)
CFLAGS += -pedantic-errors
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From some testing, -pedantic-errors looks redundant with -Werror -Wpedantic and so could be removed.
I get the same behavior with and without -pedantic-errors for the following:

Compiling unittests for samr21-xpro with

make -C tests/unittests/ tests-spiffs BOARD=samr21-xpro WPEDANTIC=1 WERROR=0

and with WERROR=1.

While having the following diff to have a warning:

diff --git a/pkg/spiffs/Makefile.spiffs b/pkg/spiffs/Makefile.spiffs
index 1cb6ab8..bd7256c 100644
--- a/pkg/spiffs/Makefile.spiffs
+++ b/pkg/spiffs/Makefile.spiffs
@@ -1,6 +1,6 @@
 MODULE := spiffs
 
 # Disable 'ISO C99 doesn’t support unnamed structs/unions [-Werror=pedantic]'
-CFLAGS += -Wno-pedantic
+#CFLAGS += -Wno-pedantic
 
 include $(RIOTBASE)/Makefile.base

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleted this block

Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, I tested the four possibilities of WERROR and WPEDANTIC by playing with spiffs pedantic configuration.

Please squash.

@jnohlgard
Copy link
Copy Markdown
Member Author

squashed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants