search

ros2 / rosbag2_bag_v2

rosbag2 plugin for replaying ros1 version2 bag files
Apache License 2.0
24 stars 4 forks source link

Prepare 0.2.0 #44

Closed Karsten1987 closed 3 years ago

Karsten1987 commented 3 years ago

same as been done for rosbag2.

Bumping 0.2.0 to master, cutting a rolling release and a galactic release

Karsten1987 commented 3 years ago

It compiles correctly over here on my side, but I haven't it in detail. Are you running into hiccups?

emersonknapp commented 3 years ago

No actual issues - I just hadn't done this build in a while so it took me a sec to get it set up correctly. I also verify that it is building and tests pass.

How do we run CI again? With the following?

EDIT: oh - but this wasn't a source change - those builds were already running this nightly. Forget that portion of the comment :)

emersonknapp commented 3 years ago

Are you already doing the followups, or would you like me to?

Karsten1987 commented 3 years ago

The tag 0.2 exists already

EDIT: The ros2/ros2.repos don't have to be updated as these packages here are not part of it.

Karsten1987 commented 3 years ago

How do we run CI again? With the following?

We are currently not testing them in any nightlies. If we wanted to test them manually, only the linux packaging jobs make sense as we don't have a ROS1 installation on all other platforms.

emersonknapp commented 3 years ago

The tag 0.2 exists already

Unfortunately that doesn't work right when we follow this process - if it was automatically created by catkin_prepare_release - since it refers to a commit that no longer exists, since this PR was squashed.

https://github.com/ros2/rosbag2_bag_v2/releases/tag/0.2.0 points to 36d497f but this PR squash-merged to 676475d

emersonknapp commented 3 years ago

I've force-pushed the correct 0.2.0, and pushed the new galactic branch.

The ros2/ros2.repos don't have to be updated as these packages here are not part of it.

Of course, you're right. I was running on autopilot.

Karsten1987 commented 3 years ago

Ah, I knew this process would throw me off. Sorry for that. I guess at this point I actually don't know what to do. It looks like you're more familiar with the process of preparing a release on a separate branch. Do you mind following up here?

emersonknapp commented 3 years ago

If we wanted to test them manually, only the linux packaging jobs make sense as we don't have a ROS1 installation on all other platforms.

Again, of course right haha. I forget that this is tied to ROS1 and so only targets Ubuntu.

emersonknapp commented 3 years ago

Ah, I knew this process would throw me off. Sorry for that. I guess at this point I actually don't know what to do. It looks like you're more familiar with the process of preparing a release on a separate branch. Do you mind following up here?

All done. I'll run the bloom releases now too

emersonknapp commented 3 years ago

I should probably write up a quick "ros-tooling release runbook" - the default bloom processes assume you just push commits to master, but I do like having branch protections and putting all changes through PRs with squash-merge. I'd personally prefer to avoid leaving that open just for the case of releases.

Karsten1987 commented 3 years ago

since it refers to a commit that no longer exists, since this PR was squashed.

Am I supposed to merge the commit here - opposed to "squash and merge" or what's the recommended way?

emersonknapp commented 3 years ago

I like squash-merge and keeping the version-bump together with the changelist update. The way I do it is to say "no" to the part that asks if it should push the tag, then to manually tag after merging the PR

emersonknapp commented 3 years ago

It's not 100% ideal but it allows us to keep the general branch configuration on, without exception. Open to alternative proposals of course

emersonknapp commented 3 years ago

Rolling https://github.com/ros/rosdistro/pull/29567

emersonknapp commented 3 years ago

Galactic https://github.com/ros/rosdistro/pull/29568

Karsten1987 commented 3 years ago

@emersonknapp Ima hijack this PR for a second, but can you confirm two things for me please?

I've just noticed that I can't get the tests to run (independently whether they pass or fail) due to linkage errors with class loader when my ros2 installation is from source with an --isolated build. It works only in a --merge-install workspace, really.

Once the tests are running, I see a reliable test failure in the play end-to-end test - which makes also sense as the Player exits with a return code other than zero when the RMW interface is not found. So I had to change line 109 to EXPECT_THAT(exit_code, Eq(EXIT_FAILURE));.

emersonknapp commented 3 years ago

the rosbag2_bag_v2_plugins compile and run (!) on an isolated build?

I had also done colcon build --packages-up-to ros1_bridge (without --merge-install) - and was able to build against it in an overlay containing rosbag2_bag_v2_plugins - but the tests do not run successfully.

1: [ RUN      ] RosbagV2StorageTestFixture.get_all_topics_and_types_returns_list_of_recorded_bag_file                                                                                                                                                                            
1: /bag_ws/build/rosbag2_bag_v2_plugins/test_rosbag_v2_storage: symbol lookup error: /bag_ws/install/lib/librosbag_default_encryption_plugins.so: undefined symbol: _ZN12class_loader4impl22AbstractMetaObjectBaseC2ERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES9_   
1: -- run_test.py: return code 127

I am now trying the underlay with colcon build --packages-up-to ros1_bridge --merge-install - to see if this is more successful

emersonknapp commented 3 years ago

Process followed just now:

cd /ros2_ws
colcon build --symlink-install --merge-install --packages-up-to ros1_bridge rosbag2 --packages-skip ros1_bridge
# finishes 157 packages
source /opt/ros/noetic/setup.bash
colcon build --symlink-install --merge-install --packages-select ros1_bridge --cmake-force-configure 
# finishes 1 package (slow package! 10 mins)

# new shell
cd /bag_v2_ws
source /opt/ros/noetic/setup.bash
source /ros2_ws/install/setup.bash
colcon build
# 2 packages succeed
colcon test --merge-install
# one test fails

3 - test_rosbag2_play_rosbag_v2_end_to_end (Failed)
3: [ RUN      ] PlayEndToEndTestFixture.play_fails_gracefully_if_rosbag_v2_storage_id_is_not_specified
3: /bag_ws/src/rosbag2_bag_v2/rosbag2_bag_v2_plugins/test/rosbag2_bag_v2_plugins/test_rosbag2_play_rosbag_v2_end_to_end.cpp:109: Failure
3: Value of: exit_code
3: Expected: is equal to 0
3:   Actual: 1 (of type int)
3: [  FAILED  ] PlayEndToEndTestFixture.play_fails_gracefully_if_rosbag_v2_storage_id_is_not_specified (846 ms)
Karsten1987 commented 3 years ago

Ok, that matches what I see here as well - in both cases actually.

My take away from this is that the ros1 plugins can't be used correctly when build in isolation. Additionally, we should change line 109 to expect a failure for the test to pass correctly.

Karsten1987 commented 3 years ago

https://github.com/ros2/rosbag2_bag_v2/pull/45

emersonknapp commented 3 years ago

My take away from this is that the ros1 plugins can't be used correctly when build in isolation.

Does this apply to the ros1_bridge in general? The README there does not suggest building with --merge-install