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.

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

Conversation

nordicjm
Copy link
Collaborator

@nordicjm nordicjm commented Apr 8, 2024

Fixes an issue whereby board overlays for board revisions were not included when configuring applications

Fixes #71203

cmake/modules/configuration_files.cmake Outdated Show resolved Hide resolved
@nordicjm nordicjm force-pushed the fixmissingoverlay branch 2 times, most recently from 9517cbe to 6ebb408 Compare April 9, 2024 14:48
@nordicjm nordicjm requested a review from tejlmand April 9, 2024 14:50
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.

cmake/modules/configuration_files.cmake Outdated Show resolved Hide resolved
nordicjm and others added 3 commits April 10, 2024 07:55
Fixes an issue whereby board overlays for board revisions were not
included when configuring applications

Signed-off-by: Jamie McCrae <[email protected]>
Renames prefixes in some functions to avoid clashes with global
variables that have the same name

Signed-off-by: Jamie McCrae <[email protected]>
for Kconfig fragments in the `boards` application subdirectory.

Signed-off-by: Grzegorz Swiderski <[email protected]>
(cherry picked from commit 3cff550)
@nordicjm nordicjm force-pushed the fixmissingoverlay branch from 6ebb408 to 9f2f78b Compare April 10, 2024 06:55
@nordicjm nordicjm requested a review from tejlmand April 10, 2024 06:57
Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

Approved, because code fixes the issue, but would still prefer cherry-picked commits with conflict resolving for trace-ability reasons.

@nordicjm
Copy link
Collaborator Author

Ping @henrikbrixandersen

@MaureenHelm MaureenHelm merged commit df2c986 into zephyrproject-rtos:v3.6-branch Apr 26, 2024
17 checks passed
@nordicjm nordicjm deleted the fixmissingoverlay branch July 12, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Build System Regression Something, which was working, does not anymore
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[v3.6-branch] Broken behaviour for boards with revision overlays
6 participants