#317 Bundled Library exception request for Gazebo
Closed: Fixed None Opened 6 years ago by rmattes.

The Gazebo package (under review at https://bugzilla.redhat.com/show_bug.cgi?id=825409 ) contains a modified version of the ODE library (https://admin.fedoraproject.org/pkgdb/acls/name/ode)

The Gazebo version of the library contains significant modifications from the upstream library, including the removal or many runtime safety checks to improve the ODE library's performance, and the addition of some physics models to facilitate new models in Gazebo. The full difference between the latest gazebo release (1.8.7) and the latest ode release (0.12) can be found at http://rmattes.fedorapeople.org/RPMS/gazebo/ode-diff.diff

Given the extent of the changes, I think that patching the Fedora ODE package would be a bad idea. It would change the upstream API, and Fedora's ode would differ from other distributions. It may also require patching other consumers of the ODE library, and would require testing to make sure none of the changes adversely affect other consumers. As far as upstreaming the patches, it would take considerable time and effort. Gazebo developers would like to see it happen, but don't currently have time to do so (and there are no guarantees that upstream would take any or all of the changes [1][2]).

I think the best course of action in the short term is to allow a bundling exception for Gazebo, and try to merge the differences upstream in the meantime. If ode does not accept all of the patches, then gazebo would need to keep bundling exception indefinitely. Otherwise, we could start building against the Fedora ODE. The Gazebo version of ODE is built and installed as libgazebo_ode.so, so it shouldn't conflict with the ODE libraries in the ode packages.

[1] http://kforge.ros.org/pipermail/gazebo-list/2012-May/003944.html
[2] http://kforge.ros.org/pipermail/gazebo-list/2013-June/004566.html


gazebo is now to the point where all of its dependencies are in Fedora and this is one of the last outstanding issues holding up the review. Can this issue be added to one of the next meeting agendas?

This might have been skipped due to the lack of answers to the std. questions:

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Exceptions

My bad. I'll address them point by point.

== Has the library behaviour been modified? If so, how has it been modified? ==

The library has been extended from the base ODE library. Most of the modifications include adding a screw joint type to the engine, and providing functions to better control the parameters of ODE's quickstep solver, which I imagine gazebo added to more easily tune performance. There are also some helper functions added for angle normalization and things. Most crucially, the debug and assertion macros have been blanked out, diverging from upstream.

== Why haven't the changes been pushed to the upstream library? ==

They weren't pushed upstream because upstream isn't interested, or at least they weren't in 2012. kforge.ros.org is gone now (and so are the old gazebo list archives), but the 2012 thread contained this response to a question about upstreaming ODE patches from Nate Koenig:

{{{
Great question. For a long time we used the upstream version of ODE.
However, the maintainers of ODE refused to take our patches. None of
the changes are specific or invasive. They are things like speed
improvements, addition of a screw joint, basic code cleanup, and a few
other things that I'm forgetting.

As a result we were forced to bundle ODE into Gazebo. We continue to
make improvements to our version of ODE.

Bullet is almost completely integrated with Gazebo. We have high hopes
that the Bullet community will be a little more responsive.
}}}

I asked again about the status in 2013 shortly before opening this bug:

{{{

Hi all,

About a year ago, I sent a message to this list asking about
the status of gazebo's version of ODE with respect to ODE upstream.
It was mentioned that there were many modifications in Gazebo's fork,
and that the ODE upstream wasn't receptive to all of the changes made
for Gazebo. I'd like to confirm that this is still the case.

The reason I'm asking is because Fedora's packaging guidelines are
very strict about bundling libraries that are frowned upon in Fedora
packages[1], and this bundled library is really all that's standing
in the way of a package review and inclusion of Gazebo into Fedora's
repositories. There are basically three ways to work around a
modified library bundled with an application:

  1. Get all the patches in upstream
  2. Host the patches in the Fedora ODE package
  3. Apply for an exception with the Fedora packaging committee

If 1 is out of the question, 2 would be a bad idea (we wouldn't want
the Fedora package to be different from every other distro's ODE package)
which leaves 3 as the only option. I'm just making sure that's necessary
before going forward.

I did do a diff between ODE 0.12 and the Gazebo ODE sources, and it
looks like a lot of modifications in Gazebo were to remove runtime
safety checks and other performance improvements, and it makes sense
that ODE upstream wouldn't be keen on accepting those changes.

Thanks,

Rich
}}}

And got this response:

{{{
Hi Rich,

We would like to have the patches moved upstream, but we
lack the time and you are correct that the patches would
mostly likely be rejected.

I agree that option 2 would be a bad idea.

I could make the case that we are not bundling a library.
Our approach is more akin to creating a fork of ODE. The
installed libraries have a different name than ode, and
there should be no conflicts between gazebo and ODE. I'm
not sure what applying for an exception entails, but for
all intents and purpose our ODE is different from the
vanilla ODE.

Let me know what you think is the best approach.

-nate
}}}

I could not find any record of a conversation between gazebo and upstream on the ode mailing lists or in their patch tracker on sourceforge.

== Have the changes been proposed to the Fedora package maintainer for the library? ==
I have not proposed the changes to the Fedora ode maintainer. Some of the modifications change library behavior, like the removal of assertions and other runtime checking when NODEBUG is set.

== Could we make the forked version the canonical version within Fedora? ==
Possibly, we could patch the current ode to add at least the extended api, though I don't know if any default behavior has changed through the exposure of the solver api for instance.

== Are the changes useful to consumers other than the bundling application? ==
Possibly, if consumers found the need for a screw joint or for tuning the solver parameters. I think we probably don't want to make a canonical fork because
There's about 8 packages that currently depend on ode (looking at repoquery -q --whatrequires ode)
Those packages would then depend on a different ode in Fedora than in every other distribution
* Upstream is still actively developing ode, so if they make another release we diverge from other distributions even more

== Is upstream keeping the base library updated ==
They are based on ode 0.12 which is the current latest upstream release (and has been since 2012-05-28).

== What is the attitude of upstream towards bundling? ==
It looks like upstream would be happy to unbundle the ode library if upstream accepted the changes they made. I don't know if if they'd drop it right away, they'd probably wait for distributions to pick up up the updated ode.

== Overview of the security ramifications of bundling ==
I think security concerns are pretty minimal. It's a physics engine used in a dynamics simulation; there's not a whole lot to attack.

== Does the maintainer of the Fedora package of the library being bundled have any comments about this? ==
I don't know.

== Is there a plan for unbundling the library at a later time? ==
Not that I'm aware of. I'm thinking no because of the concerns that upstream was not interested in the patches, and the view of upstream that they're keeping a fork of ODE. I could double-check with upstream.

== Please include any relevant documentation ==
Block quotes in-line since the mailing list that the conversations were on are now gone (archives are from my saved email)

Also, for the C++ savvy, you can view the difference between ode-0.12 and gazebo-2.2.1 at http://rmattes.fedorapeople.org/gazebo-2.2.1_ode-0.12.diff

Hi,

I'm the Fedora ode maintainer, it looks like Gazebo has more or less effectively forked ode, this is not just a few bugfixes or minor API additions. As such I believe that bundling ode is the best way forward, and I'm ok with the ode bundling exception.

Regards,

Hans

FPC discussed this at last week's meeting and had several ideas on how to proceed:

First idea: it seems like gazebo has forked ode; even to the extent that the libraries have a different name. If the fork was provided as a separate subpackage (or even better, as a separate upstream tarball), that would make clear that this was a fork (maintained by a different upstream, might be linked to by others, providing its headers for such linking, etc) rather than a bundled copy.

If we can't do that, the other avenue to pursue is to have more information about why upstream ode doesn't accept the changes and what the changes are. One FPC member noted that much of the provided diff is cosmetic differences and wished there was a better summary of the changes between the two copies if this was chosen as the desired route.

I've made the gazebo-ode subpackage as suggested:

https://bugzilla.redhat.com/show_bug.cgi?id=825409

If that's sufficient then I think the package is almost ready to be in Fedora.

As far as the differences between ODE, I think i outlined them in the first section of comment 4. I don't know what the thoughts of the ode upstream are, but I have noted that upstream has recently been active with a new release after a period of relative inactivity. I will keep trying to investigate and hopefully we can unify the two branches in the future.

With the subpackage this was approved as a case of forking rather than bundling. You're good to go!

info Approve 317 as forking rather than bundling: APPROVED (+1:5, 0:0, -1:0)

Login to comment on this ticket.

Metadata