CMake: Fix escaping of quotes in response file #15032
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ciarmcom
added
the
release-type: patch
|
@LDong-Arm, thank you for your changes. |
|
Both compilers tested with macros like this? I recall there were issues alike macros but that could be other older issue we fixed. |
I will give netsocket/mesh a try later. If they work, we can run CI to test the change more rigorously. |
LDong-Arm
force-pushed
the
response_file_escape
branch
from
e539009
to
9241e86
Compare
LDong-Arm
commented
Aug 31, 2021
Comment on lines
-10
to
+12
| # Remove macro definitions that contain spaces as the lack of escape sequences and quotation marks | ||
| # in the macro when retrieved using generator expressions causes linker errors. | ||
| # This includes string macros, array macros, and macros with operations. | ||
| # TODO CMake: Add escape sequences and quotation marks where necessary instead of removing these macros. | ||
| # Append -D to all macros and quote them as we pass these as response file to cxx compiler | ||
| set(_compile_definitions | ||
| "$<FILTER:${_compile_definitions},EXCLUDE, +>" | ||
| ) | ||
|
|
||
| # Append -D to all macros as we pass these as response file to cxx compiler | ||
| set(_compile_definitions | ||
| "$<$<BOOL:${_compile_definitions}>:-D$<JOIN:${_compile_definitions}, -D>>" | ||
| "$<$<BOOL:${_compile_definitions}>:'-D$<JOIN:${_compile_definitions},' '-D>'>" |
There was a problem hiding this comment.
@0xc0170 I've tested it with sockets and mesh, both worked fine with both GCC_ARM and ARM compilers. And this also solves problems with macros with spaces inside (e.g. '-DMBED_CONF_LORA_APPLICATION_EUI={0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}'), so I removed the previous workaround.
Hopefully it's a permanent fix, let's run CI to test it on more examples!
LDong-Arm
force-pushed
the
response_file_escape
branch
from
9241e86
to
8c736c1
Compare
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
ccli8
approved these changes
Sep 1, 2021
Mbed Core
automation
moved this from Awaiting review (3)
to Reviewer approved & awaiting CI & merge (3)
Sep 1, 2021
0xc0170
previously approved these changes
Sep 1, 2021
|
We need to address today license check, I'll continue investigating with the test team |
|
Please rebase to the latest master to get github workflow update to fix basic checks. I'll restart tests and this will go in. |
We put macros in a response file compile_time_defs.txt and pass it
to the compiler. Adding a pair of single quotes around each -D flag
ensures macro values are quoted correctly.
For example,
* String
* target_compile_definitions(): either FOO="BAR" or FOO=\"BAR\"
* response file: '-DFOO="BAR"'
* actual definition: #define FOO "BAR"
* Array of integers
* target_compile_definitions(): FOO={1, 2, 3}
* response file: '-DFOO={1, 2, 3}'
* actual definition: #define FOO {1, 2, 3}
LDong-Arm
force-pushed
the
response_file_escape
branch
from
8c736c1
to
d4ee3bd
Compare
mergify
bot
dismissed
0xc0170’s stale review
Pull request has been modified.
Mbed Core
automation
moved this from Reviewer approved & awaiting CI & merge (3)
to Review in progress (4)
Sep 1, 2021
|
Rebased |
0xc0170
approved these changes
Sep 1, 2021
Mbed Core
automation
moved this from Review in progress (4)
to Reviewer approved & awaiting CI & merge (3)
Sep 1, 2021
|
CI restarted |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 2 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
Mbed Core
automation
moved this from Reviewer approved & awaiting CI & merge (3)
to Done
Sep 1, 2021
mbedmain
added
release-version: 6.15.0
Release-pending
and removed
release-type: patch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
We put macros in a response file compile_time_defs.txt and pass it to the compiler. Adding a pair of single quotes around each -D flag ensures macro values are quoted correctly.
For example,
target_compile_definitions(): eitherFOO="BAR"orFOO=\"BAR\"'-DFOO="BAR"'#define FOO "BAR"target_compile_definitions():FOO={1, 2, 3}'-DFOO={1, 2, 3}'#define FOO {1, 2, 3}Impact of changes
Migration actions required
Documentation
None
Pull request type
Test results
Reviewers