ros / ros-overlay

Gentoo Overlay for ROS packages
33 stars 30 forks source link

ros-lunar/mavros package missing (required by ros-lunar/mavros_extras) #229

Open gemarcano opened 7 years ago

gemarcano commented 7 years ago

ros-lunar/mavros is not in the overlay. There is one for indigo, but not for lunar. Is this an oversight? I'll try to modify the indigo ebuild to install it for lunar.

allenh1 commented 7 years ago

@gemarcano this is likely due to a missing dependency. Ebuilds for this overlay are generated using the python package superflore. To get this going, you should try and find out which package is missing.

Currently, I'm adding a --only flag to the package so that users of the overlay can generate just one of the packages and get their PR submitted. See ros-infrastructure/superflore#76.

You would run as follows:

superflore-gen-ebuilds --rosdistro lunar --only mavros
gemarcano commented 7 years ago

Yeah, seems to be missing dependencies, specifically geographiclib and geographiclib-tools. I'm going to see what I can do about those...

gemarcano commented 7 years ago

Ugh, this is annoying. geographiclib installs its python files to ${PREFIX}/lib/python unconditionally, with no regards for python2 or python3 support. I have an ebuild that can install geographiclib, but it doesn't (can't?) deal with the python mess at the moment.

EDIT: It also has automagic dependencies :( . At least it's documentation related and not actual code, but still... can't say I'm impressed.

allenh1 commented 7 years ago

Ugh, this is annoying. geographiclib installs its python files to ${PREFIX}/lib/python unconditionally, with no regards for python2 or python3 support. I have an ebuild that can install geographiclib, but it doesn't (can't?) deal with the python mess at the moment.

That's never fun. I have definitely been there. I'll take a look at this once I have a few, but please feel free to file a PR (even in a broken state) so I can get a feel as to what you're trying/how you're doing this.

It also has automagic dependencies :( . At least it's documentation related and not actual code, but still... can't say I'm impressed.

Those are also pretty great.

@Alessandro-Barbieri do you have any advice for us here?

allenh1 commented 7 years ago

@gemarcano Any progress here?

Alessandro-Barbieri commented 7 years ago

geographiclib is only available in the pchrist overlay an entry was added with https://github.com/ros/rosdistro/commit/3c9f482ea6b5df642ef40d2b1aaa1e08f06e0588 should we remove that entry?

gemarcano commented 7 years ago

Sorry, I haven't had time to work on the ebuild. I may have some Friday. I could take a look at the ebuild in the pchrist overlay to see if it's a good starting point as well.

gemarcano commented 7 years ago

Looking at the ebuild from the pchrist overlay, it's old (using EAPI=3). Version number is also old, 1.11-r4 vs. latest 1.49. It also doesn't offer as much flexibility as it could (it was trivial to add some cmake enable/disable controls via useflags, namely static-libs and doc). I'm cleaning up my ebuild a little bit. I hope to put it here today.

allenh1 commented 7 years ago

Looking at the ebuild from the pchrist overlay, it's old (using EAPI=3). Version number is also old, 1.11-r4 vs. latest 1.49. It also doesn't offer as much flexibility as it could (it was trivial to add some cmake enable/disable controls via useflags, namely static-libs and doc). I'm cleaning up my ebuild a little bit. I hope to put it here today.

Great! I look forward to your PR!

gemarcano commented 7 years ago

I'm attaching my ebuild for geographiclib. I'm relatively new at making ebuilds, so it's likely I've missed some things or screwed up elsewhere (like with the Python deps). The dependencies are versioned to the version I have on my computer, since the actual build system for geographiclib doesn't seem to have much versioning information (in other words, it's possible the minimum version required for the dependencies is likely lower than what the ebuild currently has).

geographiclib-1.49.ebuild.zip

allenh1 commented 7 years ago

I'm attaching my ebuild for geographiclib.

After a quick once-over, this seems to be alright to me (with a few minor things). PR this and I'll comment on the small changes I'd like to see.

gemarcano commented 7 years ago

Sure. Although, reviewing some of the other ebuilds in the repository, they all come from bloom-generated repositories, correct? I'd have to edit/make the ebuild to point to/use a bloom-generated repository for geographiclib, which I'm not seeing exist currently in ros-gbp.

On that point, I tried emerging dev-python/bloom-0.6.1 from ros-overlay, and I got a pretty nasty error message from portage:

 *   Package installs 'test' package which is forbidden and likely a bug in the build system.

