From c7633de038c8e66ced5fa90b436fc8ff042e3c1d Mon Sep 17 00:00:00 2001 From: Marc Herbert Date: Sat, 6 Jul 2019 15:52:31 -0700 Subject: [PATCH] sanitycheck: fix priority of --extra-args=CONFIG_ over testcase.yaml sanitycheck takes any "extra_config" list found in the testcase.yaml file and generates an "overlay" file from it. This file is placed in the per-test build directory and passed to cmake/kconfig.cmake through a -DOVERLAY_CONF= option set in the (also) generated sanity-out/Makefile. This commit moves this generated config overlay to a subdirectory one level down from the build directory, otherwise kconfig.cmake picks it up *twice*: once from the -DOVERLAY_CONF= option already mentioned above and a second time because kconfig.cmake scans the build directory and blindly picks up ALL files ending with .conf[*]. The second pickup is problematic because kconfig.cmake currently gives it the top precedence, even higher than anything the user espressed with --extra-args=CONFIG_* Here's a quick and simple demonstration of the issue fixed by this commit: cd $ZEPHYR_BASE/samples/net/sockets/net_mgmt/ sanitycheck -T. -p qemu_x86 -b -v # --extra-args=CONFIG_USERSPACE=y|n grep CONFIG_USERSPACE $(find sanity-out/ -name .config) .net_mgmt.kernelmode/zephyr/.config: # CONFIG_USERSPACE is not set .net_mgmt.usermode/zephyr/.config: CONFIG_USERSPACE=y grep 'Merged configuration' $(find sanity-out/ -name build.log) Without this commit, attemps to override anything with --extra-args=CONFIG_ are silently dropped on the floor. For more background this issue was found while using the recipe in commit message 4afcc0f8af2b [*] picking up all .conf files is debatable but a much bigger debate with backward compatibility implications. This small fix makes absolutely zero difference to anyone or anything not using sanitycheck. Signed-off-by: Marc Herbert --- cmake/kconfig.cmake | 2 +- scripts/sanitycheck | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cmake/kconfig.cmake b/cmake/kconfig.cmake index 1633c90bd66..e59a32e85d1 100644 --- a/cmake/kconfig.cmake +++ b/cmake/kconfig.cmake @@ -94,7 +94,7 @@ get_cmake_property(cache_variable_names CACHE_VARIABLES) foreach (name ${cache_variable_names}) if("${name}" MATCHES "^CONFIG_") # When a cache variable starts with 'CONFIG_', it is assumed to be - # a CLI Kconfig symbol assignment. + # a Kconfig symbol assignment from the CMake command line. set(EXTRA_KCONFIG_OPTIONS "${EXTRA_KCONFIG_OPTIONS}\n${name}=${${name}}" ) diff --git a/scripts/sanitycheck b/scripts/sanitycheck index b66cc3fad71..48697145172 100755 --- a/scripts/sanitycheck +++ b/scripts/sanitycheck @@ -1334,7 +1334,8 @@ class MakeGenerator: if len(ti.test.extra_configs) > 0 or options.coverage: args.append("OVERLAY_CONFIG=\"%s %s\"" %(overlays, - os.path.join(ti.outdir, "overlay.conf"))) + os.path.join(ti.outdir, + "sanitycheck", "testcase_extra.conf"))) if ti.test.type == "unit" and options.enable_coverage: args.append("COVERAGE=1") @@ -1885,8 +1886,13 @@ class TestInstance: return build_only def create_overlay(self, platform): - file = os.path.join(self.outdir, "overlay.conf") - os.makedirs(self.outdir, exist_ok=True) + # Create this in a "sanitycheck/" subdirectory otherwise this + # will pass this overlay to kconfig.py *twice* and kconfig.cmake + # will silently give that second time precedence over any + # --extra-args=CONFIG_* + subdir = os.path.join(self.outdir, "sanitycheck") + os.makedirs(subdir, exist_ok=True) + file = os.path.join(subdir, "testcase_extra.conf") with open(file, "w") as f: content = ""