cmake: extend zephyr_get() to handle build configurations from sysbuild

Enhance sysbuild controlled configurations. The current scheme of passing settings using `-D` on the CMake invocation is vulnerable to quoting and lists. With `zephyr_get()` in place as a uniform way of handling user controlled settings (CMake cache / environment / CMake local variable) we have a mechanism in place for a cleaner handling of sysbuild controlled settings. This improves the robustness of variable passing and add the same cleans up and simplifies the logic in sysbuild. The Kconfig Zephyr CMake module has been updated accordingly so that CONFIG settings are taken from the sysbuild shadow cache when sysbuild is used as higher level build system. Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
2022-08-18 16:02:50 +02:00 · 2022-08-18 16:02:50 +02:00 · 88ae9cbb9d
parent 44a05e4439
commit 88ae9cbb9d
6 changed files with 144 additions and 95 deletions
--- a/cmake/modules/basic_settings.cmake
+++ b/cmake/modules/basic_settings.cmake
@ -0,0 +1,40 @@
+# SPDX-License-Identifier: Apache-2.0
+#
+# Copyright (c) 2022, Nordic Semiconductor ASA
+
+# Setup basic settings for a Zephyr project.
+#
+# Basic settings are:
+# - sysbuild defined configuration settings
+#
+# Details for sysbuild settings:
+#
+# Sysbuild is a higher level build system used by Zephyr.
+# Sysbuild allows users to build multiple samples for a given system.
+#
+# For this to work, sysbuild manages other Zephyr CMake build systems by setting
+# dedicated build variables.
+# This CMake modules loads the sysbuild cache variables as target properties on
+# a sysbuild_cache target.
+#
+# This ensures that qoutes and lists are correctly preserved.
+
+include_guard(GLOBAL)
+
+if(SYSBUILD)
+  add_custom_target(sysbuild_cache)
+  file(STRINGS "${SYSBUILD_CACHE}" sysbuild_cache_strings)
+  foreach(str ${sysbuild_cache_strings})
+    # Using a regex for matching whole 'VAR_NAME:TYPE=VALUE' will strip semi-colons
+    # thus resulting in lists to become strings.
+    # Therefore we first fetch VAR_NAME and TYPE, and afterwards extract
+    # remaining of string into a value that populates the property.
+    # This method ensures that both quoted values and ;-separated list stays intact.
+    string(REGEX MATCH "([^:]*):([^=]*)=" variable_identifier ${str})
+    string(LENGTH ${variable_identifier} variable_identifier_length)
+    string(SUBSTRING "${str}" ${variable_identifier_length} -1 variable_value)
+    set_property(TARGET sysbuild_cache APPEND PROPERTY "SYSBUILD_CACHE:VARIABLES" "${CMAKE_MATCH_1}")
+    set_property(TARGET sysbuild_cache PROPERTY "${CMAKE_MATCH_1}:TYPE" "${CMAKE_MATCH_2}")
+    set_property(TARGET sysbuild_cache PROPERTY "${CMAKE_MATCH_1}" "${variable_value}")
+  endforeach()
+endif()
--- a/cmake/modules/configuration_files.cmake
+++ b/cmake/modules/configuration_files.cmake
@ -35,7 +35,7 @@ else()
  set(APPLICATION_CONFIG_DIR ${APPLICATION_SOURCE_DIR})
 endif()

-zephyr_get(CONF_FILE)
+zephyr_get(CONF_FILE SYSBUILD LOCAL)
 if(DEFINED CONF_FILE)
  # This ensures that CACHE{CONF_FILE} will be set correctly to current scope
  # variable CONF_FILE. An already current scope variable will stay the same.
@ -89,7 +89,7 @@ zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR}/boards DTS APP_BOARD_DTS)
 # The CONF_FILE variable is now set to its final value.
 zephyr_boilerplate_watch(CONF_FILE)

-zephyr_get(DTC_OVERLAY_FILE)
+zephyr_get(DTC_OVERLAY_FILE SYSBUILD LOCAL)
 if(DTC_OVERLAY_FILE)
  # DTC_OVERLAY_FILE has either been specified on the cmake CLI or is already
  # in the CMakeCache.txt.
--- a/cmake/modules/extensions.cmake
+++ b/cmake/modules/extensions.cmake
@ -2316,15 +2316,23 @@ endfunction()

 # Usage:
 #   zephyr_get(<variable>)
+#   zephyr_get(<variable> SYSBUILD [LOCAL|GLOBAL])
 #
 # Return the value of <variable> as local scoped variable of same name.
 #
 # zephyr_get() is a common function to provide a uniform way of supporting
