Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v3.6-branch] cmake: modules: extensions: Fix missing board revision overlays #71212

Merged
merged 3 commits into from
Apr 26, 2024
Merged

[v3.6-branch] cmake: modules: extensions: Fix missing board revision overlays #71212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
255e587
cmake: modules: extensions: Fix missing board revision overlays
nordicjm Apr 8, 2024
03af6a2
cmake: modules: extensions: Rename prefixes in functions
nordicjm Apr 9, 2024
9f2f78b
cmake: configuration_files: Add missing FILE_SUFFIX handling
57300 Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions cmake/modules/configuration_files.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ endif()
zephyr_get(CONF_FILE SYSBUILD LOCAL)
if(NOT DEFINED CONF_FILE)
zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR} KCONF CONF_FILE NAMES "prj.conf" SUFFIX ${FILE_SUFFIX} REQUIRED)
zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR}/boards KCONF CONF_FILE)
zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR}/boards KCONF CONF_FILE SUFFIX ${FILE_SUFFIX})
else()
string(CONFIGURE "${CONF_FILE}" CONF_FILE_EXPANDED)
string(REPLACE " " ";" CONF_FILE_AS_LIST "${CONF_FILE_EXPANDED}")
Expand Down Expand Up @@ -77,8 +77,12 @@ zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR}/boards DTS APP_BOARD_DTS SUFFIX

zephyr_get(DTC_OVERLAY_FILE SYSBUILD LOCAL)
if(NOT DEFINED DTC_OVERLAY_FILE)
zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR} DTS DTC_OVERLAY_FILE
NAMES "${APP_BOARD_DTS};${BOARD}.overlay;app.overlay" SUFFIX ${FILE_SUFFIX})
if(DEFINED APP_BOARD_DTS)
set(DTC_OVERLAY_FILE ${APP_BOARD_DTS})
else()
zephyr_file(CONF_FILES ${APPLICATION_CONFIG_DIR} DTS DTC_OVERLAY_FILE
NAMES "${BOARD}.overlay;app.overlay" SUFFIX ${FILE_SUFFIX})
endif()
endif()

