8000 cmake: adding $<SEMICOLON> instead of space on zephyr_get_compile_<type> by tejlmand · Pull Request #30155 · zephyrproject-rtos/zephyr · GitHub
[go: up one dir, main page]
Skip to content

cmake: adding $<SEMICOLON> instead of space on zephyr_get_compile_<type> #30155

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

Jump to bottom
Merged
merged 3 commits into from
Dec 3, 2020
Merged

cmake: adding $<SEMICOLON> instead of space on zephyr_get_compile_<type> #30155

merged 3 commits into from
Dec 3, 2020

Conversation

tejlmand
Copy link
Contributor
@tejlmand tejlmand commented Nov 20, 2020 •
edited
Loading

The current zephyr_get_compile_<type> flags returns compile / include
flags based on generator expressions.

But they also included a space.

This have the unintended side-effect that when the properties are
fetched into another variable and later appended to lists in order to
finally be used in a CMake target function / custom command, 8000 then CMake
in some cases will quote the string, creating a quoted argument like:
"arg0 arg1 arg2": instead of individual arguments like: "arg0" "arg1"
"arg2".

By using $<SEMICOLON>, then arguments are separated correctly.

Signed-off-by: Torsten Rasmussen Torsten.Rasmussen@nordicsemi.no

@tejlmand tejlmand requested a review from MaureenHelm November 20, 2020 10:06
@tejlmand tejlmand requested a review from nashif as a code owner November 20, 2020 10:06
@tejlmand tejlmand mentioned this pull request Nov 20, 2020
Closed
@tejlmand
cmake: adding $<SEMICOLON> instead of space on zephyr_get_compile_<type>
a6b0a77 
The current zephyr_get_compile_<type> flags returns compile / include
flags based on generator expressions.

But they also included a space.

This have the unintended side-effect that when the properties are
fetched into another variable and later appended to lists in order to
finally be used in a CMake target function / custom command, then CMake
in some cases will quote the string, creating a quoted argument like:
"arg0 arg1 arg2: instead of individual arguments like: "arg0" "arg1"
"arg2".

By using $<SEMICOLON>, then arguments are seperated correctly.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand
cmake: zephyr_get_<flags>_for_lang_as_string() uses space as delimiter
8833a3e 
The use of zephyr_get_<flags>_for_lang_as_string will now use space as
delimiter, this will allow for existing used of the
zephyr_get_<flags>_for_lang_as_string() to return a string with spaces
instead of semicolons as a quoted list.

Note, in most situations it is prefered to have a proper CMake list
returned, in which case zephyr_get_<flags>_for_lang() is probably the
correct function to use.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand tejlmand force-pushed the zephyr_get_compile_flags_fix branch from 77ff932 to 8833a3e Compare November 20, 2020 12:43
@MaureenHelm
Copy link
Member

Thanks @tejlmand ! This indeed fixes the issue in micropython/micropython#6542. Looks like buildkite crashed and burned though. I looked at one of the fails and was able to fix it like this:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9c13719792..9ed7c18733 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -754,7 +754,7 @@ if(CONFIG_CODE_DATA_RELOCATION)
 endif()
 
 if(CONFIG_USERSPACE)
-  zephyr_get_compile_options_for_lang(C compiler_flags_priv)
+  zephyr_get_compile_options_for_lang_as_string(C compiler_flags_priv)
   string(REPLACE "$<TARGET_PROPERTY:compiler,coverage>" ""
          NO_COVERAGE_FLAGS "${compiler_flags_priv}"
   )

cc: @dpgeorge

@tejlmand
Copy link
Contributor Author
tejlmand commented Nov 20, 2020 •
edited
Loading

Thanks @tejlmand ! This indeed fixes the issue in micropython/micropython#6542. Looks like buildkite crashed and burned though. I looked at one of the fails and was able to fix it like this:

Great, thanks for confirming the fix.
I did notice the CI failure, but had to be away.

So thanks for the fix.

@tejlmand
cmake: using zephyr_get_compile_options_for_lang_as_string()
1cfb755 
With the distinction between zephyr_get_compile_options_for_lang() and
zephyr_get_compile_options_for_lang_as_string() regarding list handling
and thus quoting, then zephyr_get_compile_options_for_lang_as_string()
is now used for CMake source file properties for compilation of
kobject_hash.c source file.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
@tejlmand tejlmand force-pushed the zephyr_get_compile_flags_fix branch from 3b1d9f4 to 1cfb755 Compare November 20, 2020 19:20
@nashif nashif merged commit e37d9e6 into zephyrproject-rtos:master Dec 3, 2020
@tejlmand tejlmand mentioned this pull request Dec 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nashif nashif nashif approved these changes

@MaureenHelm MaureenHelm MaureenHelm approved these changes

@abrodkin abrodkin Awaiting requested review from abrodkin

@evgeniy-paltsev evgeniy-paltsev Awaiting requested review from evgeniy-paltsev

Assignees
No one assigned
Labels
area: Build System
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tejlmand @MaureenHelm @nashif
0