-# build settings that can be set from CMakeLists.txt, CMake cache, or in
-# environment.
+# build settings that can be set from sysbuild, CMakeLists.txt, CMake cache, or
+# in environment.
 #
 # The order of precedence for variables defined in multiple scopes:
-# - CMake cache, set by `-D<var>=<value>` or `set(<var> <val> CACHE ...)`
+# - Sysbuild defined when sysbuild is used.
+#   Sysbuild variables can be defined as global or local to specific image.
+#   Examples:
+#   - BOARD is considered a global sysbuild cache variable
+#   - blinky_BOARD is considered a local sysbuild cache variable only for the
+#     blinky image.
+#   If no sysbuild scope is specified, GLOBAL is assumed.
+# - CMake cache, set by `-D<var>=<value>` or `set(<var> <val> CACHE ...)
 # - Environment
 # - Locally in CMakeLists.txt before 'find_package(Zephyr)'
 #
@ -2333,9 +2341,32 @@ endfunction()
 # using `-DZEPHYR_TOOLCHAIN_VARIANT=<val>`, then the value from the cache is
 # returned.
 function(zephyr_get variable)
-  cmake_parse_arguments(GET_VAR "" "" "" ${ARGN})
+  cmake_parse_arguments(GET_VAR "" "SYSBUILD" "" ${ARGN})

-  if(DEFINED CACHE{${variable}})
+  if(DEFINED GET_VAR_SYSBUILD)
+    if(NOT (${GET_VAR_SYSBUILD} STREQUAL "GLOBAL" OR
+            ${GET_VAR_SYSBUILD} STREQUAL "LOCAL")
+    )
+      message(FATAL_ERROR "zephyr_get(... SYSBUILD) requires GLOBAL or LOCAL.")
+    endif()
+  else()
+    set(GET_VAR_SYSBUILD "GLOBAL")
+  endif()
+
+  if(SYSBUILD)
+    get_property(sysbuild_name TARGET sysbuild_cache PROPERTY SYSBUILD_NAME)
+    get_property(sysbuild_main_app TARGET sysbuild_cache PROPERTY SYSBUILD_MAIN_APP)
+    get_property(sysbuild_${variable} TARGET sysbuild_cache PROPERTY ${sysbuild_name}_${variable})
+    if(NOT DEFINED sysbuild_${variable} AND
+       (${GET_VAR_SYSBUILD} STREQUAL "GLOBAL" OR sysbuild_main_app)
+    )
+      get_property(sysbuild_${variable} TARGET sysbuild_cache PROPERTY ${variable})
+    endif()
+  endif()
+
+  if(DEFINED sysbuild_${variable})
+    set(${variable} ${sysbuild_${variable}} PARENT_SCOPE)
+  elseif(DEFINED CACHE{${variable}})
    set(${variable} $CACHE{${variable}} PARENT_SCOPE)
  elseif(DEFINED ENV{${variable}})
    set(${variable} $ENV{${variable}} PARENT_SCOPE)
--- a/cmake/modules/kconfig.cmake
+++ b/cmake/modules/kconfig.cmake
@ -72,7 +72,7 @@ if(CONF_FILE)
 string(REPLACE " " ";" CONF_FILE_AS_LIST "${CONF_FILE}")
 endif()

-zephyr_get(OVERLAY_CONFIG)
+zephyr_get(OVERLAY_CONFIG SYSBUILD LOCAL)
 if(OVERLAY_CONFIG)
  string(REPLACE " " ";" OVERLAY_CONFIG_AS_LIST "${OVERLAY_CONFIG}")
 endif()
@ -181,7 +181,25 @@ endforeach()
 # This feature is experimental and undocumented until it has undergone more
 # user-testing.
 unset(EXTRA_KCONFIG_OPTIONS)
-get_cmake_property(cache_variable_names CACHE_VARIABLES)
+if(SYSBUILD)
+  get_property(sysbuild_variable_names TARGET sysbuild_cache PROPERTY "SYSBUILD_CACHE:VARIABLES")
+  zephyr_get(SYSBUILD_MAIN_APP)
+  zephyr_get(SYSBUILD_NAME)
+
+  foreach (name ${sysbuild_variable_names})
+    if("${name}" MATCHES "^${SYSBUILD_NAME}_${KCONFIG_NAMESPACE}_")
+      string(REGEX REPLACE "^${SYSBUILD_NAME}_" "" org_name ${name})
+      get_property(${org_name} TARGET sysbuild_cache PROPERTY ${name})
+      list(APPEND cache_variable_names ${org_name})
+    elseif(SYSBUILD_MAIN_APP AND "${name}" MATCHES "^${KCONFIG_NAMESPACE}_")
+      get_property(${name} TARGET sysbuild_cache PROPERTY ${name})
+      list(APPEND cache_variable_names ${name})
+    endif()
+  endforeach()
+else()
+  get_cmake_property(cache_variable_names CACHE_VARIABLES)
+endif()
+
 foreach (name ${cache_variable_names})
  if("${name}" MATCHES "^CLI_${KCONFIG_NAMESPACE}_")
    # Variable was set by user in earlier invocation, let's append to extra
