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

Split packages (repackaging OpenIMAJ as OSGi bundles) #130

Open
limstepf opened this Issue Jul 21, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@limstepf
Copy link
Contributor

limstepf commented Jul 21, 2017

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:

  • no.uib.cipr.matrix.EconomySVD.class

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:

  • org.openimaj.math.geometry.point.ScaleSpacePoint.class
  • org.openimaj.math.statistics.distribution.Histogram.class
  • org.openimaj.math.statistics.distribution.MultidimensionalHistogram.class

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:

  • org.openimaj.feature.OrientedFeatureVector.class from image-local-features has a split package with core-feature (as a quick-fix: needs to be merged with the core-math, core-feature, ... bundle). Interestingly the other "close calls" in here, org.openimaj.image.feature.local.* and org.openimaj.feature.local.* use an exclusive sub-package, so that's okay.
  • org.openimaj.math.matrix.ThreadedMatrixMulti.class from machine-learning has a split package with core-math (quick-fix: also merge this into the core-math, etc. bundle). This single class probably should just be moved to the core-math package as a proper solution.
  • nearest-neighbour has a split package with clustering since both have classes in org.openimaj.knn.pq (quick-fix: merge the two into a single bundle).
  • vector-image has classes in org.openimaj.image which is a split package with core-image (quick-fix: the two need to be merged).

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

  • core-math*: containing core-math, core-feature, image-local-features, machine-learning and mtj.
  • core-image*: containing core-image and vector-image.
  • clustering*: containing clustering and nearest-neighbour.

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

This comment has been minimized.

Copy link
Member

jonhare commented Sep 7, 2017

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.