-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add alpine support #616
Add alpine support #616
Conversation
With ros-infrastructure/rospkg#148 and ros-infrastructure/rosdep#616, ros_core is ready to be built on Alpine Linux (edge version)
With ros-infrastructure/rospkg#148 and ros-infrastructure/rosdep#616, ros_core can be built from source on Alpine Linux (edge version)
With ros-infrastructure/rospkg#148 and ros-infrastructure/rosdep#616, All packages required by ros_core can be installed by rosdep on Alpine Linux (edge version)
With ros-infrastructure/rospkg#148 and ros-infrastructure/rosdep#616, all packages required by ros_core can be installed by rosdep on Alpine Linux (edge version)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had one quick comment on the copyright.
The other elements look reasonable. I haven't had a chance to test it on a live system.
src/rosdep2/platforms/alpine.py
Outdated
@@ -0,0 +1,90 @@ | |||
#!/usr/bin/env python | |||
# Copyright (c) 2018, rosdep authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'rosdep authors' is not a valid copyright entity. If you want to assign the copyright please use the foundation, Open Source Robotics Foundation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it to:
# Copyright (c) 2018, Open Source Robotics Foundation, Inc.
# Copyright (c) 2018, SEQSENSE, Inc.
Is it OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the copyright is to SEQSENSE, Inc don't add OSRF. We haven't setup a formal copyright assignment process. So I'd suggest just use the 2nd line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks for your suggestion.
test/test_rosdep_alpine.py
Outdated
@@ -0,0 +1,91 @@ | |||
# Copyright (c) 2018, rosdep authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
I have a couple of alpine systems, one physical, one an lxc container, that I can review this on. |
@Minipada if you could test this it would be great too. Thanks |
I just tested your container, everything is working well. Check one modified key is resolved (your patch here)
Check one can install from rosdep:
|
* Add minimal alpine rosdeps for ros_core With ros-infrastructure/rospkg#148 and ros-infrastructure/rosdep#616, all packages required by ros_core can be installed by rosdep on Alpine Linux (edge version) * Use apk gtest and src gmock * Use libressl instead of openssl since Alpine Linux recommends to use libressl instead of openssl (https://bugs.alpinelinux.org/issues/4970) * Fix to use python2-dev instead of python2
Latest rospkg with Alpine Linux detection is released. |
@at-wat It looks to be failing on OSX due to a test config error something must have moved in the osx packaging.
I know it's not your issue, but if you could look into it it would help us land this patch. |
Oh, I've forgotten to merge master into this. Thank you @nuclearsandwich |
src/rosdep2/platforms/alpine.py
Outdated
@@ -0,0 +1,90 @@ | |||
#!/usr/bin/env python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shebang is not needed here (#630)
Codecov Report
@@ Coverage Diff @@
## master #616 +/- ##
=========================================
+ Coverage 75.19% 75.49% +0.3%
=========================================
Files 30 31 +1
Lines 2890 2926 +36
=========================================
+ Hits 2173 2209 +36
Misses 717 717
Continue to review full report at Codecov.
|
ea2fcec
to
57f1e7b
Compare
same fix as ros-infrastructure#630
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, I don't have alpine to test on. Thanks for the mocked tests to make sure there's some coverage.
What's holding this up being merged? Let me know how I can help. Maybe it would be a good idea to get it merged after all the source dependencies are removed from rosdistro. I will try and work on upstreaming the remaining dependencies. |
There seems to be some conflicts between the pip installed PyYAML and the apk installed PyYAML. It causes the rosdep installing of dependencies to fail. If pip installs pyyaml before it is installed by apk then we get this failure.
Also, should we just opt for python3 by default in adding the alpine platform? |
In addition to this, the newest version of boost has removed the deprecated boost-signals library, so this fails on building roscpp. As seen in ros/ros_comm#1580 |
@russkel the problems around PyYAML and boost-signals are independent of this PR. @tfoote @nuclearsandwich Our company is already using ROS on Alpine Linux. |
Yep I understand this. I just wanted to document the issues one would have trying to bring up a ROS melodic setup with this (and I am trying to work on them so I can get melodic running in docker). Thanks for your repositories! Good to know you are having success with Alpine at your company. |
This PR adds support for Alpine Linux apk package manager.
It's currently not ready to be merged, since it requires latest (not yet released) rospkg with alpine detection.The change on rospkg has been merged and released.I have tested
rosdep install
with forked rosdistro with some alpine package definitions, in docker alpine 3.7. (https://github.com/at-wat/alpine-ros)