Skip to content
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.

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

Crystal Patch Release 3 #656

Closed
nuclearsandwich opened this Issue Feb 15, 2019 · 27 comments

Comments

Projects
None yet
8 participants
@nuclearsandwich
Copy link
Member

nuclearsandwich commented Feb 15, 2019

This issue will track the list of changes that should be included in the next Crystal patch release.
The date for this patch release is likely 2019-03-14 with Bloom releases happening 2019-03-08 so that rebuilds can occur over the weekend and testing the following week.

Repos file aggregating backport branches against crystal development branches for CI. gist

Repositories ready for release

PRs which need a backport for this patch release:

Workaround for nested messages in actions. ros2/rosidl#348
Fix the include directory order of overlay workspaces.
Export the CMake logic to find ROS 1 packages through the ros1_bridge.
Actions support in Python
Add rcl_logging_log4cxx.
Remove make_notifier() call in flake8 to fix ROS 2 CI.
SROS2 XML and XSLT permissions transforms (and follow-on)
Fix a bug introduced in SROS2 artifact generation.
Fix linter invocation in SROS2.

Changes in progress for this patch release.

Proposed changes which have been dropped or deferred

Display the robot description correctly in rviz. Breaks API. Will not be released into Crystal.
Add idlpp option to maintian include file namespace. Not a release blocker. May be released in a future patch release.

All pull requests being released

Organized by repository and backport PR

