Skip to content

Add current workspace to the rosdep install command#1115

Merged
ruffsl merged 1 commit intomasterfrom
unknown repository
Sep 13, 2019
Merged

Add current workspace to the rosdep install command#1115
ruffsl merged 1 commit intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Sep 13, 2019

Basic Info

Info Please fill out this column
Ticket(s) this addresses This PR addresses the issue noted here #1107 (comment)
Primary OS tested on
Robotic platform tested on

Description of contribution in a few bullet points

  • CircleCI is currently only running rosdep on the underlay(navstack dependencies) workspace, and not the nav2 workspace. This PR adds the current workspace to the rosdep command.
  • When the CircleCI install_dependencies command is run on the underlay, the two paths will collapse to the same directory and should be harmless but redundant. When the install_dependencies command is run on the overlay (nav2) workspace, then underlay and overlay will be scanned for dependencies as desired.
  • Once Extend workspace packages using ament index for ROS2 support ros-infrastructure/rosdep#699 is released, the $UNDERLAY_WS/src component can be removed and the redundancy can be eliminated.

@ghost ghost requested a review from ruffsl September 13, 2019 18:48
@ruffsl
Copy link
Member

ruffsl commented Sep 13, 2019

Yea, this must have been a typo from when I refactored the config to use functions.

When the CircleCI install_dependencies command is run on the underlay, the two paths will collapse to the same directory and should be harmless but redundant.

Just note that changes to package.xml manifest in nav2 packages that require new dependencies will then break the cache for the underlay, given the checksum of installed packages would change. But I don't think that breaking the cache restore should impact CI much after the initial PR builds.

Yep, exactly! This will help support arbitrary workspace layering, and shore up cache restores.

@ruffsl ruffsl merged commit 32d36f8 into ros-navigation:master Sep 13, 2019
@ruffsl
Copy link
Member

ruffsl commented Sep 13, 2019

Just note that changes to package.xml manifest in nav2 packages that require new dependencies will then break the cache for the underlay

Nevermind, forgot that src was a relative path in the same PWD.

ghost pushed a commit that referenced this pull request Oct 4, 2019
* Remove COLCON_IGNORE

* Revert branch typo

* Merge pull request #1115 from crdelsey/circleci-rosdep

Add current workspace to the rosdep install command

* Cache per packge.xml via multistage

* Include COLCON_IGNORE in cache

* Colcon mixin now installed from base image

* Revert FROM tag change

* Comment new directives

* Update labels

* Cache both underlay and overlay layers

* Fix underlay parameter for install_overlay_dependencies

Otherwise the checksum.txt for the overlay is improperly copied from `/opt/ros_ws` instead of `/opt/underlay_ws`.

* Include complete path of checksum.txt in checksum

To make the checksum output easier to read, trace and debug.

* Simplify checksum commands for consistency

* Add test for rmw_cyclonedds_cpp to nightly job

* Use ament index to resolve rosdep install

* Update design for providing transform to bt nodes (#1157)

* Making a transform (buffer) available on the behavior tree
blackboard for nodes to pull and use.
Updating `GoalReachedCondition` to use the blackboard provided tf.

* Removed unnecessary include

Changing to non-lifecycle node for tf listener.

The lifecycle interface doesn't exist in dashing.

* Always build underlay as release

* Instead of copying icons, reference them in origin package. (#1188)

* Instead of copying icons, reference them in origin package.

* Remove icon install step as well.
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.

1 participant