Skip to content
This repository has been archived by the owner on Dec 21, 2021. It is now read-only.

Nullable cause issues with OSGi #31

Closed
mickaelistria opened this issue Jul 26, 2016 · 18 comments
Closed

Nullable cause issues with OSGi #31

mickaelistria opened this issue Jul 26, 2016 · 18 comments

Comments

@mickaelistria
Copy link
Contributor

Intoduction of the dependency on Nullable make it complex to integrate ls-api in OSGI:

  • Import-Package: javax.annotation resolves to some other bundle that do not contain Nullable, driving to NoClassDefFound at runtime
  • Adding a dependency to org.apache.servicemix.bundles.jsr305 from io.typefox.lsapi via a fragment leads to conflcts with guava resolving the javax.annotation package from the JVM directly
osgi> start io.typefox.lsapi
gogo: BundleException: Could not resolve module: io.typefox.lsapi [18600]
  Bundle was not resolved because of a uses contraint violation.
  org.osgi.service.resolver.ResolutionException: Uses constraint violation. Unable to resolve resource io.typefox.lsapi [osgi.identity; type="osgi.bundle"; version:Version="0.3.0.201607261001"; osgi.identity="io.typefox.lsapi"] because it is exposed to package 'javax.annotation' from resources org.apache.servicemix.bundles.jsr305 [osgi.identity; type="osgi.bundle"; version:Version="3.0.1.1"; osgi.identity="org.apache.servicemix.bundles.jsr305"] and org.eclipse.osgi [osgi.identity; type="osgi.bundle"; version:Version="3.11.0.v20160603-1336"; osgi.identity="org.eclipse.osgi"; singleton:="true"] via two dependency chains.

Chain 1:
  io.typefox.lsapi [osgi.identity; type="osgi.bundle"; version:Version="0.3.0.201607261001"; osgi.identity="io.typefox.lsapi"]
    import: (osgi.wiring.package=javax.annotation)
     |
    export: osgi.wiring.package: javax.annotation
  org.apache.servicemix.bundles.jsr305 [osgi.identity; type="osgi.bundle"; version:Version="3.0.1.1"; osgi.identity="org.apache.servicemix.bundles.jsr305"]

Chain 2:
  io.typefox.lsapi [osgi.identity; type="osgi.bundle"; version:Version="0.3.0.201607261001"; osgi.identity="io.typefox.lsapi"]
    import: (&(osgi.wiring.package=com.google.common.collect)(&(version>=14.0.0)(!(version>=19.0.0))))
     |
    export: osgi.wiring.package=com.google.common.collect; uses:=javax.annotation
  com.google.guava [osgi.identity; type="osgi.bundle"; version:Version="15.0.0.v201403281430"; osgi.identity="com.google.guava"]
    import: (osgi.wiring.package=javax.annotation)
     |
    export: osgi.wiring.package: javax.annotation
  org.eclipse.osgi [osgi.identity; type="osgi.bundle"; version:Version="3.11.0.v20160603-1336"; osgi.identity="org.eclipse.osgi"; singleton:="true"]

Basically, the issue is that that the javax.annotation package is split across several sources: JVM, javax.annotation bundle, org.apache.servicemix.bundles.jsr305... OSGi cannot create a classpath merging all those splits.

@briandealwis @iloveeclipse: are you aware of any good way to simultaneously depend on guava and some jsr305 bundle?

@iloveeclipse
Copy link

Mickael: no. We removed import-package directive from guava's manifest to workaround the issue locally.

@briandealwis
Copy link

You can use the often-overlooked bundle-symbolic-name attribute on your Import-Package to specify which bundle should be used for the resolution:

Import-Package: javax.annotation; bundle-symbolic-name="org.jsr305"

It's nearly equivalent to specifying Require-Bundle.

@briandealwis
Copy link

You could use Require-Bundle for both org.apache.servicemix.bundles.jsr305 and com.google.guava: OSGi will merge their respective contributions for the javax.annotation package.

@spoenemann
Copy link
Member

We could also consider introducing our own annotation, since we're using it only for (a) documentation of what elements may be null and (b) run-time checking of incoming and outgoing messages. In both cases we don't need the annotation to be from the "standard" javax.annotation package. The standard package could just be useful for interoperability with other findbugs-style tools.

@mickaelistria
Copy link
Contributor Author

A possible hack would be that you include a copy of javax.annotation.Nullable in the bundle that requires it.

