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

Migrate CI to bionic #148

Merged
merged 34 commits into from
Jun 7, 2018
Merged

Migrate CI to bionic #148

merged 34 commits into from
Jun 7, 2018

Conversation

mikaelarguedas
Copy link
Member

@mikaelarguedas mikaelarguedas commented Apr 16, 2018

start migrating Linux CI to bionic

connects to ros2/ros2#481

@mikaelarguedas mikaelarguedas added the in progress Actively being worked on (Kanban column) label Apr 16, 2018
@mikaelarguedas mikaelarguedas mentioned this pull request Apr 16, 2018
31 tasks
@dirk-thomas
Copy link
Member

I would suggest to split the bullets into two groups:

  • what needs to happen to get this merged and use Bionic in the CI jobs
  • what should happen afterwards

@mikaelarguedas
Copy link
Member Author

I would suggest to split the bullets into two groups:

what needs to happen to get this merged and use Bionic in the CI jobs
what should happen afterwards

Agreed, though it depends what we consider "enough" to merge this on CI.
If we want all our nightly to build on bionic without warnings we will need almost all of them to be addressed.

I see several stages:

  • Being able to run our regular ci_linux
  • Being able to run all the nightlies (including tb2 demo jobs)
  • Being able to run all ci jobs including the ones using opensplice.
  • Audit dependencies and previous hacks

I think we need at least the first 2 to be done before merging this on CI. Do you agree?

@dirk-thomas
Copy link
Member

I think we need at least the first 2 to be done before merging this on CI. Do you agree?

Yes, except that I don't see the tb2 demo as a blocker - these jobs could continue to use Xenial until kobuki has been released.

@nuclearsandwich
Copy link
Member

nuclearsandwich commented Apr 16, 2018


Edit: moving this question to https://github.com/ros2/ros2/issues/481

@mikaelarguedas
Copy link
Member Author

Status update:
This branch with connected PRs gives green CI for both opensplice and connext including cross_vendor tests between the 2.

Fast-RTPS still segfaults on any operation involving setting parameters or security

@mikaelarguedas
Copy link
Member Author

The todos have been moved to ros2/ros2#481

@mikaelarguedas mikaelarguedas changed the title [WIP] migrate to bionic Migrate CI to bionic May 4, 2018
@mikaelarguedas
Copy link
Member Author

All bionic fixup PR's have been merged

  • Linux job Build Status

  • Packaging amd64 Build Status

  • Coverage: Build Status

  • Linux Debug Build Status

  • Linux Release Build Status

aarch jobs have xenial hardcoded, set of job using upstream arm64 bionic docker images:

  • ci aarch64: Build Status
  • packaging aarch64: Build Status

Newly unveiled issue: there's a new snprintf warning that shows up only when building with RelWithDebInfo (aka packaging job).
All the other jobs are yellow because of false positive CMake warnings.

@mikaelarguedas
Copy link
Member Author

New round of packaging job including the warning fixups from ros2/system_tests#269 and ros2/rosidl#275

  • Packaging amd64 Build Status
  • Packaging Windows: Build Status
  • Packaging MacOS: Build Status

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented May 8, 2018

Squashed and rebased all commits.

Ready for review

Last round of CI to confirm all is well:

  • Linux Build Status

  • Linux-aarch64 Build Status (custom job)

  • macOS Build Status

  • Windows Build Status

  • Linux Debug Build Status

  • Linux Release Build Status

  • Coverage: Build Status (custom job)

  • Packaging aarch64: Build Status (custom job)

  • Packaging amd64: Build Status (custom job)

@mikaelarguedas mikaelarguedas added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels May 8, 2018
Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

The patch looks good to me 👍

@mikaelarguedas
Copy link
Member Author

Looking a bit more into it we were not using custom arm images but the images from that project: https://hub.docker.com/r/aarch64/ubuntu/

Now that the official docker library/ubuntu hosts armhf and arm64 images I kept my change as is to use the upstream docker images.
If the need to provide CI that can run with qemu comes up we move to use our custom images. In the meantime we'll keep upstream to avoid having custom "magic"

@nuclearsandwich
Copy link
Member

Now that the official docker library/ubuntu hosts armhf and arm64 images I kept my change as is to use the upstream docker images.
If the need to provide CI that can run with qemu comes up we move to use our custom images. In the meantime we'll keep upstream to avoid having custom "magic"

