towards a centralised libphidget release

muhrix commented 9 years ago

Hello,

I am one of the maintainers of phidgets_drivers, and we have recently released the package for Hydro and Indigo.

We used to implement a similar solution for downloading and building libphidget before releasing the package. However, some changes were made for the release, and phidgets_drivers now depends on libphidgets because you guys had already released the library and we wanted to avoid conflicts.

Such changes sparkled an interesting discussion at https://github.com/ccny-ros-pkg/phidgets_drivers/pull/18.

Whilst I am aware that libphidgets, as it is, currently breaks REP 136, @v4hn pointed out some other disadvantages, with the outdated version of libphidget being the key criticism, and @mintar provided suggestions to overcome the issues discussed.

Therefore, we were wondering whether cob_extern would be willing to drop libphidgets so that phidgets_drivers can take over the release of the libphidget library.

We believe it would make more sense for libphidget to reside in phidgets_drivers rather than cob_extern, and we would also ensure the library release will benefit from bug fixes of latest versions, always testing compatibility prior to release.

Would you be willing to go forward with these changes? What are your concerns and suggestions?

Thanks!

fmessmer commented 9 years ago

Thanks for the suggestion! We'll discuss it and then get back to you...

@ipa-bnm: What do you think?

@ipa-fmw, @ipa-lth: FYI

floweisshardt commented 9 years ago

no problem from my side.

@ipa-bnm: If we get your "go" that the two packages are compatible I can arrange thinks to drop libphidgets out of cob_driver:

remove libphidgets from ipa320/cob_extern
add dependency to phidgets_drivers in cob_driver
trigger a new release (indigo-only) for cob_extern without libphidgets
wait for new release of phidgets_drivers containing libphidgets (@muhrix will trigger the release)
trigger new release of cob_driver

muhrix commented 9 years ago

To ensure compatibility, I would add the exact same libphidgets package to phidgets_drivers.

I think it might be better to trigger Hydro/Indigo releases of phidgets_drivers first (I suppose you guys are not using that package, so no conflicts expected). Once those packages are released, you can then drop libphidgets from cob_extern and add dependency to phidgets_drivers, without the need to wait for the build farm to release phidgets_drivers (which cob_extern will have as dependency).

Does that make sense?

benmaidel commented 9 years ago

Can't argue with that. From my side this is ok.

floweisshardt commented 9 years ago

@muhrix I'm OK with your suggestion. Please let me know as soon as I can trigger new releases for cob_extern.

mintar commented 9 years ago

@muhrix : That probably won't work. The hydro/distribution.yaml and indigo/distribution.yaml files of ros/rosdistro already contain entries for the libphidgets package (in cob_extern), so you'll get an error when you try to release a duplicate. It first has to be removed from cob_extern, the pull request has to be merged into rosdistro, and then you can re-add it.

mintar commented 9 years ago

+1 for adding an exact duplicate now and cleaning it up separately.

muhrix commented 9 years ago

True, very good point @mintar.

I was trying to think of a way to cause as little disruption to cob_extern as possible.

An alternative would be to release the package under the name originally used within phidgets_drivers. If we released libphidgets as phidgets_api, then cob_extern will be able to make the switch afterwards, without depending on a yet-to-be-released package.

The potential problem here is for those who install BOTH phidgets_drivers and cob_extern. The libraries would be overwritten (though they would be identical anyway).

mintar commented 9 years ago

I don't like the phidgets_api name, since it's hard to tell that it actually contains libphidget. The original name of the library is libphidget (without the s), so why not use that?