ament/ament_cmake
ament/ament_lint
ros2/examples
ros2/rcl_interfaces
ros2/rcl_logging
  • ros2/rcl_logging#4
  • ros2/rclpy
    ros2/ros1_bridge
    ros2/rosidl_python
    ros2/rosidl_typesupport
    ros2/sros2
    ros2/system_tests
    @jacobperron

    This comment was marked as resolved.

    Copy link
    Member

    jacobperron commented Feb 15, 2019

    PRs for Python Actions in merge order:

    @dirk-thomas

    This comment was marked as resolved.

    Copy link
    Member

    dirk-thomas commented Feb 20, 2019

    PR fixing the include dir order of overlay workspaces: ament/ament_cmake#157

    @Karsten1987

    This comment was marked as resolved.

    Copy link

    Karsten1987 commented Feb 20, 2019

    @Karsten1987

    This comment was marked as resolved.

    Copy link

    Karsten1987 commented Feb 25, 2019

    display the robot description correctly in rviz: ros2/rviz#378

    @dirk-thomas

    This comment has been minimized.

    Copy link
    Member

    dirk-thomas commented Feb 28, 2019

    PR fixing naming collisions with OpenSplice: ros2/rosidl_typesupport_opensplice#20

    @Karsten1987

    This comment was marked as resolved.

    Copy link

    Karsten1987 commented Mar 4, 2019

    PR for ament_lint as discussed on the last patch release ticket:
    ament/ament_lint#124

    see discussion here: #647 (comment)

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 7, 2019

    PR fixing naming collisions with OpenSplice: ros2/rosidl_typesupport_opensplice#20

    This pull request has since been reverted and the re-application is still in progress. We won't wait for that PR to settle, if it makes this patch release that's fantastic. If not it can go into the next one after it's ready.

    @sloretz

    This comment has been minimized.

    @jacobperron

    This comment has been minimized.

    Copy link
    Member

    jacobperron commented Mar 8, 2019

    Additional backports needed for Python Action support (added to the list):

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    Updated the issue body with the open backport PRs. Rather than try and track the backports as both "in progress" work and backports of needed features from development branches I organized them all by what they backport. That leaves the In progress list for tasks which may still have development required before even a backport is ready.

    I've also put up a link to a gist I'll try to keep in sync with the current status of backport branches so we can use it to test incoming PRs against Crystal patch 3.

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    @sloretz Here's CI for the backport branches fixing ros2/rosidl#348 testing test_communication.

    • Linux Build Status
    • Linux-aarch64 Build Status
    • macOS Build Status
    • Windows Build Status
    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    The macOS failure looks like a bad interaction with Fast-RTPS and TinyXML2 caused by the infra machines using TinyXML2 7.x whereas Crystal requires TinyXML2 < 7.0.0

    @nuclearsandwich

    This comment was marked as outdated.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    @jacobperron here's CI for the rclpy Actions branches testing test_communication and rclpy.

    • Linux Build Status
    • Linux-aarch64 Build Status
    • macOS (crystal infra) Build Status
    • Windows Build Status
    @nuclearsandwich

    This comment was marked as resolved.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    FYI I just somehow managed to mangle a bunch of the recent edits in the issue body and GitHub doesn't appear to give me a way to undo that damage. Please refrain from making edits while I fix the mess.

    Edit: All good now... I'm pretty sure.

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 8, 2019

    My browser tab is cursed. Somehow destroyed all the edits again 😭 😭 😭

    @jacobperron

    This comment has been minimized.

    Copy link
    Member

    jacobperron commented Mar 9, 2019

    Backporting ament/ament_lint#122 should resolve the lint errors. We can either put it on top of ament/ament_lint#130 or wait for that to be merged and open a follow up PR.

    @jacobperron

    This comment was marked as outdated.

    Copy link
    Member

    jacobperron commented Mar 9, 2019

    CI for rclpy Actions, this time with backport ament/ament_lint#131:

    • Linux Build Status
    • Linux-aarch64 Build Status
    • macOS Build Status
    • Windows Build Status
    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 9, 2019

    Rebuild of #656 (comment) with the updated branch now that the rcl_logging backport has merged.

    • Linux Build Status
    • Linux-aarch64 Build Status
    • macOS (Crystal infra) Build Status
    • Windows Build Status
    @jacobperron

    This comment has been minimized.

    Copy link
    Member

    jacobperron commented Mar 13, 2019

    Backport for Python actions communication tests: ros2/system_tests#337

    @mjcarroll

    This comment has been minimized.

    Copy link
    Contributor

    mjcarroll commented Mar 13, 2019

    The backported changes in sros2 introduce a dependency on the rosdep key: python3-lxml

    The installation documentation has been updated in ros2/ros2_documentation#138, but I'll highlight the changes here:

    Depending on the platform, users may need to get the additional dependency:

    • With Ubuntu debs: apt will install suitable the suitable dependency
    • With Ubuntu binary releases: The rosdep tool should resolve the dependency (installs python3-lxml)
    • With macOS: The dependency should be resolved by installing lxml via pip until using rosdep is an option (see ros/rosdistro#20560 (comment))
    • With Windows: The dependency should be resolved by installing lxml via pip
    • With Windows (debug): The dependency should be resolved by installing a custom-built lxml via pip: https://github.com/ros2/ros2/releases/tag/lxml-archives
    @mikaelarguedas

    This comment has been minimized.

    Copy link
    Contributor

    mikaelarguedas commented Mar 13, 2019

    With macOS: The dependency should be resolved either with rosdep or by installing lxml via pip

    There are currently no rosdep macos rules for python3-lxml so it will not be resolved.

    Based on ros/rosdistro#20560 (comment) it looks like it will stay the case for now until we have a way to specify / test which pip is used to install a package via rosdep.
    The instructions linked above do mention to install it via pip so users following the instructions should not face any issue 👍

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 14, 2019

    Thanks for the edit @mikaelarguedas I've amended #656 (comment) which will go into the release notes imminently.

    @mjcarroll

    This comment has been minimized.

    Copy link
    Contributor

    mjcarroll commented Mar 14, 2019

    @mikaelarguedas You are correct, I mis-remembered the rosdep keys, I'll also make sure that the documentation is in sync with that.

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 14, 2019

    Packaging jobs

    • Linux Bionic Build Status
    • Linux Bionic ARM64 Build Status
    • Linux Xenial Build Status
    • Linux Xenial ARM64 Build Status
    • macOS Build Status
    • WindowsBuild Status
    • Windows Debug Build Status
    @dejanpan

    This comment has been minimized.

    Copy link

    dejanpan commented Mar 14, 2019

    @nuclearsandwich I am sorry for a late chime in but I think that these 2 bugs area serious:

    1. Race condition in rmw_fastrtps: ros2/rmw_fastrtps#258. We confirmed that the fix works: ros2/rmw_fastrtps#258 (comment).
    2. eProsima/Fast-RTPS#444 aka FastRTPS 1.7.2 release. What is concretely serious here is a memory leak reported here ros2/rmw_fastrtps#257 and fixed here eProsima/Fast-RTPS#434. Should including 1.7.2 not be acceptable by you since according to @wjwwood there are API breaking changes since 1.7.0, then I think at least a known issue must be clearly documented in ROS 2 Crystal Patch 3 Release:
      1. TRANSIENT_LOCAL QoS should not be used in current ROS 2 Crystal Patch 3 when RMW_IMPLEMENTATION=rmw_fastrtps_cpp
      2. ros2/rmw_fastrtps#257 (comment)
    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 14, 2019

    @dejanpan the window is definitely closed for new changes in patch release 3. Since the Fast-RTPS issues have been present in Crystal since its release rather than recent regressions, I am opting not to delay this patch release and instead we can target these fixes for the next one (#668).

    Open Robotics had a brief discussion about whether or not the improvements in Fast-RTPS 1.7.1/1.7.2 are worth allowing breaking changes and I expect that discussion to continue in #668.

    In the meantime, documenting these issues is a important, although I think adding them to the documentation for Crystal makes more sense than putting them in the release notes since they're not new, just newly reported. I will definitely add a remark re-linking the Crystal release page and known issues in the release announcement.

    @nuclearsandwich

    This comment has been minimized.

    Copy link
    Member Author

    nuclearsandwich commented Mar 15, 2019

    The patch release has shipped! ⛵️

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    You can’t perform that action at this time.
    You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.