--- a/cmake/modules/zephyr_default.cmake
+++ b/cmake/modules/zephyr_default.cmake
@ -54,6 +54,9 @@ list(APPEND zephyr_cmake_modules user_cache)
 list(APPEND zephyr_cmake_modules extensions)
 list(APPEND zephyr_cmake_modules version)

+# Load basic settings
+list(APPEND zephyr_cmake_modules basic_settings)
+
 #
 # Find tools
 #
--- a/share/sysbuild/cmake/modules/sysbuild_extensions.cmake
+++ b/share/sysbuild/cmake/modules/sysbuild_extensions.cmake
@ -32,105 +32,60 @@ function(ExternalZephyrProject_Add)
    )
  endif()

-  set(sysbuild_vars
-      "APP_DIR"
-      "SB_CONF_FILE"
-  )
-
-  # General variables that should be propagated to all Zephyr builds, for example:
-  # - ZEPHYR_MODULES / ZEPHYR_EXTRA_MODULES
-  # - ZEPHYR_TOOLCHAIN_VARIANT
-  # - *_TOOLCHAIN_PATH
-  # - *_ROOT
-  # etc.
-  # Note: setting vars on a single image can be done by using
-  #       `<image>_CONF_FILE`, like `mcuboot_CONF_FILE`
+  # CMake variables which must be known by all Zephyr CMake build systems
+  # Those are settings which controls the build and must be known to CMake at
+  # invocation time, and thus cannot be passed though the sysbuild cache file.
  set(
-    shared_image_variables_list
+    shared_cmake_variables_list
    CMAKE_BUILD_TYPE
    CMAKE_VERBOSE_MAKEFILE
-    BOARD
-    ZEPHYR_MODULES
-    ZEPHYR_EXTRA_MODULES
-    ZEPHYR_TOOLCHAIN_VARIANT
-    EXTRA_KCONFIG_TARGETS
  )

-  set(shared_image_variables_regex
-      "^[^_]*_TOOLCHAIN_PATH|^[^_]*_ROOT"
-  )
+  set(sysbuild_cache_file ${CMAKE_BINARY_DIR}/${ZBUILD_APPLICATION}_sysbuild_cache.txt)

-  set(app_cache_file ${CMAKE_BINARY_DIR}/CMake${ZBUILD_APPLICATION}PreloadCache.txt)
-
-  if(EXISTS ${app_cache_file})
-    file(STRINGS ${app_cache_file} app_cache_strings)
-    set(app_cache_strings_current ${app_cache_strings})
-  endif()
-
-  get_cmake_property(variables_cached CACHE_VARIABLES)
-  foreach(var_name ${variables_cached})
-    # Any var of the form `<app>_<var>` should be propagated.
-    # For example mcuboot_<VAR>=<val> ==> -D<VAR>=<val> for mcuboot build.
-    if("${var_name}" MATCHES "^${ZBUILD_APPLICATION}_.*")
-      list(APPEND application_vars ${var_name})
-      continue()
-    endif()
-
-    # This means there is a match to another image than current one, ignore.
-    if("${var_name}" MATCHES "^.*_CONFIG_.*")
-      continue()
-    endif()
-
-    # sysbuild reserved namespace.
-    if(var_name IN_LIST sysbuild_vars OR "${var_name}" MATCHES "^SB_CONFIG_.*")
-      continue()
-    endif()
-
-    if("${var_name}" MATCHES "^CONFIG_.*")
-      if(ZBUILD_MAIN_APP)
-        list(APPEND application_vars ${var_name})
-      endif()
-      continue()
-    endif()
-
-    if(var_name IN_LIST shared_image_variables_list)
-      list(APPEND application_vars ${var_name})
-      continue()
-    endif()
-
-    if("${var_name}" MATCHES "${shared_image_variables_regex}")
-      list(APPEND application_vars ${var_name})
+  get_cmake_property(sysbuild_cache CACHE_VARIABLES)
+  foreach(var_name ${sysbuild_cache})
+    if(NOT "${var_name}" MATCHES "^CMAKE_.*")
+      # We don't want to pass internal CMake variables.
+      # Required CMake variable to be passed, like CMAKE_BUILD_TYPE must be
+      # passed using `-D` on command invocation.
+      get_property(var_type CACHE ${var_name} PROPERTY TYPE)
+      set(cache_entry "${var_name}:${var_type}=${${var_name}}")
+      string(REPLACE ";" "\;" cache_entry "${cache_entry}")
+      list(APPEND sysbuild_cache_strings "${cache_entry}\n")
    endif()
  endforeach()
