Convert retreived path to CMake path for use. #244
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
While working I encountered an error due to mixed CMake and Windows
style paths from this package (Example below).
With this change the path will be converted to a CMake path as soon as
it is validated.
```
CMake Error at cmake/add_launch_test.cmake:67 (if):
Syntax error in cmake code at
C:/Users/Administrator/jacob_ws/src/ros2/launch/launch_testing_ament_cmake/cmake/add_launch_test.cmake:67
when parsing string
c:\Users\Administrator\jacob_ws\install\launch_testing/share/launch_testing/examples/args.test.py
Invalid character escape '\U'.
Call Stack (most recent call first):
cmake/add_launch_test.cmake:105 (parse_launch_test_arguments)
CMakeLists.txt:28 (add_launch_test)
```
Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
|
tfoote
approved these changes
May 18, 2019
dirk-thomas
reviewed
May 18, 2019
Move the conversion into the add_launch_test function so all callers can benefit from it. The conversion must happen before the internal argument parsing macro is called in order to avoid string escape errors on Windows. Signed-off-by: Steven! Ragnarök <steven@nuclearsandwich.com>
nuclearsandwich
force-pushed
the
convert-to-cmake-path
branch
from
0388188
to
37a6404
Compare
|
|
The second commit moves the conversion into the add_launch_test function. I tried at first to do it inside the argument parsing macro but the macro invocation itself has errors if the string contains unescaped backslashes at that point. So it has to be just before it is called. |
jacobperron
approved these changes
May 18, 2019
|
Just saw this on CI: https://ci.ros2.org/job/ci_windows/6980/testReport/junit/(root)/projectroot/C__J_workspace_ci_windows_ws_install_share_launch_testing_examples_args_test_py/. Could be it a regression related to this patch? |
Actually, this was premature. It looks like #239 which landed on top of this independently reproduces the above error. |
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.
While working I encountered an error due to mixed CMake and Windows
style paths from this package (Example below).
With this change the path will be converted to a CMake path as soon as
it is validated.