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

Revert CI image to build from osrf/ros2:testing #2392

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Conversation

ruffsl
Copy link
Member

@ruffsl ruffsl commented Jun 7, 2021

Reverts parts of: #2348
Depends on: osrf/docker_images#542

@ruffsl ruffsl marked this pull request as ready for review June 8, 2021 19:58
@ruffsl
Copy link
Member Author

ruffsl commented Jun 8, 2021

@SteveMacenski could we wait on #2397 until it's need for it propigates to rolling-testing?
I manually updated the CI image to test this PR to check nav2 builds before merging.

@ruffsl
Copy link
Member Author

ruffsl commented Jun 8, 2021

Builds are passing now when building from rolling-testing repo!

@ruffsl ruffsl merged commit 083f321 into main Jun 8, 2021
@ruffsl ruffsl deleted the ci-rolling-testing branch June 8, 2021 20:51
@ruffsl
Copy link
Member Author

ruffsl commented Jun 8, 2021

No longer having to build a huge underlay and use rolling-testing really cut down on the image build time, wow:

image

@SteveMacenski
Copy link
Member

Awesome!!

@wilcobonestroo
Copy link
Contributor

All repos in the tools/underlay.repos file are commented now. If you follow the steps to build the Nav2 Dependencies, you don't get the dependecies when you do the vcs import. Should there be a separate file for CI and one for manually building? Anyway, the steps described at the Build and Install page should be updated.

@ruffsl
Copy link
Member Author

ruffsl commented Jun 12, 2021

We've been gradually migrating our CI and development to build from rolling testing as conveyed in the root level Dockerfile. This avoids the need to build a large amount of packages within your underlay, and can save us all a lot of build time. The underlay repo file is still there so if people do need to submit PRs that built against custom versions of dependencies in transit, they still have a way to configure the CI to do that. I've only commented out the repos so folks can easily find and uncomment dependencies that are outside of the core ros2 packages.

If you find where the documentation is out of sync, PRs are welcome. Feel free to ping me for review.

@wilcobonestroo
Copy link
Contributor

I am not sure if I completely understand it right at the moment. I think you are referring to the CI and I was referring to the description how to build Nav2 on your local machine. The steps to manually build the Nav2 Dependencies describe the following commands to perform:

source ros2_ws/install/setup.bash
mkdir -p ~/nav2_depend_ws/src
cd ~/nav2_depend_ws
wget https://raw.githubusercontent.com/ros-planning/navigation2/main/tools/underlay.repos
vcs import src < underlay.repos
rosdep install -y -r -q --from-paths src --ignore-src --rosdistro <ros2-distro>
colcon build --symlink-install --cmake-args -DCMAKE_BUILD_TYPE=Release

If you follow these steps, you don't get the repos at the vcs import command, unless you uncomment the repos in the file.

Is the solution to add a description that you should look into the underlay.repos file and uncomment the lines that you need? As far as I understand, I needed all lines to be able to build Nav 2. Is that correct? Or is there another way for me to get the rolling dependencies on my development machine?

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.

3 participants