+  list(APPEND sysbuild_cache_strings "SYSBUILD_NAME:STRING=${ZBUILD_APPLICATION}\n")

-  foreach(app_var_name ${application_vars})
-    string(REGEX REPLACE "^${ZBUILD_APPLICATION}_" "" var_name "${app_var_name}")
-    get_property(var_type  CACHE ${app_var_name} PROPERTY TYPE)
-    set(new_cache_entry "${var_name}:${var_type}=${${app_var_name}}")
-    if(NOT new_cache_entry IN_LIST app_cache_strings)
-      # This entry does not exists, let's see if it has been updated.
-      foreach(entry ${app_cache_strings})
-        if("${entry}" MATCHES "^${var_name}:.*")
-          list(REMOVE_ITEM app_cache_strings "${entry}")
-          break()
-        endif()
-      endforeach()
-      list(APPEND app_cache_strings "${var_name}:${var_type}=${${app_var_name}}")
-      list(APPEND app_cache_entries "-D${var_name}:${var_type}=${${app_var_name}}")
-    endif()
-  endforeach()
-
-  if(NOT "${app_cache_strings_current}" STREQUAL "${app_cache_strings}")
-    string(REPLACE ";" "\n" app_cache_strings "${app_cache_strings}")
-    file(WRITE ${app_cache_file} ${app_cache_strings})
+  if(ZBUILD_MAIN_APP)
+    list(APPEND sysbuild_cache_strings "SYSBUILD_MAIN_APP:BOOL=True\n")
  endif()

  if(DEFINED ZBUILD_BOARD)
-    list(APPEND app_cache_entries "-DBOARD=${ZBUILD_BOARD}")
-  elseif(NOT ZBUILD_MAIN_APP)
-    list(APPEND app_cache_entries "-DBOARD=${BOARD}")
+    # Only set image specific board if provided.
+    # The sysbuild BOARD is exported through sysbuild cache, and will be used
+    # unless <image>_BOARD is defined.
+    list(APPEND sysbuild_cache_strings "${ZBUILD_APPLICATION}_BOARD:STRING=${ZBUILD_BOARD}\n")
  endif()

+  file(WRITE ${sysbuild_cache_file}.tmp ${sysbuild_cache_strings})
+  zephyr_file_copy(${sysbuild_cache_file}.tmp ${sysbuild_cache_file} ONLY_IF_DIFFERENT)
+
+  set(shared_cmake_vars_argument)
+  foreach(shared_var ${shared_cmake_variables_list})
+    if(DEFINED CACHE{${ZBUILD_APPLICATION}_${shared_var}})
+      get_property(var_type  CACHE ${ZBUILD_APPLICATION}_${shared_var} PROPERTY TYPE)
+      list(APPEND shared_cmake_vars_argument
+           "-D${shared_var}:${var_type}=$CACHE{${ZBUILD_APPLICATION}_${shared_var}}"
+      )
+    elseif(DEFINED CACHE{${shared_var}})
+      get_property(var_type  CACHE ${shared_var} PROPERTY TYPE)
+      list(APPEND shared_cmake_vars_argument
+           "-D${shared_var}:${var_type}=$CACHE{${shared_var}}"
+      )
+    endif()
+  endforeach()
+
  set(image_banner "* Running CMake for ${ZBUILD_APPLICATION} *")
  string(LENGTH "${image_banner}" image_banner_width)
  string(REPEAT "*" ${image_banner_width} image_banner_header)
@ -142,7 +97,9 @@ function(ExternalZephyrProject_Add)
  execute_process(
    COMMAND ${CMAKE_COMMAND}
      -G${CMAKE_GENERATOR}
-      ${app_cache_entries}
+      -DSYSBUILD:BOOL=True
+      -DSYSBUILD_CACHE:FILEPATH=${sysbuild_cache_file}
+      ${shared_cmake_vars_argument}
      -B${CMAKE_BINARY_DIR}/${ZBUILD_APPLICATION}
      -S${ZBUILD_SOURCE_DIR}
    RESULT_VARIABLE   return_val