Be My Ardent-tine #135
Be My Ardent-tine #135
Conversation
|
Next up is to add support for things like python (for using distutils instead of CMake), as, currently, ROS2 ebuilds fail due to a lack of Also, there's complaints about a missing opensplice dependency, so nothing is buildable past the type support. |
There was a problem hiding this comment.
Yeah, with ROS2 we support different build types instead of wrapping it all into CMake. That will be a little more complicated. There's also some special logic for the environment setup at the moment with a dependency injection in bloom.
I'm not entirely sure about how much sense it makes to hard code the ros2/rosdistro path. In the long term it will likely be merged back and maintained in parallel. And the tool would be more flexible if it just read the environment variable. Like: https://github.com/ros2/ros2/wiki/Releasing-a-ROS-2-package-with-bloom Using the environment variable also support releasing from forks much easier which is one of the values of superflore, that you can build a fully customized rosdistro based on a fork.
| @@ -40,6 +40,7 @@ def commit_changes(self, distro): | |||
| 'lunar': 'regenerate ros-lunar, ', | |||
| 'indigo': 'regenerate ros-indigo, ', | |||
| 'kinetic': 'regenerate ros-kinetic, ', | |||
| 'ardent': 'regenerage ros2-ardent, ', | |||
…ation, as well as parse the build_type tag from the export line of the package.xml.
allenh1
force-pushed
the
add-ros2-generation
branch
from
cbc07d3
to
f28a73d
Compare
It would be a big change, and maybe not worth making at the moment, but adding ebuild generation to bloom and then using it in superflore might make sense long term. Since 0.6.0 bloom uses different templates for generating different package build types. Even if it doesn't make sense to try and incorporate bloom now, you can still see how the debian packages are constructed. https://github.com/ros-infrastructure/bloom/tree/master/bloom/generators/debian/templates |
|
Ok, I'm a little upset about these random failures on Travis. Under "pr", the CI passes for pypy3, and for "push" it fails only for python 3.5. And for string orderings? I don't get it. Must be some |
| ret += "ROS_DISTRO=\"{0}\"\n".format(self.distro) | ||
| ret += "ROS_PREFIX=\"opt/ros/${ROS_DISTRO}\"\n" | ||
|
|
||
| # TODO(allenh1): This is gross. No. |
There was a problem hiding this comment.
I'm regularly guilty of this as well, but instead of saying "This is gross." A more useful XXX or TODO comment might give an example of when the code will fail
There was a problem hiding this comment.
It's just there so I fix it before this gets merged (I will not merge that into the codebase -- that's just awful).
But, in the future, that's a good point.
There was a problem hiding this comment.
Actually, on second thought, I really don't think this is as awful as I initially thought it was.
The more robust solution is to create an eclass for this, but that's not really in the spirit of things if it's just for unpacking things. So I think I'll just leave a note that says "if you need to update this, then do so. Otherwise, this is sufficient."
|
I think this just about does it for this PR. I ended up adding in a template tl;dr The remaining fixes will be done to the eclasses, so this can be merged. Next step there is getting @nuclearsandwich's Edit: I also need to do a PR to figure out how to fix some of this test jitter. Maybe that's just allowing some tests to try again, but I'm rather annoyed with the current status of the CI. |
| from superflore.utils import save_pr | ||
| from superflore.utils import url_to_repo_org | ||
| from superflore.utils import warn | ||
|
|
||
| # Modify if a new distro is added | ||
| active_distros = ['indigo', 'kinetic', 'lunar'] | ||
| # TODO(allenh1): It would be super nice make this a configuration option. |
There was a problem hiding this comment.
👍 ROSDISTRO_INDEX_URL
Just use rosdistro.get_index_url() https://github.com/ros-infrastructure/rosdistro/blob/master/src/rosdistro/__init__.py#L68
There was a problem hiding this comment.
I figured there was going to be something like that. I'll look into this in a follow up. Thanks.
* Add ardent, and change the ROSDISTRO_INDEX_URL to point to the ros2 version. * Index ROS1 and ROS2. * Separate (for more modularity) several parts of the ebuild text generation, as well as parse the build_type tag from the export line of the package.xml. * Fix syntax, centralize the ros2_distro list. * Extract tag with a regex, because the other way is disgustingly hackey, and lint. * Rename class to reflect overlay. * Begin working on ROS2 patches. * Fix build_type detection logic. * Add mapping for uncrustify license. * Added the eclass. * Add cmake build_type mapping. * Add LGPL-2.1 test detection test (and fix). * Change distros to file in sorted order so as to make testing more consistent.
This is a work in progress still! I'm not super pleased with the quality of this patch.
But, in its current state, this does add ROS 2 support, which is pretty neat.