You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2016/11/03 08:48:26 UTC

[jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

This is based on https://github.com/jclouds/jclouds/pull/1033.

This PR fixes the build of the previous one and makes the OSGi framework version configurable, so we can run builds for different environments. The scope of the OSGi dependencies is `provided`, so we can expect target platforms to have different versions than the ones used to build jclouds.

With this change, the build succeeds with the base version, and also with the latest versions:

    mvn clean install -Dosgi.version=6.0.0 -Dosgi.compendium.version=5.0.0
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds/pull/1034

-- Commit Summary --

  * org/jclouds/scriptbuilder/functionloader/osgi/BundleFunctionLoader.java:
  * Make code compatible with newer OSGi framework versions

-- File Changes --

    M core/pom.xml (2)
    M core/src/test/java/org/jclouds/osgi/BundlesTest.java (14)
    M core/src/test/java/org/jclouds/osgi/MetadataBundleListenerTest.java (42)
    M project/pom.xml (12)
    M resources/modernizer_exclusions.txt (1)
    M scriptbuilder/pom.xml (1)
    M scriptbuilder/src/main/java/org/jclouds/scriptbuilder/functionloader/osgi/BundleFunctionLoader.java (10)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/1034.patch
https://github.com/jclouds/jclouds/pull/1034.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Ignasi Barrera <no...@github.com>.

Changes are made to accommodate the tests to the signatures of those methods in newer OSGi framework versions..

Basically, they change from Dictionary (raw) to Dictionary<String, ?>, and in the case of the `loadClass`method, it returns a Class (raw) object in 4.2.0 but a Class<?> in 6.0.0.

The change sin this PR make the osgi versions configurable (I've already created a jenkins build that uses the latest osgi 6.0.0) and safely accommodates the method signatures.

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034#issuecomment-258101532

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Andrew Phillips <no...@github.com>.

demobox commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

> I think the exclusion makes sense there, globally, to ensure we can use the OSGi 6 framework method signatures where needed.

Makes sense - thanks for the additional explanations, @nacx and @andrewgaul!

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Ignasi Barrera <no...@github.com>.

nacx commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

No, I meant `HashMap`. The modernizer plugin forbids the use of Hashtable in favor of HashMap, but HashMap does not extend Dictionary. Thus it can't be used as an argument to the BundleContext.registerService method. That's why I just excluded it from modernizer, as we now actually need a Hashtable instance. See [this comment](https://github.com/jclouds/jclouds/pull/1033#issuecomment-258078992) for further details.

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Andrew Gaul <no...@github.com>.

andrewgaul commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

Unfortunate limitation of Modernizer:

https://github.com/andrewgaul/modernizer-maven-plugin/issues/3

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Ignasi Barrera <no...@github.com>.

nacx commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

@andrewgaul Note that the signatures of the OSGi framework require a `Dictionary<String, ?>`, and the properties object is no longer compatible, as it extends `Hashtable<Object, Object>`. We need to pass a proper dictionary there, thus this exclusion (`HashMap` does not extend `Dictionary`).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034#pullrequestreview-6969233

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Ignasi Barrera <no...@github.com>.

nacx commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

Unfortunately no :( The modernizer plugin just reads the exclusions but you cannot configure them "just for one package or class".
We could override the configuration just for the scriptbuilder project, but if at some point we need to use the registerService in core, we'll have the same issue. I think the exclusion makes sense there, globally, to ensure we can use the OSGi 6 framework method signatures where needed.

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Andrew Phillips <no...@github.com>.

demobox commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

> The modernizer plugin forbids the use of Hashtable in favor of HashMap, but HashMap does not extend Dictionary

Ah, like so. Would it be possible/make sense to apply this exclusion to a more limited scope, or do we need to create `Dictionary` objects also in jclouds core for this?

Thanks for explaining, @nacx!

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Andrew Phillips <no...@github.com>.

demobox commented on this pull request.



> @@ -20,3 +20,4 @@ com/google/common/primitives/Longs.compare:(JJ)I
 com/google/inject/Inject
 com/google/inject/Provider
 java/lang/StringBuffer."<init>":()V
+java/util/Hashtable."<init>":(I)V

> HashMap does not extend Dictionary

Do you mean `Hashtable`?

-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034

Re: [jclouds/jclouds] Make jclouds compatible with newer OSGi framework versions (#1034)

Posted by Andrew Phillips <no...@github.com>.

demobox commented on this pull request.

The changes appear to be mainly in tests - are those related to the OSGi version bit, or just cleanup?

Thanks for taking this on, @nacx!



-- 
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/1034#pullrequestreview-6973312