set(DTC_OVERLAY_FILE ${DTC_OVERLAY_FILE} CACHE STRING "If desired, you can \
Expand Down
95 changes: 48 additions & 47 deletions cmake/modules/extensions.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2481,76 +2481,76 @@ Please provide one of following: APPLICATION_ROOT, CONF_FILES")
set(multi_args CONF_FILES NAMES)
endif()

cmake_parse_arguments(FILE "${options}" "${single_args}" "${multi_args}" ${ARGN})
if(FILE_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "zephyr_file(${ARGV0} <val> ...) given unknown arguments: ${FILE_UNPARSED_ARGUMENTS}")
cmake_parse_arguments(ZFILE "${options}" "${single_args}" "${multi_args}" ${ARGN})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of creating a new commit with combination of multiple commits, then I believe the proper way is to actually do a cherry-pick of needed commits from zephyr/main.

That will both make it easier to identify identical changes / commits between branches, as well as help identifying cases where some commits might have been forgotten.

In this case it appears to me that the changes in 6ebb408921c535c45ca4c31b9ded672a0dae456c lacks the fixes from 3cff550, and hence while trying to fix #71203 you introduce a new error.

Please cherry-pick relevant commits instead, which seems to be the following:
50f0454
3cff550
0301305 (from PR #71280)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not cherry-picked because it does not apply, the changes in those linked commits use MERGE functions which are not in 3.6

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's not cherry-picked but you can still cherry-pick and during conflict resolving you'll get the chance to adjust the commit wrt. the MERGE functionality and the board qualifiers.

And the -x makes it even easier to see the origin of the fix.

End result in code should of course be identical to re-implementing the changes, but making it much easier to trace which fixes from main that has been applied in the branch.

if(ZFILE_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "zephyr_file(${ARGV0} <val> ...) given unknown arguments: ${ZFILE_UNPARSED_ARGUMENTS}")
endif()

if(FILE_APPLICATION_ROOT)
if(ZFILE_APPLICATION_ROOT)
# Note: user can do: `-D<var>=<relative-path>` and app can at same
# time specify `list(APPEND <var> <abs-path>)`
# Thus need to check and update only CACHED variables (-D<var>).
set(CACHED_PATH $CACHE{${FILE_APPLICATION_ROOT}})
set(CACHED_PATH $CACHE{${ZFILE_APPLICATION_ROOT}})
foreach(path ${CACHED_PATH})
# The cached variable is relative path, i.e. provided by `-D<var>` or
# `set(<var> CACHE)`, so let's update current scope variable to absolute
# path from `APPLICATION_SOURCE_DIR`.
if(NOT IS_ABSOLUTE ${path})
set(abs_path ${APPLICATION_SOURCE_DIR}/${path})
list(FIND ${FILE_APPLICATION_ROOT} ${path} index)
list(FIND ${ZFILE_APPLICATION_ROOT} ${path} index)
if(NOT ${index} LESS 0)
list(REMOVE_AT ${FILE_APPLICATION_ROOT} ${index})
list(INSERT ${FILE_APPLICATION_ROOT} ${index} ${abs_path})
list(REMOVE_AT ${ZFILE_APPLICATION_ROOT} ${index})
list(INSERT ${ZFILE_APPLICATION_ROOT} ${index} ${abs_path})
endif()
endif()
endforeach()

# Now all cached relative paths has been updated.
# Let's check if anyone uses relative path as scoped variable, and fail
foreach(path ${${FILE_APPLICATION_ROOT}})
foreach(path ${${ZFILE_APPLICATION_ROOT}})
if(NOT IS_ABSOLUTE ${path})
message(FATAL_ERROR
"Relative path encountered in scoped variable: ${FILE_APPLICATION_ROOT}, value=${path}\n \
Please adjust any `set(${FILE_APPLICATION_ROOT} ${path})` or `list(APPEND ${FILE_APPLICATION_ROOT} ${path})`\n \
"Relative path encountered in scoped variable: ${ZFILE_APPLICATION_ROOT}, value=${path}\n \
Please adjust any `set(${ZFILE_APPLICATION_ROOT} ${path})` or `list(APPEND ${ZFILE_APPLICATION_ROOT} ${path})`\n \
to absolute path using `\${CMAKE_CURRENT_SOURCE_DIR}/${path}` or similar. \n \
Relative paths are only allowed with `-D${ARGV1}=<path>`")
endif()
endforeach()

# This updates the provided argument in parent scope (callers scope)
set(${FILE_APPLICATION_ROOT} ${${FILE_APPLICATION_ROOT}} PARENT_SCOPE)
set(${ZFILE_APPLICATION_ROOT} ${${ZFILE_APPLICATION_ROOT}} PARENT_SCOPE)
endif()

if(FILE_CONF_FILES)
if(DEFINED FILE_BOARD_REVISION AND NOT FILE_BOARD)
if(ZFILE_CONF_FILES)
if(DEFINED ZFILE_BOARD_REVISION AND NOT ZFILE_BOARD)
message(FATAL_ERROR
"zephyr_file(${ARGV0} <path> BOARD_REVISION ${FILE_BOARD_REVISION} ...)"
"zephyr_file(${ARGV0} <path> BOARD_REVISION ${ZFILE_BOARD_REVISION} ...)"
" given without BOARD argument, please specify BOARD"
)
endif()

if(NOT DEFINED FILE_BOARD)
if(NOT DEFINED ZFILE_BOARD)
# Defaulting to system wide settings when BOARD is not given as argument
set(FILE_BOARD ${BOARD})
set(ZFILE_BOARD ${BOARD})
if(DEFINED BOARD_REVISION)
set(FILE_BOARD_REVISION ${BOARD_REVISION})
set(ZFILE_BOARD_REVISION ${BOARD_REVISION})
endif()
endif()

if(FILE_NAMES)
set(dts_filename_list ${FILE_NAMES})
set(kconf_filename_list ${FILE_NAMES})
if(ZFILE_NAMES)
set(dts_filename_list ${ZFILE_NAMES})
set(kconf_filename_list ${ZFILE_NAMES})
else()
zephyr_build_string(filename
BOARD ${FILE_BOARD}
BUILD ${FILE_BUILD}
BOARD ${ZFILE_BOARD}
BUILD ${ZFILE_BUILD}
)
set(filename_list ${filename})

zephyr_build_string(filename
BOARD ${FILE_BOARD}
BOARD_REVISION ${FILE_BOARD_REVISION}
BUILD ${FILE_BUILD}
BOARD ${ZFILE_BOARD}
BOARD_REVISION ${ZFILE_BOARD_REVISION}
BUILD ${ZFILE_BUILD}
)
list(APPEND filename_list ${filename})
list(REMOVE_DUPLICATES filename_list)
Expand All @@ -2561,73 +2561,74 @@ Relative paths are only allowed with `-D${ARGV1}=<path>`")
list(TRANSFORM kconf_filename_list APPEND ".conf")
endif()

if(FILE_DTS)
foreach(path ${FILE_CONF_FILES})
if(ZFILE_DTS)
foreach(path ${ZFILE_CONF_FILES})
foreach(filename ${dts_filename_list})
if(NOT IS_ABSOLUTE ${filename})
set(test_file ${path}/${filename})
else()
set(test_file ${filename})
endif()
zephyr_file_suffix(test_file SUFFIX ${FILE_SUFFIX})
zephyr_file_suffix(test_file SUFFIX ${ZFILE_SUFFIX})

if(EXISTS ${test_file})
list(APPEND ${FILE_DTS} ${test_file})
list(APPEND ${ZFILE_DTS} ${test_file})

if(DEFINED FILE_BUILD)
if(DEFINED ZFILE_BUILD)
set(deprecated_file_found y)
endif()

if(FILE_NAMES)
if(ZFILE_NAMES)
break()
endif()
endif()
endforeach()
endforeach()

# This updates the provided list in parent scope (callers scope)
set(${FILE_DTS} ${${FILE_DTS}} PARENT_SCOPE)
set(${ZFILE_DTS} ${${ZFILE_DTS}} PARENT_SCOPE)

if(NOT ${FILE_DTS})
if(NOT ${ZFILE_DTS})
set(not_found ${dts_filename_list})
endif()
endif()

if(FILE_KCONF)
foreach(path ${FILE_CONF_FILES})
if(ZFILE_KCONF)
foreach(path ${ZFILE_CONF_FILES})
foreach(filename ${kconf_filename_list})
if(NOT IS_ABSOLUTE ${filename})
set(test_file ${path}/${filename})
else()
set(test_file ${filename})
endif()
zephyr_file_suffix(test_file SUFFIX ${FILE_SUFFIX})

zephyr_file_suffix(test_file SUFFIX ${ZFILE_SUFFIX})

if(EXISTS ${test_file})
list(APPEND ${FILE_KCONF} ${test_file})
list(APPEND ${ZFILE_KCONF} ${test_file})

if(DEFINED FILE_BUILD)
if(DEFINED ZFILE_BUILD)
set(deprecated_file_found y)
endif()

if(FILE_NAMES)
if(ZFILE_NAMES)
break()
endif()
endif()
endforeach()
endforeach()

# This updates the provided list in parent scope (callers scope)
set(${FILE_KCONF} ${${FILE_KCONF}} PARENT_SCOPE)
set(${ZFILE_KCONF} ${${ZFILE_KCONF}} PARENT_SCOPE)

if(NOT ${FILE_KCONF})
if(NOT ${ZFILE_KCONF})
set(not_found ${kconf_filename_list})
endif()
endif()

if(FILE_REQUIRED AND DEFINED not_found)
if(ZFILE_REQUIRED AND DEFINED not_found)
message(FATAL_ERROR
"No ${not_found} file(s) was found in the ${FILE_CONF_FILES} folder(s), "
"No ${not_found} file(s) was found in the ${ZFILE_CONF_FILES} folder(s), "
"please read the Zephyr documentation on application development."
)
endif()
Expand Down Expand Up @@ -2691,9 +2692,9 @@ endfunction()
#
function(zephyr_file_suffix filename)
set(single_args SUFFIX)
cmake_parse_arguments(FILE "" "${single_args}" "" ${ARGN})
cmake_parse_arguments(SFILE "" "${single_args}" "" ${ARGN})

if(NOT DEFINED FILE_SUFFIX OR NOT DEFINED ${filename})
if(NOT DEFINED SFILE_SUFFIX OR NOT DEFINED ${filename})
# If the file suffix variable is not known then there is nothing to do, return early
return()
endif()
Expand All @@ -2709,7 +2710,7 @@ function(zephyr_file_suffix filename)
# Search for the full stop so we know where to add the file suffix before the file extension
cmake_path(GET file EXTENSION file_ext)
cmake_path(REMOVE_EXTENSION file OUTPUT_VARIABLE new_filename)
cmake_path(APPEND_STRING new_filename "_${FILE_SUFFIX}${file_ext}")
cmake_path(APPEND_STRING new_filename "_${SFILE_SUFFIX}${file_ext}")

# Use the filename with the suffix if it exists, if not then fall back to the default
if(EXISTS "${new_filename}")
Expand Down
Loading