eligosource / eventsourced

A library for building reliable, scalable and distributed event-sourced applications in Scala
Apache License 2.0
828 stars 98 forks source link

Add OSGi meta-data to eventsourced #34

Closed rocketraman closed 11 years ago

rocketraman commented 11 years ago

Patch to build to add OSGi meta-data.

NOTE: I had to rebuild sbt-osgi [1] myself locally, and publish the 0.5.0-SNAPSHOT. It doesn't seem to resolve correctly from the typesafe repo.

[1] https://github.com/sbt/sbt-osgi

rocketraman commented 11 years ago

Issue #32 already open for this.

krasserm commented 11 years ago

Thanks Raman. To get it working I switched to sbt-osgi 0.4.0 which can be resolved. At the time of writing this comment, versions 0.4.0 and 0.5.0-SNAPSHOT are identical. Can you please verify that the latest additions to master work properly regarding OSGi?

Thanks, Martin

rocketraman commented 11 years ago

Yes, everything is working perfectly on master, thanks for merging!

krasserm commented 11 years ago

Hi Raman,

I factored out journal implementations into separate modules on master. With this refactoring I'm not sure if I screwed things up with the OSGi metadata. Can you please do me a favor and check if everything is still working in an OSGi environment? The common OSGi settings are in Build.scala whereas the module-specific settings are in the build.sbt files of the individual modules. Currently, publishing of OSGi bundles is disabled and only sbt osgi-bundle will create OSGi bundles locally.

Thanks in advance!

Cheers, Martin

rocketraman commented 11 years ago

Sure, I can do that. Can you give me a few days?

krasserm commented 11 years ago

Hi Raman, take as much time as you need. Thanks a lot, highly appreciate it.

Cheers, Martin

rocketraman commented 11 years ago

Ok, I have a couple of initial comments:

1) With the leveldbjni 0.6 release, upstream has added the OSGi meta-data (https://github.com/fusesource/leveldbjni/commit/c875916c8a8a5c595f00416f3e7447212af9d758). However, they modified the jar to export the leveldb-api (package org.iq80.leveldb) with version 0.5. So the imports for eventsourced have to change to match (in es-journal/es-journal-leveldb/build.sbt).

2) You have introduced a "split package" situation. This is where more than one jar exports the same package:

$ bnd print es-core/target/scala-2.10/eventsourced-core_2.10-0.5-SNAPSHOT.jar 
...
Export-Package
  org.eligosource.eventsourced.core     {version=0.5.0.SNAPSHOT}
  org.eligosource.eventsourced.patterns.reliable.requestreply{version=0.5.0.SNAPSHOT}
...

$ bnd print es-journal/es-journal-leveldb/target/scala-2.10/eventsourced-journal-leveldb_2.10-0.5-SNAPSHOT.jar 
...
Export-Package
  org.eligosource.eventsourced.core     {version=0.5.0.SNAPSHOT}
  org.eligosource.eventsourced.journal.common{version=0.5.0.SNAPSHOT}
  org.eligosource.eventsourced.journal.leveldb{version=0.5.0.SNAPSHOT}
  org.eligosource.eventsourced.patterns.reliable.requestreply{version=0.5.0.SNAPSHOT}
...

This is generally a bad thing for OSGi (and more generally for modular applications) because more than one jar is exporting the same package. However, the good news appears to simply be a build problem, because the leveldb journal jar appears to export the same packages/classes as core:

$ diff -u <(jar tvf es-journal/es-journal-leveldb/target/scala-2.10/eventsourced-journal-leveldb_2.10-0.5-SNAPSHOT.jar | grep core) <(jar tvf es-core/target/scala-2.10/eventsourced-core_2.10-0.5-SNAPSHOT.jar | grep core)
--- /dev/fd/63  2013-02-22 13:13:19.295185192 -0500
+++ /dev/fd/62  2013-02-22 13:13:19.295185192 -0500
@@ -1,4 +1,4 @@
-     0 Fri Feb 22 13:03:22 EST 2013 org/eligosource/eventsourced/core/
+     0 Fri Feb 22 13:03:08 EST 2013 org/eligosource/eventsourced/core/
   1534 Fri Feb 22 13:03:06 EST 2013 org/eligosource/eventsourced/core/Behavior$$anonfun$receive$1.class
   2762 Fri Feb 22 13:03:06 EST 2013 org/eligosource/eventsourced/core/Behavior$class.class
   2363 Fri Feb 22 13:03:06 EST 2013 org/eligosource/eventsourced/core/Behavior.class

Other packages may have the same "issue" e.g. journal.common and others.

If you intended the eventsourced-journal-leveldb_2.10-0.5-SNAPSHOT.jar to contain the core and journal common classes so that it can be used on its own, then this is perfectly fine and I guess no change is needed. But if you intend the user to use the core jar, the journal-common jar, and the journal implementation jar of their choice, then this should be fixed.

rocketraman commented 11 years ago

So the imports for eventsourced have to change to match (in es-journal/es-journal-leveldb/build.sbt)

Also, I'd recommend upping the leveldb import to 1.6 to ensure you are getting at least the 1.6 version at runtime.

krasserm commented 11 years ago

Hi Raman,

thanks a lot for reviewing and your feedback.

Cheers, Martin

krasserm commented 11 years ago

ad 2: packaging the classes from dependent modules is not intended: I currently don't know why this is done by the plugin and how to turn that off (need to look at the docs/plugin again)

Do you know how this can be turned off?

rocketraman commented 11 years ago

Do you know how this can be turned off?

I think if you remove them from "Export-Package" and add them to the "Private-Package" header the plugin should cause them to not be exported by the bundle, and therefore not included in the jar, e.g.:

OsgiKeys.privatePackage := Seq(
  "org.eligosource.eventsourced.core.*"
)

OsgiKeys.exportPackage := Seq(
  "!org.eligosource.eventsourced.core.*"
)

The maven plugin has much better defaults than the sbt plugin does -- check the section "Default Behavior" here: http://felix.apache.org/site/apache-felix-maven-bundle-plugin-bnd.html. You generally want to follow these defaults.

I tried the above for journal-common but was unsuccessful -- it still included the core classes. To be honest I don't know why. I've had issues with the sbt plugin before too -- it's really sensitive to minutiae. I really dislike it (along with sbt more generally).

krasserm commented 11 years ago

Hi Raman, thanks for the hints. I'll see what I can do (and will make further updates to issue #68).