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

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

Open
gemarcano opened this issue Oct 27, 2017 · 22 comments
Open

Comments

@gemarcano
Copy link
Contributor

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
Copy link
Contributor

allenh1 commented Oct 27, 2017

@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
Copy link
Contributor Author

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

@gemarcano
Copy link
Contributor Author

gemarcano commented Oct 27, 2017

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
Copy link
Contributor

allenh1 commented Oct 28, 2017

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
Copy link
Contributor

allenh1 commented Nov 2, 2017

@gemarcano Any progress here?

@Alessandro-Barbieri
Copy link
Contributor

geographiclib is only available in the pchrist overlay
an entry was added with ros/rosdistro@3c9f482
should we remove that entry?

@gemarcano
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

allenh1 commented Nov 3, 2017

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
Copy link
Contributor Author

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
Copy link
Contributor

allenh1 commented Nov 3, 2017

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
Copy link
Contributor Author

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
Copy link
Contributor

allenh1 commented Nov 3, 2017

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
Copy link
Contributor Author

gemarcano commented Nov 3, 2017

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
Copy link
Contributor Author

gemarcano commented Nov 3, 2017

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: ros-infrastructure/bloom#444

@allenh1
Copy link
Contributor

allenh1 commented Nov 6, 2017

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

@gemarcano
Copy link
Contributor Author

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
Copy link
Contributor

allenh1 commented Nov 7, 2017

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
Copy link
Contributor

@gemarcano Can you give a look at this?

@gemarcano
Copy link
Contributor Author

gemarcano commented Nov 7, 2017

@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
Copy link
Contributor Author

gemarcano commented Nov 7, 2017

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
Copy link
Contributor Author

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.

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

No branches or pull requests

3 participants