makefiles/cflags.inc.mk: handle optional cflags

[cladmi]

Contribution description

Handle declaring OPTIONAL_CFLAGS and blacklisting them with
OPTIONAL_CFLAGS_BLACKLIST.

This should replace checking everytime if options are supported.

The idea, is that these optional options are toolchain specific. So can be saved for the reference platform and allow users to still define other ones.

The BLACKLIST was defined in the docker image to match the previous in CFLAGS.

I did not add them to docker exported variables are they are supposed to be qualified in the docker image.

Testing procedure

All examples must correctly compile in CI as it is the reference toolchain.

I compared the value of CFLAGS and got the same value.

I did not re-do the handling for CXXUWFLAGS += -fno-delete-null-pointer-checks but if everything compiles correctly, in docker it should be ok.

For testing I used this diff to compare the sorted value of CFLAGS_WITH_MACROS as it is the full value.

buildtest print CFLAGS

diff --git a/makefiles/buildtests.inc.mk b/makefiles/buildtests.inc.mk
index 734176baa..cdee194e3 100644
--- a/makefiles/buildtests.inc.mk
+++ b/makefiles/buildtests.inc.mk
@@ -1,23 +1,14 @@
 .PHONY: buildtest buildtest-indocker

-BUILDTEST_MAKE_REDIRECT ?= >/dev/null 2>&1
+BUILDTEST_MAKE_REDIRECT ?=

 buildtest:
        @ \
        RESULT=true ; \
        for board in $(BOARDS); do \
                if BOARD=$${board} $(MAKE) check-toolchain-supported > /dev/null 2>&1; then \
-                       $(COLOR_ECHO) -n "Building for $$board ... " ; \
                        BOARD=$${board} RIOT_CI_BUILD=1 \
-                               $(MAKE) clean all -j $(NPROC) $(BUILDTEST_MAKE_REDIRECT); \
-                       RES=$$? ; \
-                       if [ $$RES -eq 0 ]; then \
-                               $(COLOR_ECHO) "$(COLOR_GREEN)success.$(COLOR_RESET)" ; \
-                       else \
-                               $(COLOR_ECHO) "$(COLOR_RED)failed!$(COLOR_RESET)" ; \
-                               RESULT=false ; \
-                       fi ; \
-                       BOARD=$${board} $(MAKE) clean-intermediates >/dev/null 2>&1 || true; \
+                               $(MAKE) --no-print-directory info-debug-variable-CFLAGS_WITH_MACROS | xargs -n1 | sort -u | xargs; \
                fi; \
        done ; \
        $${RESULT}

And compared the value in examples/hello-world with both TOOLCHAIN=gnu and TOOLCHAIN=llvm.
The output was the same with this PR and in master.

time BUILD_IN_DOCKER=1 DOCKER="sudo docker" make --no-print-directory -C examples/hello-world/ buildtest-indocker

Timing impact

The execution time was of course lower without re-checking the toolchain support, and even more with llvm.

toolchain	master	pull request
gnu	1m4	50s
llvm	2m12	1m6

Issues/PRs references

This work is done in the context of removing immediate variable evaluations and exports
#10850

[jcarrano]

This is good! Maybe in the future we can add a static check to enforce that optional flags only contain "-W*", "-f*" and "-g*"

[smlng]

this broke support for GCC 5.4 which doesn't know about -Wformat-overflow and -Wformat-truncation. We discovered that bc part of our HIL setup was running Raspbian stretch (Debian 9) where the arm-none-eabi-gcc is still version 5.x.

We currently update to Raspbian Buster which has GCC 7 (same version as riotdocker image) that should fix the issue.

[jcarrano]

The riot docker image is considered the "minimum", that is, any system with older packages is not supported.

If you encounter this issue again you could add some site-specific blacklisting via RIOT_MAKEFILES_GLOBAL_PRE.

[cladmi]

You can either export OPTIONAL_CFLAGS_BLACKLIST="-Wformat-overflow -Wformat-truncation" in your environment to globally disable checking for this warning or, as @jcarrano said define it per board in a RIOT_MAKEFILES_GLOBAL_PRE.

Also, note, only the arm-none-eabi-gcc downloaded from arm is supported
https://github.com/RIOT-OS/riotdocker/blob/db38dabadc321e591fb7f2f77c7a2ddf072c9d8e/Dockerfile#L106-L107
(wich was previously https://launchpad.net/gcc-arm-embedded)

Even the one from ubuntu-bionic is blacklisted as does not work at all with RIOT. #10404

[cladmi]

Oh, and thank you for the review by the way :D