/snapcraft

ament plugin: new plugin #1583

Merged
merged 2 commits into from Nov 21, 2017

Conversation

Reviewers

@elopio elopio

@sergiusens sergiusens

@kalikiana kalikiana

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  2.36
4 participants
@kyrofa @kalikiana @sergiusens @elopio
@kyrofa
Member

kyrofa commented Oct 3, 2017 •
Edited 2 times

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
  • If this is a new feature. Have you discussed the design on the forum?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh unit?

As ROS2 nears a stable release, it's a good time to introduce support for it by way of a beta plugin. This PR resolves LP: #1686850 (and #1443) by doing exactly that. We're starting simple, just bootstrapping ROS2 and using it to build the provided source. ROS2 doesn't have a lot of tooling just yet, so this plugin will need to grow with it.

Note that the high line count is due to an integration test: the plugin itself is relatively small.

@kyrofa kyrofa referenced this pull request Oct 5, 2017

Closed

snapd integration tests: print stdout/stderr #1591

6 of 6 tasks complete

@kyrofa kyrofa deleted a comment from elopio Oct 19, 2017

@kyrofa kyrofa deleted a comment from elopio Oct 19, 2017

@kyrofa kyrofa deleted a comment from kalikiana Oct 19, 2017

@kalikiana

kalikiana reviewed Oct 20, 2017
View changes

Should the tests be series-specific? Just wondering since that's what we ended up needing for the catkin ones.

integration_tests/snaps/ros2/snap/snapcraft.yaml
+ environment:
+ # Required to get interleaved stdout, see
+ # https://discourse.ros.org/t/ros2-launch-not-interleaving-stdout
+ PYTHONUNBUFFERED: 1
@kalikiana

kalikiana Oct 20, 2017

Collaborator

How about setting this in the plugin's env so it doesn't have to be repeated in every snap?

@kyrofa

kyrofa Oct 20, 2017

Member

Because I currently believe it to be a hackaround as a result of using Python, whose support is new. It was not requires in ROS1, and I expect ROS2's launch will eventually take care of it as well. If not then we can revisit.