@mickaelistria
Copy link
Contributor Author

mickaelistria commented Jul 26, 2016

Another workaround would be that the code lookup for Nullable via reflection and skip checks if not found

Class<?> nullableClass = null;
try {
  nullableClass = getClass().getClassLoader().loadClass("javax.annotation.Nullable")
} catch (NoClassDefFound e) {
 // log Null check disabled
 return;
}

and make the import-package optional.

@briandealwis
Copy link

briandealwis commented Jul 26, 2016

The @Nullable and @Nonnull attributions aren't required at runtime, so an easy solution is to just not import them at all. Findbugs and Eclipse JDT's Null Analysis will pull the references out from the .class files.

To clarify: the annotations are build-time artifacts, and not run-time artifacts. In Maven terms, you'd have a compile-time dependency on org.apache.servicemix.bundles.jsr305 or wherever you're obtaining them from, but you wouldn't include javax.annotation in your Import-Package header.

@mickaelistria
Copy link
Contributor Author

@briandealwis In this case, these annotations are used at runtime here to automate some checks in the marshaller/unmarshaller: https://github.com/TypeFox/ls-api/blob/master/io.typefox.lsapi.services/src/main/java/io/typefox/lsapi/services/validation/ReflectiveMessageValidator.xtend#L39

@briandealwis
Copy link

A possible hack would be that you include a copy of javax.annotation.Nullable in the bundle that requires it.

And instead of 2 different sources of the javax.annotation.Null annotations, we'll now have 3 :-)

It's an ugly situation and with JSR-305 dormant, it doesn't look like it will get any better. I think I'd go with @spoenemann's suggestion to define your own annotations. At least you have control over them and you can add new annotations should you see the need instead of feeling shoehorned into an existing set.

@svenefftinge
Copy link
Member

If we were introducing our own annotation we wouldn't leverage any nullable checkers.
Couldn't we carefully check whether the nullable annotation is on the classpath and if not just disable the checking?

@iloveeclipse
Copy link

iloveeclipse commented Jul 26, 2016

If we were introducing our own annotation we wouldn't leverage any nullable checkers.

That's not true. See jgit/egit projects. JDT can be configured to use custom style annotations, and FindBugs accepts different package names for NP annotation types.

@briandealwis
Copy link

It seems there are two separate issues:

  1. Using nullness annotations within a bundle so that you can use @Nullable/@Nonnull within your own code.
  2. Using nullness annotations to validate code from other bundles. For this to work reliably, since you can't really control where these other bundles are obtaining their nullness annotations, you will need to use a bytecode library, like ASM, to pull out the annotations and match them by name. java.lang.reflect's getAnnotations() methods use the introspected class' classloader to resolve annotation classes, and annotations that can't be resolved are skipped. When the introspected class is from a different bundle than your analyzer then you may see different annotation class instances altogether, which can be very confusing (my @Nullable comes from org.jsr305, yours comes from org.apache.servicemix.bundles.jsr305). @mickaelistria's idea of resolving the annotation from the introspected class won't work if that bundles followed my suggestion of not actually importing the javax.annotation package.

@spoenemann
Copy link
Member

spoenemann commented Jul 27, 2016

We're going for a solution with catching NoClassDefFoundError where we read the annotations at runtime, and adding resolution:=optional to the javax.annotation like Guava does. We could also add bundle-symbolic-name="..." as suggested before, but which bundle should we put there?

@mickaelistria
Copy link
Contributor Author

The only bundle I found so far is the org.apache.servicemix.bundles.jsr305 one. But maybe there are alternatives. Seems like @briandealwis found a org.jsr305 bundle somewhere...

spoenemann added a commit that referenced this issue Jul 27, 2016
@briandealwis
Copy link

com.google.code.findbugs:jsr305:3.0.1 is the jar/bundle I've seen most often used. The OSGi metadata was first added for 3.0.1 and its Bundle-SymbolicName is org.jsr-305.

It looks like org.apache.servicemix.bundles.jsr305 was a bundle wrapping an version 3.0.0 of that same jar.

@briandealwis
Copy link

I added a comment to the commit: the code there will work for a flat classpath. But when verifying a class under a managed classpath, you want to use the @Nullable as seen by that class.

@mickaelistria
Copy link
Contributor Author

Thanks, the patch does its job so the ls-api works in OSGi world without complex classloading tweaks now. Should we close this report?

@svenefftinge
Copy link
Member

thanks for trying.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants