TWCable / gradle-plugin-scr

Makes it easier to work for with the Felix SCR annotations for Gradle
Apache License 2.0
4 stars 2 forks source link

Gradle multiple slf4j bindings found #1

Closed leojplin closed 8 years ago

leojplin commented 8 years ago

When executing the build task on my bundle with the scr plugin, I get this error. I have asked on Stackoverflow: http://stackoverflow.com/questions/34514641/gradle-internal-slf4j-is-conflicting-with-my-own-slf4j-dependency

williamsonm commented 8 years ago

The problem seems to be inside generateErrorMsg, when it tries to load a class that contains SLF4J loggers. I commented out that section temporarily, so I can at least build my project.

https://github.com/liljerk/gradle-plugin-scr/commit/340c1558b24723ba7bce3a9846adfe083623fa58

ghost commented 8 years ago

+1

jdigger commented 8 years ago

@liljerk Can you please submit a PR for the change?

jdigger commented 8 years ago

I wrote a test case to try to reproduce the issue, and can't. See writeBuildFile where I make sure to have a project with both this and logback in the classpath.

Is it truly failing the build, or just giving a warning message?

leojplin commented 8 years ago

I think it would occur if you include your own slf4j libraries, even if compile time only dependencies.

ghost commented 8 years ago

it is failing the build. workaround introduced by @liljerk fixes the problem.

jdigger commented 8 years ago

Unfortunately I can't evaluate @liljerk's patch unless I have a test that manifests the problem, and I'm having an extremely difficult time reproducing this.

I copied & pasted the build files in the StackOverflow post, filled in the "obvious" missing pieces (settings.gradle, a service class with SCR annotations, etc.) and created a fully self-contained project that I've attached: scr-slf4j.zip

$ unzip ../scr-slf4j.zip
Archive:  ../scr-slf4j.zip
  inflating: build.gradle
   creating: gradle/
   creating: gradle/wrapper/
  inflating: gradle/wrapper/gradle-wrapper.jar
  inflating: gradle/wrapper/gradle-wrapper.properties
  inflating: gradlew
 extracting: settings.gradle
   creating: submod/
  inflating: submod/build.gradle
   creating: submod/src/
   creating: submod/src/main/
   creating: submod/src/main/java/
   creating: submod/src/main/java/testpkg/
  inflating: submod/src/main/java/testpkg/SimpleService.java

$  ./gradlew build
:compileJava UP-TO-DATE
:processResources UP-TO-DATE
:classes UP-TO-DATE
:jar
:assemble
:compileTestJava UP-TO-DATE
:processTestResources UP-TO-DATE
:testClasses UP-TO-DATE
:test UP-TO-DATE
:check UP-TO-DATE
:build
:submod:compileJava
:submod:processResources UP-TO-DATE
:submod:classes
:submod:processScrAnnotations
:submod:jar
:submod:assemble
:submod:compileTestJava UP-TO-DATE
:submod:processTestResources UP-TO-DATE
:submod:testClasses UP-TO-DATE
:submod:test UP-TO-DATE
:submod:check UP-TO-DATE
:submod:build

BUILD SUCCESSFUL

Total time: 1.259 secs

Everything runs just fine: no warnings or failures. What's different/missing?

leojplin commented 8 years ago

I am trying this out on an actual AEM project that has bundle and content folders, which is an usual structure for an AEM project. Can you also try it? Some information http://www.aemcasts.com/aem/episode-1.html

I am using this plugin in conjunction with the cq-bundle plugin as well.

jdigger commented 8 years ago

@leoxy520 That's also what we have on my "real" (for pay) project. It is a huge collection of bundles and content folders distributed across multiple AEM packages all built with a multi-module Gradle build. It works perfectly for us (for well over three years), so I know that that configuration in-and-of-itself is not a problem.

There has to be something else going on.

If you can give an example project (either modify the one I provided or make a new one, obviously without revealing any proprietary information) that manifests the issue, then I'll have something tangible to test against.

jdigger commented 8 years ago

FWIW, in the sample project it's also using the the cq-bundle plugin -- it's a copy & paste of what you pasted on StackOverflow.

williamsonm commented 8 years ago

Clone the sample project I created below to see the exception that gets thrown during processScrAnnotations. The problem is the static logger inside the Handler class. If you remove the static keyword the build will succeed.

https://github.com/liljerk/gradle-broken-scr-plugin

jdigger commented 8 years ago

Thanks, that gives me something I can work with.

jdigger commented 8 years ago

I've been able to create a test case for it, allowing me to fix it without breaking existing functionality (i.e., the safety check for interfaces). I hope to have a new version released today.

jdigger commented 8 years ago

Fixed in v1.1.0

williamsonm commented 8 years ago

Thanks @jdigger

What's the deal with the interface check anyway? I've got a legacy codebase with a dozen or so References to classes that implement osgi.service.event interfaces, but aren't interfaces themselves, and some other References to abstract classes (like granite's UserPropertiesService), and they all seem to bind just fine.

jdigger commented 8 years ago

It works because is Felix being very "forgiving" with the spec. See http://wiki.osgi.org/wiki/Declarative_Services and https://felix.apache.org/documentation/subprojects/apache-felix-service-component-runtime.html

In addition to the use of interfaces just being a better general design, they also enable the runtime to do a lot more work and be more efficient. The same is true, of course, for the SCR processor itself since it will automatically populate the @Service({..}) for you with all implemented interfaces. It's not as vital to DS, but if you were to upgrade to OSGi Blueprint it absolutely requires interfaces so it can use proxies to handle the more flexible lifecycle management.

I did add a flag to disable the check, but by default it's off because it's not "proper" OSGi.