snapcraft/plugins/ament.py
+
+ def _source_setup_sh(self, root):
+ return textwrap.dedent('''
+ if [ -f {ros_setup} ]; then
@kalikiana

kalikiana Oct 20, 2017

Collaborator

What's the reason for the -f here? Can't you assume the setup.sh is already in place?

@kyrofa

kyrofa Oct 20, 2017

Member

Not necessarily. Remember that env() is used both to generate the wrappers in prime as well as when you call self.run() to build. While setup.sh will definitely be there by the end, it has to get there first.

@kalikiana

kalikiana Oct 23, 2017

Collaborator

Very true. I had a feeling I was missing something. Thanks for clarifying.

@kyrofa
Member

kyrofa commented Oct 20, 2017 •
Edited 1 time

Should the tests be series-specific? Just wondering since that's what we ended up needing for the catkin ones.

That's because each release of ROS1 supports a specific subset of Ubuntu releases. ROS2 isn't released yet. That said, right now the plugin is hard-coded to xenial, as I'm not sure if the ROS repository has other releases filled out with what we require for stage-packages. I'll check on that now. Until then, running the xenial:amd64 autopkgtest.

Scratch that. The repos won't work due to our key issue huh. I'll update the test to be xenial-only once I see if it passes or not.
@kalikiana
Collaborator

kalikiana commented Oct 23, 2017

Looks like zesty indeed fails with key errors: apt_pkg.Error: W:GPG error: http://packages.ros.org/ros/ubuntu xenial InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 5523BAEEB01FA116, E:The repository 'http://packages.ros.org/ros/ubuntu xenial InRelease' is not signed.
@kalikiana

kalikiana approved these changes Oct 23, 2017
View changes

Looks good to me then, with skipping the tests on non-Xenial. Nice work!

integration_tests/snapd/test_ament_snap.py
+ subprocess.check_output(
+ ['ros2-example.launch-project'],
+ universal_newlines=True, stderr=subprocess.STDOUT),
+ Contains("I heard: Hello world"))
@elopio

elopio Oct 24, 2017

Member

Hey, double quotes!!! :o

@kyrofa

kyrofa Oct 24, 2017

Member

What... I'm horrified at myself!

@kyrofa

kyrofa Oct 24, 2017

Member

Fixed.

@elopio

elopio approved these changes Oct 24, 2017
View changes

The plugin is very readable, I'm surprised. Thank you!

@kyrofa kyrofa referenced this pull request in canonical-docs/snappy-docs Oct 24, 2017

Merged

languages: add ROS2 guide #140

@kyrofa
Member

kyrofa commented Oct 25, 2017

@nuclearsandwich, @tfoote, @wjwwood here is the final piece of the puzzle regarding ROS2 support in Snapcraft. You already reviewed the most important part, that is, the bootstrapping of ROS2, but if you have some time I'd love for you to check this out as well.

The rubber meets the road (i.e. the ROS2 bootstrapper is actually used) in the Ament plugin. Some quick background information:

  • Plugins build their code by adhering to the plugin API. The important bits here are:
    • The pull function, where the source itself is pulled as well as the ROS2 underlay
    • The build function, where the ROS2 underlay is built, and then used to build the source. Note that everything we want to end up in the snap needs to be in self.installdir, but the ROS2 underlay is built elsewhere so we don't need to build it again upon failure. As a result, you'll see some workarounds there when it comes to prefixes. I'm curious if there are better ways to deal with this.

If you want to actually poke at this, check out this simple ROS2 snap guide, and use the snapcraft specific to this PR:

sudo snap install snapcraft --channel=edge/pr-1583 --classic

@sergiusens sergiusens added this to the 2.36 milestone Oct 26, 2017

@sergiusens
Collaborator

sergiusens commented Nov 19, 2017 

ERROR: test_ament_support (test_ament_snap.AmentTestCase)
test_ament_snap.AmentTestCase.test_ament_support

kyrofa added some commits Sep 14, 2017

@kyrofa
ament plugin: new plugin 
As ROS2 nears a stable release, it's a good time to introduce support
for it by way of a beta plugin. Start simple, just bootstrap ROS2 and
use it to build the provided source. ROS2 doesn't have a lot of tooling
yet, so this plugin will need to grow with it.

LP: #1686850
Resolve #1443

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits
fca23e5
@kyrofa
Quote launch path 
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
This commit was signed with a verified signature.
@kyrofa kyrofa Kyle Fazzari
GPG key ID: A7D23FB0E778DEDF Learn about signing commits
ea5eec3
@kyrofa
Member

kyrofa commented Nov 20, 2017 

ERROR: test_ament_support (test_ament_snap.AmentTestCase)
test_ament_snap.AmentTestCase.test_ament_support

Looks like this was caused by #1578, this was passing before the rebase. I need to quote the path, now.

@sergiusens sergiusens merged commit 23c22fe into snapcore:master Nov 21, 2017
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

elopio added a commit to elopio/snapcraft that referenced this pull request Nov 21, 2017

@kyrofa @elopio
ament plugin: new plugin (#1583) 
As ROS2 nears a stable release, it's a good time to introduce support
for it by way of a beta plugin. Start simple, just bootstrap ROS2 and
use it to build the provided source. ROS2 doesn't have a lot of tooling
yet, so this plugin will need to grow with it.

LP: #1686850
Fixes #1443

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
c75e3fa

@kyrofa kyrofa deleted the kyrofa:feature/1686850/ament_plugin branch Nov 21, 2017

matiasb added a commit to matiasb/snapcraft that referenced this pull request Nov 24, 2017

@kyrofa @matiasb
ament plugin: new plugin (#1583) 
As ROS2 nears a stable release, it's a good time to introduce support
for it by way of a beta plugin. Start simple, just bootstrap ROS2 and
use it to build the provided source. ROS2 doesn't have a lot of tooling
yet, so this plugin will need to grow with it.

LP: #1686850
Fixes #1443

Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
5fed0d3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment