Skip to content

Conversation

@jacobperron
Copy link
Contributor

@jacobperron jacobperron commented Jun 22, 2021

Changes so that we can build and test against Galactic (the latest ROS 2 distro).

This includes ports of the following changes from the OSRF fork:

* Switch to 'galactic' branches of upstream repositories where applicable
* Move all ROS 2 Java repos into subdirectory 'ros2-java' (this matches the org name)
* Use new 'main' branches for ros2-java repositories

Signed-off-by: Jacob Perron <[email protected]>
These are no longer being used.

Signed-off-by: Jacob Perron <[email protected]>
* Depend on new package rosidl_runtime_c for rosidl C structures
* Ensure Java support library links against C generator target

Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
@jacobperron jacobperron requested review from esteve and ivanpauno June 22, 2021 23:16
jacobperron and others added 5 commits June 22, 2021 16:27
Opensplice is not distributed with ROS 2 since Foxy.

Signed-off-by: Jacob Perron <[email protected]>
It is not necessary and causes a circular dependency.

Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
Signed-off-by: Jacob Perron <[email protected]>
<test_depend>rosidl_generator_c</test_depend>
<!-- It's not clear why we need to depend on rosidl_generator_cpp, but without it
the tests will not compile.
See https://github.com/ros2/rosidl_python/pull/100#issuecomment-608558735 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

this is interesting ...

- uses: ros-tooling/setup-ros@v0.2
with:
required-ros-distributions: dashing
required-ros-distributions: galactic
Copy link
Contributor

Choose a reason for hiding this comment

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

this will introduce conflicts with changes in https://github.com/osrf/ros2_java/

Copy link
Contributor

Choose a reason for hiding this comment

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

this conflicts don't matter much, I'm ok with them

foreach(_typesupport_impl ${_typesupport_impls})
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Request.ep.${_typesupport_impl}.cpp")
list(APPEND _generated_extension_${_typesupport_impl}_files "${_output_path}/${_parent_folder}/${_idl_name}_Response.ep.${_typesupport_impl}.cpp")
list(APPEND _generated_extension_${_typesupport_impl}_files
Copy link
Contributor

Choose a reason for hiding this comment

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

this changes will introduce confilcts with osrf#73 if not imported from that commit

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer to import osrf#73 if possible, which doesn't seem to be the case.

Copy link
Contributor Author

@jacobperron jacobperron Jun 25, 2021

Choose a reason for hiding this comment

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

I tried cherry-picking osrf#73 but it doesn't apply cleanly. It seemed easier to directly address the cmake lint warnings here..

@jacobperron
Copy link
Contributor Author

I'm looking into the Android CI failure.

@mclayton7
Copy link

@jacobperron I started looking into the Android CI issue and it seems to be related to foonathan/memory#60. I'm not a ROS2/Ament expert, but it looks like a cross compilation issue that may be resolved by pointing cmake to qemu via CMAKE_CROSSCOMPILING_EMULATOR

@ivanpauno
Copy link
Contributor

@jacobperron I started looking into the Android CI issue and it seems to be related to foonathan/memory#60. I'm not a ROS2/Ament expert, but it looks like a cross compilation issue that may be resolved by pointing cmake to qemu via CMAKE_CROSSCOMPILING_EMULATOR

Thanks, let's see if 9a78180 fixes the issue.

@ivanpauno
Copy link
Contributor

Thanks, let's see if 9a78180 fixes the issue.

That seem to have solved the foonathan_memory_vendor compile error, now there's a different issue with rosidl_runtime_cpp.

@ivanpauno
Copy link
Contributor

Cloning into 'mimick-f171450b5ebaa3d2538c762a059dfc6ab7a01039'...
HEAD is now at f171450 Add armv7l as a 32-bit ARM architecture. (#16)
CMake Error at CMakeLists.txt:88 (message):
Architecture 'armv7-a' is not supported.

That one is unfortunate ....
IMO, we should make mimick tests optional, and disable them when mimick is not present.

@mclayton7
Copy link

Cloning into 'mimick-f171450b5ebaa3d2538c762a059dfc6ab7a01039'...
HEAD is now at f171450 Add armv7l as a 32-bit ARM architecture. (#16)
CMake Error at CMakeLists.txt:88 (message):
Architecture 'armv7-a' is not supported.

That one is unfortunate ....
IMO, we should make mimick tests optional, and disable them when mimick is not present.

It looks like https://github.com/ros2/Mimick supports ARM/ARM64, so maybe it's just a matter of tweaking the cmake scripts in https://github.com/ros2/mimick_vendor? I'll try to look at it this weekend.

@ivanpauno
Copy link
Contributor

It looks like https://github.com/ros2/Mimick supports ARM/ARM64, so maybe it's just a matter of tweaking the cmake scripts in https://github.com/ros2/mimick_vendor? I'll try to look at it this weekend.

Android uses a different ABI, so I'm not sure if it will work, probably it won't.
Mimick is a C mocking library that works by poisoning the GOT of a dynamic library at runtime, so it's pretty platform dependent.
IMO, it should be possible to disable all tests using mimick in the ros2 core by passing a cmake flag, I will see what I can do.

We can also just not build tests at all.... but that doesn't sound like a good idea.

@ivanpauno
Copy link
Contributor

I have opened a PR in rcl to skip mimick tests ros2/rcl#923.
We will need to do similar work in rcutils, rclcpp and in the other packages in the rcl repo, and that will likely fix the android builds.

@ivanpauno
Copy link
Contributor

The last five commits skipped building tests on the packages using mimick, so we are not blocked on PRs in the ros2 core to merge this.

Now this is failing in the rcutils shared library helpers ....
This is getting fun 😄

@ivanpauno ivanpauno closed this Nov 1, 2021
@ivanpauno ivanpauno reopened this Nov 1, 2021
@ivanpauno
Copy link
Contributor

@jacobperron CI is passing now.
The only job that failed is duplicated (push and pr triggers), and had a random issue.

@jacobperron
Copy link
Contributor Author

@ivanpauno Thanks for looking into it! I think we can merge this now. Maybe we can remove the duplicate jobs in a follow-up.

performance_test_fixture
ros2_tracing
libstatistics_collector
python_cmake_module
rpyutils
libyaml_vendor
rmw_dds_common

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
It's not clear why this dependency is required.

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@jacobperron
Copy link
Contributor Author

I've tried to cleanup the commits a little bit; I'll avoid squashing to preserve history.

@ivanpauno ivanpauno merged commit 960f896 into main Nov 3, 2021
@ivanpauno ivanpauno deleted the galactic_update branch November 3, 2021 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants