From d29652350b82e9e3af6a620383848e51a4b1ec73 Mon Sep 17 00:00:00 2001
From: Marian Buschsieweke <marian.buschsieweke@posteo.net>
Date: Wed, 21 Feb 2024 20:55:41 +0100
Subject: [PATCH] build system: Improve failure mode for FEAUTRES_REQUIRED_ANY

Failing to provide any of the required features can provide a message
such as:

    There are unsatisfied feature requirements: periph_uart|periph_lpuart

This can be confusing and may hide the actual. E.g. above message
was generated when using SPI on the `msb-430` and `stdio_uart`. However,
the MSB-430 board *does* provide `periph_uart`, so this looks like a bug
in the feature resolution. This changes the failure mode of
`FEATURES_REQUIRED_ANY` to just pick the first of the alternatives
given if none of the alternative is usable, which gives in the example
the following message instead:

    The following features may conflict: periph_spi periph_uart
    Rationale: Both SPI and UART are provided by the same USART
               peripheral

The output is less surprising and can provide non-obvious reasons
why `FEATURES_REQUIRED_ANY` failed to pick a feature. The downside is
that the alternatives are no longer visible. However, that output
likely was so confusing this might be for the best.

Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
---
 makefiles/features_check.inc.mk                       | 11 ++++++-----
 .../tests/feature_resolution/test-complex-failure.mk  |  6 +++---
 .../feature_resolution/test-required-any-fail.mk      |  4 ++--
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/makefiles/features_check.inc.mk b/makefiles/features_check.inc.mk
index 9f010b36ce..907be7b1ca 100644
--- a/makefiles/features_check.inc.mk
+++ b/makefiles/features_check.inc.mk
@@ -44,17 +44,18 @@ FEATURES_USED_SO_FAR := $(sort $(FEATURES_REQUIRED) $(FEATURES_OPTIONAL_USED))
 #   ==> If one (or more) already used features is listed in item, this (or
 #       one of these) will be in the front of "tmp" and be taken
 #   ==> If one (or more) usable features is listed in item, this will come
-#       afterwards. If no no feature in item is used so for, one of the
+#       afterwards. If no feature in item is used so far, one of the
 #       features supported and listed in item will be picked
-#   ==> At the end of the list item itself (with pipes between features) is the
-#       last item in "tmp". If no feature is item is supported or used, this
-#       will be the only item in "tmp" and be picked
+#   ==> At the end of the list, feature alternatives in item itself are added.
+#       If no feature in item is already used or usable, the first alternative is
+#       just added, triggering a warning about a feature being missing or
+#       conflicting later on.
 FEATURES_REQUIRED_ONE_OUT_OF := $(foreach item,\
                                   $(FEATURES_REQUIRED_ANY),\
                                   $(word 1,\
                                     $(filter $(FEATURES_USED_SO_FAR),$(subst |, ,$(item)))\
                                     $(filter $(FEATURES_USABLE),$(subst |, ,$(item)))\
-                                    $(item)))
+                                    $(subst |, ,$(item))))
 
 # Features that are required by the application but not provided by the BSP
 # Having features missing may case the build to fail.
diff --git a/makefiles/tests/feature_resolution/test-complex-failure.mk b/makefiles/tests/feature_resolution/test-complex-failure.mk
index b371a89156..8c44e40ccd 100644
--- a/makefiles/tests/feature_resolution/test-complex-failure.mk
+++ b/makefiles/tests/feature_resolution/test-complex-failure.mk
@@ -7,9 +7,9 @@ FEATURES_BLACKLIST := a
 FEATURES_CONFLICT := f:e c:d
 
 # expected results
-EXPECTED_FEATURES_USED := a|h|i b c d
-EXPECTED_FEATURES_MISSING := a|h|i
-EXPECTED_FEATURES_USED_BLACKLISTED :=
+EXPECTED_FEATURES_USED := a b c d
+EXPECTED_FEATURES_MISSING :=
+EXPECTED_FEATURES_USED_BLACKLISTED := a
 EXPECTED_FEATURES_CONFLICTING := c d
 
 include Makefile.test
diff --git a/makefiles/tests/feature_resolution/test-required-any-fail.mk b/makefiles/tests/feature_resolution/test-required-any-fail.mk
index 137614e95e..aa7b4dabf6 100644
--- a/makefiles/tests/feature_resolution/test-required-any-fail.mk
+++ b/makefiles/tests/feature_resolution/test-required-any-fail.mk
@@ -7,8 +7,8 @@ FEATURES_BLACKLIST :=
 FEATURES_CONFLICT :=
 
 # expected results
-EXPECTED_FEATURES_USED := d|b|a
-EXPECTED_FEATURES_MISSING := d|b|a
+EXPECTED_FEATURES_USED := d
+EXPECTED_FEATURES_MISSING := d
 EXPECTED_FEATURES_USED_BLACKLISTED :=
 EXPECTED_FEATURES_CONFLICTING :=