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

Include underlay source folder in rosdep path #2025

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Conversation

ruffsl
Copy link
Member

@ruffsl ruffsl commented Oct 6, 2020

As OMPL isn't yet registering properly with the ament index for ros2
and searching all of << parameters.underlay >>/install will stall rosdep
when searching entire $ROS_WS/install folder

Related: #2021 (comment)

As OMPL isn't yet registering properly with the ament index for ros2
and searching all of << parameters.underlay >>/install will stall rosdep
when searching entire $ROS_WS/install folder
@ruffsl ruffsl requested a review from SteveMacenski October 6, 2020 23:54
@ruffsl ruffsl mentioned this pull request Oct 6, 2020
5 tasks
dependencies=$(
rosdep install -q -y \
--from-paths src \
$underlay_ws \
Copy link
Member

Choose a reason for hiding this comment

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

To make sure I understand this, this is saying if we're in the overlay workspace, include the underlay workspace in the rosdep install command? Is that because when it runs with both sources, it combines them and views this as within the same list?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. The --from-paths is to specify a list of paths to recursively search for package.xml files. With pwd being the overlay workspace, we just add the absolute path to the underlay source folder to ensure we find any packages that are missing from the ament index we sourced:

https://github.com/ros-planning/navigation2/blob/5d3c7e33d2a739acd5eb4ac7e5e4253aa643c96c/.circleci/config.yml#L56

This patch isn't very formulaic, as it can't really recursively scale, e.g. if a package fails to install itself correctly in your underlay's underlay, your SOL; vs. ament index that recursively daisy chains for us.

Recursively searching paths in python is also slow, and seems to stall on the ros install folder.

@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

❗ No coverage uploaded for pull request base (main@8f971aa). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2025   +/-   ##
=======================================
  Coverage        ?   85.01%           
=======================================
  Files           ?      294           
  Lines           ?    15011           
  Branches        ?        0           
=======================================
  Hits            ?    12762           
  Misses          ?     2249           
  Partials        ?        0           
Flag Coverage Δ
#project 85.01% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f971aa...5d3c7e3. Read the comment docs.

@SteveMacenski SteveMacenski merged commit 13c878b into main Oct 7, 2020
@SteveMacenski SteveMacenski deleted the ci-rosdep-patch branch October 12, 2020 19:28
ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Apr 16, 2021
ruffsl added a commit that referenced this pull request Apr 16, 2021
ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Jun 6, 2021
now that ompl issue is resolved
ompl/ompl#753
ruffsl added a commit that referenced this pull request Jun 7, 2021
now that ompl issue is resolved
ompl/ompl#753
ruffsl added a commit that referenced this pull request Jun 7, 2021
now that ompl issue is resolved
ompl/ompl#753
ruffsl added a commit that referenced this pull request Jun 8, 2021
now that ompl issue is resolved
ompl/ompl#753
ruffsl added a commit that referenced this pull request Jun 8, 2021
* Revert to testing image for CI

* Use ompl from testing repo
and modify Dockerfile to account for empty underlay

* Clean up RUN directive for caching

* Revert #2025
now that ompl issue is resolved
ompl/ompl#753

* Continue checkout if ccache doesn't yet exist
ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Jul 2, 2021
As OMPL isn't yet registering properly with the ament index for ros2
and searching all of << parameters.underlay >>/install will stall rosdep
when searching entire $ROS_WS/install folder
ruffsl added a commit to ruffsl/navigation2 that referenced this pull request Jul 2, 2021
* Revert to testing image for CI

* Use ompl from testing repo
and modify Dockerfile to account for empty underlay

* Clean up RUN directive for caching

* Revert ros-navigation#2025
now that ompl issue is resolved
ompl/ompl#753

* Continue checkout if ccache doesn't yet exist
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.

2 participants