Thoughts?

allenh1 commented 7 years ago

On that point, I tried emerging dev-python/bloom-0.6.1 from ros-overlay, and I got a pretty nasty error message from portage

Yes, bloom is currently broken. :( No idea how to fix it.

gemarcano commented 7 years ago

Found the reason why it's broken:

packages=find_packages(exclude=['test']),

as found in bloom's setup.py isn't enough to prevent setuptools from finding all the test packages being used/made by bloom. I'm looking into how to properly block those directories/packages. I can always hard-code them away, but it would be great if I could do something like exclude=['test, test.*'], which is what I'm looking into right now.

gemarcano commented 7 years ago

That does it! I applied a test user-patch using that approach and it worked. Here's the patch:

diff --git a/setup.py b/setup.py
index 6e53df1..c7a6a2a 100755
--- a/setup.py
+++ b/setup.py
@@ -23,7 +23,7 @@ if sys.version_info[0] == 2 and sys.version_info[1] <= 6:
 setup(
     name='bloom',
     version='0.6.1',
-    packages=find_packages(exclude=['test']),
+    packages=find_packages(exclude=['test', 'test.*']),
     package_data={
         'bloom.generators.debian': [
             'bloom/generators/debian/templates/*',

EDIT: This should probably make it upstream, since technically they're polluting site-packages with a bunch of test.* packages.

EDIT2: https://github.com/ros-infrastructure/bloom/pull/444

allenh1 commented 7 years ago

@gemarcano Thank you for the patch! It's sitting in the patch folder and working on my machine.

gemarcano commented 7 years ago

While working on this a bit more, bloom, even though it only supports python 3.5 (at least that's the only python version it reports on Gentoo), blows up on itself when trying a bloom-release. Specifically, it screws up bytes and strings (classic python2/3 issue).

Specific to this issue, what do you actually need? A large part of the problem is that ROS doesn't have a geographiclib-release package or anything like that, which superflore can use/leverage. If we want to be able to re-generate ebuilds, we need a release repository for this package. If we don't care about rebuilding, I may be able to hack together an ebuild that simply adds the package.xml describing geographiclib and updating the ebuild to use the ros-cmake eclass.

allenh1 commented 7 years ago

While working on this a bit more, bloom, even though it only supports python 3.5 (at least that's the only python version it reports on Gentoo), blows up on itself when trying a bloom-release.

Hm... Well, I suppose we should force it back into version 2, or help the upstream patch for 3.

Specific to this issue, what do you actually need?

I need to have the key resolve within rosdep (this would be a pr into ros/rosdistro that would add in a Gentoo definition for the rosdep key that is missing). At that point, an ebuild can be generated by superflore. Finally, an ebuild for the missing dependency would make this resolved.

If we don't care about rebuilding, I may be able to hack together an ebuild that simply adds the package.xml describing geographiclib and updating the ebuild to use the ros-cmake eclass.

We probably shouldn't be regenerating geographiclib, as it's not ROS-specific (at least to my knowledge). So if you can just get a simple ebuild going for it, we'll stick it in the appropriate category folder and we should be good to go.

Alessandro-Barbieri commented 7 years ago

@gemarcano Can you give a look at this?

gemarcano commented 7 years ago

@Alessandro-Barbieri, I'm not sure that reporting this upstream would accomplish much, since the ros-*/ packages really don't mesh well with the upstream tree. At least not until the ros-*/ packages make it to upstream (if they ever do).

I've submitted a patch for bloom-release upstream that should fix the bytes vs. string issue.

So, looks that before we can do anything else, we need to PR to ros/rosdistro to add the gentoo definition for geographiclib and geographiclib-tools (yes, there's that other packages I haven't even begun to look at) before we can begin to work on the mavros package. I suppose I'll get the ball rolling on ros/rosdistro. I'll also PR the lunar tree in the overlay with the ebuild for geographiclib (I think I've configured properly for inclusion in the tree, although I may be abusing epatch slightly).

gemarcano commented 7 years ago

For ros/rosdistro, is it the rosdep/base.yaml that I should add geographiclib to?

EDIT: after reading a bit more, looks like it'll have to be the lunar/distribution.yaml, which looks complicated and wants a repository of some kind. Is there any documentation on the format for this file? I'm guessing there's a PEP for it...

gemarcano commented 7 years ago

On the matter of geographiclib-tools, they are included in the building of geographiclib itself, so just compiling and installing geographiclib deals with both of those dependencies.