From f166e7469665f3231dda4b31cf8ccb019c774890 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 20 Nov 2024 21:45:14 +0100 Subject: [PATCH 1/2] build system: error on conflicting features When the build system is aware of an invalid configuration - such as multiple C libs being used or a single piece of hardware being used in two conflicting roles - this now is treated as an error rather than a warning. --- Makefile.include | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Makefile.include b/Makefile.include index 9a5f8292d4..dcea932fa1 100644 --- a/Makefile.include +++ b/Makefile.include @@ -936,7 +936,6 @@ include $(RIOTMAKE)/usb-codes.inc.mk # Warn if the selected board and drivers don't provide all needed features: ifneq (, $(filter all flash, $(if $(MAKECMDGOALS), $(MAKECMDGOALS), all))) EXPECT_ERRORS := - EXPECT_CONFLICT := # Test if there where dependencies against a module in DISABLE_MODULE. ifneq (, $(filter $(DISABLE_MODULE), $(USEMODULE))) @@ -962,12 +961,12 @@ ifneq (, $(filter all flash, $(if $(MAKECMDGOALS), $(MAKECMDGOALS), all))) # Test if any used feature conflict with another one. ifneq (,$(FEATURES_CONFLICTING)) - $(shell $(COLOR_ECHO) "$(COLOR_YELLOW)The following features may conflict:$(COLOR_RESET)"\ + $(shell $(COLOR_ECHO) "$(COLOR_RED)The following features conflict:$(COLOR_RESET)"\ "$(FEATURES_CONFLICTING)" 1>&2) ifneq (, $(FEATURES_CONFLICT_MSG)) $(shell $(COLOR_ECHO) "$(COLOR_YELLOW)Rationale: $(COLOR_RESET)$(FEATURES_CONFLICT_MSG)" 1>&2) endif - EXPECT_CONFLICT := 1 + EXPECT_ERRORS := 1 endif # If there is a whitelist, then test if the board is whitelisted. @@ -1000,10 +999,6 @@ ifneq (, $(filter all flash, $(if $(MAKECMDGOALS), $(MAKECMDGOALS), all))) endif endif - ifneq (, $(EXPECT_CONFLICT)) - $(shell $(COLOR_ECHO) "\n$(COLOR_YELLOW)EXPECT undesired behaviour!$(COLOR_RESET)" 1>&2) - endif - # Fail by default when errors are expected CONTINUE_ON_EXPECTED_ERRORS ?= 0 ifneq (, $(EXPECT_ERRORS)) From d8304fa1965afed19177e761393598a532eecfc7 Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Wed, 20 Nov 2024 21:51:39 +0100 Subject: [PATCH 2/2] tests/build_system: drop warn_conflict app This is no longer a warning but an error. Also: This is already covered by the static tests via `dist/tools/feature_resolution/check.sh`, which uses a lot less CPU time in the CI. --- tests/build_system/warn_conflict/Makefile | 13 ----- tests/build_system/warn_conflict/README.md | 41 --------------- tests/build_system/warn_conflict/main.c | 38 -------------- .../warn_conflict/tests/01-make.py | 51 ------------------- 4 files changed, 143 deletions(-) delete mode 100644 tests/build_system/warn_conflict/Makefile delete mode 100644 tests/build_system/warn_conflict/README.md delete mode 100644 tests/build_system/warn_conflict/main.c delete mode 100755 tests/build_system/warn_conflict/tests/01-make.py diff --git a/tests/build_system/warn_conflict/Makefile b/tests/build_system/warn_conflict/Makefile deleted file mode 100644 index 7154a24cd5..0000000000 --- a/tests/build_system/warn_conflict/Makefile +++ /dev/null @@ -1,13 +0,0 @@ -BOARD ?= stm32f4discovery -include ../Makefile.build_system_common - -# The stm32f4discovery is the only board that provides known conflicting features, -# so using this compile test on other boards will not provide the expected warning. -BOARD_WHITELIST := stm32f4discovery - -# These features have a chance to use/access the shared `SPI_DEV(0)` on -# stm32f4discovery, which would probably produce an unexpected behavior in the -# user application. -FEATURES_REQUIRED = periph_dac periph_spi - -include $(RIOTBASE)/Makefile.include diff --git a/tests/build_system/warn_conflict/README.md b/tests/build_system/warn_conflict/README.md deleted file mode 100644 index 98ae732225..0000000000 --- a/tests/build_system/warn_conflict/README.md +++ /dev/null @@ -1,41 +0,0 @@ -Test warning on conflicting features -================================================== -Using conflicting features provided by boards was invisible for the user until the used features resulted in a traceable problem or the user was aware of the conflict in advance from documentation etc. -Now, existing and known conflicts can be recorded into `FEATURES_CONFLICT` for each board to inform the user on a conflict situation during compile time. - -This test requires conflicting features in its `Makefile`, i.e. `FEATURES_REQUIRED = periph_dac periph_spi`. -It is expected to be presented with a warning on the conflicts with a short description message during compile time for the [stm32f4discovery](https://doc.riot-os.org/group__boards__stm32f4discovery.html) by now, i.e. : - -``` -$ make BOARD=stm32f4discovery -The following features may conflict: periph_dac periph_spi -Rationale: On stm32f4discovery boards there are the same pins for the DAC and/or SPI_DEV(0). - -EXPECT undesired behaviour! -``` -The warning presents the conflicting features derived from `FEATURES_CONFLICT` and an optional message derived from `FEATURES_CONFLICT_MSG` provided int the `./RIOT/board/stm32f4discovery/Makefile.features`. - -Whenever an application, such as this test, requires board features that match a _conflict group_, e.g. `FEATURES_REQUIRED = periph_dac periph_spi`, a similar warning to the above will be displayed during compile time. - - ---------- -###Usage of _conflict groups_: - -* Conflicting features are described in groups separated by a `:` (doublecolon) for each feature, e.g.: -`FEATURES_CONFLICT = periph_spi:periph_dac`, which states that `periph_spi` conflicts with `periph_dac`. -As seen above, this is the conflict of `SPI_DEV(0)` pinout is shared with `DAC` on the [stm32f4discovery](https://doc.riot-os.org/group__boards__stm32f4discovery.html) board. - -* Distinct groups of conflicts are whitespace separated, e.g.: -`featureA:featureB featureC:featureD`, which states that `featureA` conflicts with `featureB`, and `featureC` conflicts with `featureD`. -This also means, that e.g. `FEATURES_REQUIRED = featureA featureD` would **not** produce a warning. - -* The groups can have an arbitrary number of conflicting features, e.g.: -`featureA:featureB:featureC:featureX:featureY:featureZ` - -* An optional information can be given using the `FEATURES_CONFLICT_MSG`, e.g.: -`FEATURES_CONFLICT_MSG = "featureA uses the same pins as featureB"` - -* If the required features match multiple conflict groups, **ALL** conflicting features are provided to the user, e.g.: -`FEATURES_CONFLICT = featureA:featureB featureC:featureD` and -`FEATURES_REQUIRED = featureA featureB featureC featureD` -would result in: `The following features may conflict: featureA featureB featureC featureD` diff --git a/tests/build_system/warn_conflict/main.c b/tests/build_system/warn_conflict/main.c deleted file mode 100644 index 6e5230d6b1..0000000000 --- a/tests/build_system/warn_conflict/main.c +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (C) 2014 Martin Landsmann - * - * This library is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2.1 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA - */ - -/** - * @ingroup tests - * @{ - * - * @file - * @brief Test whether EXPECT_CONFLICT works. - * - * @author Martin Landsmann - * - * @} - */ - -#include - -int main(void) -{ - puts("Hello, nothing to do here.\nbye."); - - return 0; -} diff --git a/tests/build_system/warn_conflict/tests/01-make.py b/tests/build_system/warn_conflict/tests/01-make.py deleted file mode 100755 index 0e67d10acb..0000000000 --- a/tests/build_system/warn_conflict/tests/01-make.py +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env python3 - -import os -import sys -import subprocess -from traceback import print_tb -import pexpect - -BOARD = os.getenv('BOARD', 'stm32f4discovery') -MAKE = os.environ.get('MAKE', 'make') - - -def testfunc(): - cross_gcc = "arm-none-eabi-gcc" - - try: - devnull = open(os.devnull) - subprocess.Popen([cross_gcc], - stdout=devnull, stderr=devnull).communicate() - except OSError as exc: - if exc.errno == os.errno.ENOENT: - print("ABORTING TEST: {} seems to be missing.\n".format(cross_gcc)) - else: - child = pexpect.spawnu([MAKE], env=os.environ) - child.logfile = sys.stdout - - try: - if BOARD == 'stm32f4discovery': - child.expect_exact('\x1b[1;33mThe following features may conflict:' - '\x1b[0m periph_dac periph_spi') - child.expect_exact('\x1b[1;33mRationale: ' - '\x1b[0mOn stm32f4discovery boards there are ' - 'the same pins for the DAC and/or SPI_0.') - child.expect_exact('\x1b[1;33mEXPECT undesired behaviour!\x1b[0m') - else: - child.expect_exact('\x1b[1;31mThe selected BOARD={} is not whitelisted:\x1b[0m stm32f4discovery' - .format(BOARD)) - except pexpect.TIMEOUT: - print("\x1b[1;31mTimeout in expect script\x1b[0m") - print_tb(sys.exc_info()[2]) - sys.exit(1) - except pexpect.EOF: - print("\x1b[1;31mUnexpected end of file in expect script\x1b[0m") - print_tb(sys.exc_info()[2]) - sys.exit(1) - finally: - child.close() - - -if __name__ == '__main__': - testfunc()