Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.Sign up
add full VERSIONs / SONAMEs to our libraries #273
This has been on my mind for quite some time and I consider the discussion (and hopefully merge) of this request as a blocker for the upcoming indigo and kinetic releases.
This request is still incomplete and only adds versions to
As a result of this request the libraries do not install as
Because this sets each library's SONAME to the full version, this enforces that every binary links with the versioned library file from now on and has to be relinked with each new release of MoveIt!.
An alternative would be to set the SONAME to
The reason for this commit is that it is practically impossible to maintain full ABI compatibility within each ROS distribution and still add the the features/patches the community asks for. This has resulted in more than one ABI-incompatible MoveIt! release in the recent past within a ROS distribution. Because the libraries have not been versioned up to now, there was no way to indicate the incompatible changes and users who did not rebuild their whole workspace with the new release encountered weird and hard-to-track segfaults or broken behavior.
The main alternative I see for us at the moment is to set hard-coded versions for each library. Then, whenever we do a release and changed the ABI of individual libraries within MoveIt, we could increase the version of each of these libraries independent of each other. As a result, only binaries that link against libraries that actually changed their ABI would have to be rebuild with a new release.
@jspricke I discussed the need for SONAMEs with you quite a bit, so it would be great if you could have a look at it too.
referenced this pull request
Oct 9, 2016
+1. If we have to break things, the louder and earlier it happens during the build, the better.
As for using using the moveit version or manually bumping on ABI breakage: it sounds like the manual method is too easy to forget. I'd prefer to err on the safe side even if that means unnecessary rebuilds. So unless someone thinks they can put in the effort for reliably bumping sonames on releases with ABI change, my preference is for using the moveit version.
As for including the patch number or not: it would be nice if it can be left out. That should be fine as long as we bump minor version when ABI compatibility breaks. The only problem with that is the indigo/jade/kinetic releases conflicting with each other, but I think that is a different discussion. But as long as we break ABI in patch-level releases we should include it.
On Mon, Oct 10, 2016 at 12:49:26AM -0700, Maarten de Vries wrote:
Yes and yes.
The problem is that we do (and want to) backport new features and ABI changes to older branches, but will not backport API changes.*
*This, by the way, somewhat relates to the proposal to have a "master" branch that should work across the supported ROS distributions/Linux distributions.
+1 to including the patch version in the sonames, we release so rarely I don't think the rebuild time is that important and I do forsee us still breaking ABI between patch versions due to the way our versioning is done between I/J/K. I also like having the sonames the same as the versions, to ease maintenance. I'd like to see this PR expanded to all of MoveIt!, as @v4hn intends.
@wjwwood perhaps you have any wisdom on this change for moveit?
I like the idea of introducing sonames in general. However, I think we missed some important points:
@rhaschke its not clear what you are proposing - just that we shouldn't include the patch version in the sonames?
After your points its also not clear to me what the advantage of this PR is - currently people who use MoveIt! debians sometimes have their catkin workspace broken after a ROS update because of ABI changes. With this new PR, it will be the same except that will always happen - correct?
The point is that with sonames, the breakage will be explicit (ie: things will not start because they cannot find the libraries they were linked against), while right now, things will appear to be fine (because all the libraries are there), but due to ABI incompatibilities, users may run into all sorts of hard to understand / debug segfaults.
As @rhaschke writes, with the current infrastructure (Bloom, etc) , the other advantage of sonames (ie: linking against a particular version, with several different versions all present in parallel) will not work.
On Mon, Oct 17, 2016 at 10:49:47AM -0700, Robert Haschke wrote:
Yes. Having sonames in the libraries supports having multiple versions of the same library installed.
And also: Yes, bloom and OSRF's packaging schemes do not account for such version "transitions" (that's the term for it in Debian) at all. As far as I've been told this was discussed at length some years ago and (I've been told) the final argument was that OSRF does not want to "guarantee the stability this would imply". The argument does not make any sense to me, but this thread is clearly not the place to discuss this.
Yes, this is intentional.
All "industry" setups I know (including OSRF's build system) rebuilds the whole dependent software stack for each update.
We should definitely minimize ABI changes, I agree. Also to avoid having to recompile your whole moveit development workspace all the time.
In theory I agree. This does not work with our current versioning scheme though.
Of course ABI and API are two entirely different things. We should definitely not backport any API change and did not do that in the recent past.
I considered this and don't think this makes sense for the moment. It would leave us with the same problems we have right now.
I think, we agree on most points. My post was just to bring some pros and cons of the approach to our attention. Sorry, if I failed @davetcoleman.
I agree. However, I see a third option: increasing the patch level of soname independent of the patch level of the release version (slower or leaving out / not increasing when ABI stability is maintained).
I didn't argued against backporting at all, but to carefully think about what to backport and what not. Furthermore, I would like to reduce the effort to rebuild downstream packages as much as possible - which is possible if we manually increase the patch version of SONAMES.
I think, if we increase the patch version manually, it could work. Maybe, you didn't yet considered this option.
This PR still only covers moveit_core and is not ready to be merged in. This change should also be held off until the next Kinetic release - a line must be drawn in the sand for releasing software or it will never happen. There is still too much debate going on here.
How about a middle of the road that promotes getting a kinetic release out, but also managing the upgrade path:
I just added the remaining libraries. I was too tired to do this yesterday evening.
@mikeferguson Your whole proposal presupposes that binaries built against
Oct 19, 2016
1 check passed
On Wed, Oct 19, 2016 at 10:35:41AM -0700, Dave Coleman wrote:
I don't think so. At least not the way we use sonames in MoveIt now.
For a package like geometric_shapes, I believe it shouldn't be a problem to keep the ABI stable within each ROS distribution,