Another thing: If we really want to comply with REP 136, we shouldn't import libphidget into ccny-ros-pkg/phidgets_drivers. Instead, we should create a new release repository for it and follow this bloom tutorial. I've never done that, so I can't say how hard it is to maintain in the long run (right now, having dozens of branches seems a bit awkward). But at least it's the recommended way to release third-party packages, so perhaps we should give it a try. One advantage of using a separate repository would be that we could use the upstream version numbers (like 2.1.8.20150410), perhaps with a suffix like -ros1 if we want to add patches (in case bloom doesn't add that automatically). Any thoughts @muhrix @gavanderhoorn @jspricke?

muhrix commented 9 years ago

We could use the name libphidget, sounds good!

Let me try and organise my thoughts here. This is what (I think) we need to do:

Move libphidgets across to phidgets_drivers
1. Decide whether cob_extern can afford dropping the package first, or agreeing on a new name for the package (such as libphidget)
2. Make pull requests to ros/rosdistro and trigger releases (breaking REP 136 for now)
3. Wait until everything is working
After step 1 is complete, work out the best approach to deal with REP 136, options being:
1. Follow @mintar's suggestion and create a new release repo
2. Leave as it is (would be my choice, explained below*)

(does it sound reasonable?) Thus the question to be answered at this point is how do we approach 1.i.?

*I got in touch with people working on releasing the actual Phidgets library as .deb packages, but the roadmap is Ubuntu 15.10. Given the amount of work required to create a new release and the uncertainties involved in the maintenance, I'd rather use my time to contribute to the release of the .deb packages so we have the library as a system dependency (which I think is ideal and eventually will happen) at the cost of breaking REP 136 until then. In any case, this is step 2.

mintar commented 9 years ago

Actually, I think we should decide now whether we want to follow REP 136 or not. Otherwise, you would have to undo everything you did in step 1. So it's not really first 1, then 2; it's 1 or 2. I'm fine with either option.

Whichever option we choose, I'd suggest we pick the name "libphidget". Then we can release that package first, and when it's done, libphidgets can be dropped from cob_extern and replaced with a dependency on libphidget.

+1 for getting libphidget into Debian proper; however, the Ubuntu 15.10 deadline is ambitious, and even if you make it, the earliest ROS distro where it will be available is K-Turtle in April 2016. So we'll be stuck with this solution for a while. :-)

v4hn commented 9 years ago

On Mon, Apr 13, 2015 at 07:38:12AM -0700, Murilo FM wrote:

After step 1 is complete, work out the best approach to deal with REP 136, options being:

Leave as it is (would be my choice, explained below*)

I agree that it's a good idea to wait for the official debian package iff they succeed in getting it in there this time.

However, I would like to ask to support an official (and recommended) way to detect the provided libphidget21, which works without catkin. Either by providing the original package-config file, or (as already suggested) by providing a FindLibPhidget.cmake file and contributing it to the upstream libphidget project.

floweisshardt commented 9 years ago

the suggestion of @mintar (adding a libphidget first and then remove libphidgets) is fine for me.

fmessmer commented 9 years ago

so what's the next step here?

mintar commented 9 years ago

I think the next thing to do would be to create a separate REP 136 compliant repo called libphidget (no "s"), following this tutorial. Since it's going to be closely integrated with phidgets_drivers, @muhrix would be the ideal maintainer. I would volunteer helping with setting up the repo, if you need help.

sevenbitbyte commented 8 years ago

Whats the current status here. Is libphidgets distributed by a package other than cob_extern/libphidgets now? I'm trying to make use of both catkin packages libphidgets and phidgets_api on jade and ran into issues with the version of the library included in this repo and have patched it in PR #55 to solve my issues.

v4hn commented 8 years ago

The libphidgets module is not released into jade yet. Imho, this provides a nice way to move the module to phidgets_drivers. @muhrix: could you add the libphidgets module from here to phidgets_drivers and release it into jade? To account for the upstream pkg-config file I mentioned earlier, it is actually enough to rename the module to libphidget21 (this is the official name of the upstream project). Therefore I also propose to do this renaming with the jump to jade.

This way it would be enough if cob_extern would drop their libphidgets module from cob_extern if/when they wish to move to jade.

muhrix commented 8 years ago

Yes, I can add libphidgets to phidgets_drivers and renamed it to libphidgets21 in the transition, as long as cob_extern is alright with dropping libphidgets (the actual package name wouldn't conflict, but the library names would).

That should push me to try Jade (I'm still on Indigo)...

v4hn commented 8 years ago

On Wed, Feb 24, 2016 at 10:58:24AM -0800, Murilo FM wrote:

Yes, I can add libphidgets to phidgets_drivers and renamed it to libphidgets21 in the transition,

libphidget21 - without the additional 's'.

Great!

as long as cob_extern is alright with dropping libphidgets (the actual package name wouldn't conflict, but the library names would).

They don't need to drop it. It would be nice though, if they agree not to release their libphidgets package into jade (as mentioned, they did not do this yet). Instead they can use the libphidget21 package that should be available there by then. @ipa-fmw: is this reasonable at your end?

fmessmer commented 8 years ago

@muhrix @sevenbitbyte @v4hn @mintar @ipa-fmw @ipa-bnm

Well, as we said before:

we will not release libphidgets into Jade anytime soon
feel free to go ahead and release it as libphidget21 under phidgets_drivers
after this is done, we (ipa320) will test our packages against phidgets_drivers/libphidget21 in favor of cob_extern/libphidgets

Note, all this can be done without name conflict - neither in Indigo nor in Jade/Kinetic - as mentioned by @v4hn

mintar commented 8 years ago

@muhrix : I'll do it if you can give me "write" (or better still "admin") permissions on muhrix/phidgets_drivers-release and ccny-ros-pkg/phidgets_drivers .

I don't know if you can do that for ccny-ros-pkg/phidgets_drivers ; I've asked @idryanov about that long ago, but he ignored that.

Sorry for spamming this issue with internal organization stuff, but it probably makes sense to keep the discussion in one place.