Makes complete sense to me. If there's no qemu support to preserve then there's no reason not to go straight to upstream images.

Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

One non-blocking question inline. Everything else looks good. Thanks for all the time invested here! 🙇‍♂️ 🙇‍♂️ 🙇‍♂️

# We are building poco from source for now as we need at least 1.4.1p1 and xenial ships with 1.3.6p1 (https://github.com/ros2/poco_vendor/pull/10)
# RUN apt-get update && apt-get install --no-install-recommends -y libpoco-dev libpocofoundation9v5 libpocofoundation9v5-dbg
# Install build dependencies for class_loader.
# TODO(mikaelarguedas) Uncomment this when we decide what to do about Poco debug builds
Copy link
Member

Choose a reason for hiding this comment

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

Are we cool to merge with this TODO still pending?

Copy link
Member Author

Choose a reason for hiding this comment

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

TL;DR: IMO yes, this doesnt change the current behavior ATM.

Poco doesn't ship debs with debug libraries anymore (Bionic or Brew) and the release libraries will be found even in Debug builds.
We should be able to just use system [poco on Ubuntu] (https://ci.ros2.org/job/ci_packaging_linux/91/) and MacOS and build in Debug on top without facing ros2/poco_vendor#11.
@sloretz is currently modifying his poco_vendor PR to build only in Release and not overriding the Poco CMake module, my hope is that this should work on windows and allow us to build poco, only on Windows and only in Release that should save use some CI time.

@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Jun 7, 2018

New round of CI using the last state of this branch and the newly generated jobs with the ROS1DISTRo selector:

Copy link
Member

@dhood dhood left a comment

Choose a reason for hiding this comment

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

lgtm, not sure if you were looking for "fresh" eyes on it though

@mikaelarguedas
Copy link
Member Author

not sure if you were looking for "fresh" eyes on it though

Not necessarily :) as we worked on different parts of the PR I think that's fine 👍

It feels a bit weird to have the CI_UBUNTU_DISTRO option now displayed on all jobs including the Macos/Windows ones but I think that's the price to pay for having a single template for all of them.

I'm planning on merging this as is in ~1hr.

If you happen to want to review and this is already merged, please feel free to comment here and I'll address comments post merge in a follow-up.

@mikaelarguedas
Copy link
Member Author

And some jobs for the turtlebot demo:

  • tb2 amd64: Build Status
  • tb2 aarch64: Build Status

@mikaelarguedas mikaelarguedas merged commit b0bf985 into master Jun 7, 2018
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Jun 7, 2018
@mikaelarguedas mikaelarguedas deleted the bionic branch June 7, 2018 22:36
@mikaelarguedas
Copy link
Member Author

mikaelarguedas commented Jun 8, 2018

Nightly turtlebot jobs failed due to an outstanding PR that wasn't merged. This is now fixed

  • tb2 amd64: Build Status
  • tb2 aarch64: Build Status

minggangw pushed a commit to minggangw/rclnodejs-1 that referenced this pull request Jun 14, 2018
As the ros2 ci build farm has switched the ubuntu from Xenial to
Bionic(ros2/ci#148), we have to change the url
of the package url in order to get the right one.

Fix NONE
minggangw pushed a commit to minggangw/rclnodejs-1 that referenced this pull request Jun 14, 2018
As the ros2 ci build farm has switched the ubuntu from Xenial to
Bionic(ros2/ci#148), we have to change the url
of the package url in order to get the right one.

Fix NONE
minggangw pushed a commit to minggangw/rclnodejs-1 that referenced this pull request Jun 14, 2018
As the ros2 ci build farm has switched the ubuntu from Xenial to
Bionic(ros2/ci#148), we have to change the url
of the package url in order to get the right one.

Fix NONE
minggangw pushed a commit to RobotWebTools/rclnodejs that referenced this pull request Jun 14, 2018
As the ros2 ci build farm has switched the ubuntu from Xenial to
Bionic(ros2/ci#148), we have to change the url
of the package url in order to get the right one.

Fix NONE
minggangw pushed a commit to RobotWebTools/rclnodejs that referenced this pull request Jun 29, 2018
As the ros2 ci build farm has switched the ubuntu from Xenial to
Bionic(ros2/ci#148), we have to change the url
of the package url in order to get the right one.

Fix NONE
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.

5 participants