openimaj / openimaj

The OpenIMAJ source code repository
http://www.openimaj.org
Other
330 stars 116 forks source link

Split packages (repackaging OpenIMAJ as OSGi bundles) #130

Open limstepf opened 7 years ago

limstepf commented 7 years ago

This is an issue that doesn't affect your usual OpenIMAJ use case (with just a single class-loader), but makes repackaging OpenIMAJ dependencies as OSGi bundles needlessly complicated (and at the cost of losing out on modularity).

OSGi background

Split packages (i.e. multiple jars offering classes under the same package namespace) are a problem in the OSGi world, leading to the situation that only one (partial) package is visible to a given class-loader (from some bundle). The situation could be dealt with "Require-Bundle", but that's rather ugly (see OSGi 6.0.0 core specification, 3.13 Requiring Bundles, p.79).

Problem: OpenIMAJ split packages

There might be more such cases, but so far I've found two of them:

1) no.uib.cipr.matrix.* is (primarily) provided by com.googlecode.matrix-toolkits-java.mtj-1.0.2 (a dependency of core-math), but unfortunately core-math also provides the package no.uib.cipr.matrix with a single class EconomySVD.

2) org.openimaj.math.* is (primarily) provided by core-math, but unfortunately core-feature also has two packages: org.openimaj.math.geometry and org.openimaj.math.statistics (which are both also to be found in core-math).

A simple and straight-forward repackaging (e.g. with maven-bundle-plugin) of core-math is thus not possible.

Fix (in the meantime)

To solve this issue, I didn't go the require-bundle route, instead I've built an extended core-math bundle including all classes from core-math, core-feature, and mtj (maven-dependency-plugin:unpack), thereby merging them (and the split-packages) into a single bundle. This works fine, but it's a bit of a hack (and depending on licenses maybe not even legal), and obviously takes away modularity.

Solution

The proper solution would be to not have split packages in the first place. And I think this could be easily achieved with a tiny bit of refactoring. The two cases above show a clear pattern of a second bundle "adding to/extending" another bundle. A simple and clear fix would, while still using the same (root) namespace, at least use its very own, distinct sub-package. For example:

In core-math move:

to no.uib.cipr.matrix.core-math.EconomySVD.class (or a similarly exclusive sub-package of no.uib.cipr.matrix - if it has to be in there in the first place; chances are it might be cleaner to keep it under org.openimaj.math somewhere).

And in core-feature there are the three classes:

that need to be moved out of their packages into their own, exclusive ones. Again either something like org.openimaj.math.geometry.point.core-feature and org.openimaj.math.statistics.distribution.core-feature, or stay with these three classes under org.openimaj.math somewhere.

Now, I understand such a fix/refactoring will break someone's code, so this is probably not something for a minor update. But maybe consider this for the next major update, and until then, maybe make sure to not introduce even more split packages in the meantime.

P.S. Bonus points for adding OSGi bundle manifests to all OpenIMAJ jars in the future. ;)

EDIT: There are some more split packages:

...thus so far, I ended up creating three "extended" bundles to deal with these split packages, thereby losing quite some modularity:

The case with mtj is especially nasty, since it's a split package with some third party. To lessen the pain, I've embedded (or rather inlined) version 1.0.2 in the extended core-math bundle (which also gets exported from this bundle in that version), while I also have a standalone bundle of mtj around (other bundles depend on) in version 1.0.4. This works, but still...

jonhare commented 7 years ago

Some of these will be do-able without too much problem. The EconomySVD one is a pain because it uses package-private methods from other classes in that package, so can't be relocated - I'm not sure if there are any good solutions to that?