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

m2e slf4j-over-logback vs. groovy vs. Eclipse classloader #269

Closed
reckart opened this issue Jul 9, 2021 · 20 comments
Closed

m2e slf4j-over-logback vs. groovy vs. Eclipse classloader #269

reckart opened this issue Jul 9, 2021 · 20 comments

Comments

@reckart
Copy link

reckart commented Jul 9, 2021

The installation of Eclipse Groovy 4.2.0 and higher forces the removal of the m2e slf4j-over-logback plugin causing Maven logs to be no longer available in the console view.

It appears that there is a story behind that related to a change in the classloaders in Eclipse 4.19 and that at least one group has been able to address the issue by internalizing its logback dependency.

Maybe m2e can also be adjusted such that the separate m2e slf4j-over-logback bundle is no longer required and thus also removing the problems with Groovy / Eclipse classloading?

Related issues:

@laeubi
Copy link
Member

laeubi commented Jul 9, 2021

Embedding dependencies is plainly wrong... So if Eclipse Groovy is doing strange things it should be fixed there instead of forcing unrelated projects adopting bad practices.

@reckart
Copy link
Author

reckart commented Jul 12, 2021

There is what seems to be a pretty good analysis of the problem here: pmd/pmd-eclipse-plugin#134 (comment) - including how the PMD project chose to resolve it by internalizing the logback dependency (not the Groovy project).

Quote:


Ok, PMD Plugin depends on logback, but Groovy Eclipse Plugin does not work with logback - so they configured a explicit incompatibility. When you install Groovy Eclipse Plugin, you need to deinstall e.g. "m2e - slf4j over logback logging (Optional)" or any other plugin, that depends on logback (such as PMD).

Logback classic itself depends on some groovy packages (optionally). But when Groovy Eclipse Plugin is installed, eclipse wires them together. Now the Groovy Eclipse Plugin does some OSGI magic for switching between different Groovy versions (it unloads the bundles with the old groovy version and loads the bundles with the new version - and OSGI in turn should refresh all dependent bundles - including logback). This somehow does not work reliably anymore and one reason was/is logback. So they declared a explicit incompatibility with that bundle.

If you want to know the whole story, read: groovy/groovy-eclipse#1237 and groovy/groovy-eclipse#981 and groovy/groovy-eclipse#317

I'll change the PMD Plugin to not depend on the logback/slf4j bundles anymore and instead bring in these dependencies on my own (via bundle classpath).

That fixes the installation problem and probably also the crash of the original issue.


So the problem seemed to have been be a cross dependency between logback-classic and Groovy at a low level and how the Eclipse OSGI classloader then handles resolving the problem. It doesn't seem to me to be something that is the fault of the Groovy Eclipse plugin.

However, it is an overall complicated and slightly annoying situation and I wonder if m2e would be able to do something to mitigate the issue on its side as well - even if it is only providing more insight on a better overall solution of the situation.

@laeubi
Copy link
Member

laeubi commented Jul 12, 2021

So the problem seemed to have been be a cross dependency between logback-classic and Groovy at a low level

Then it should be solved there

how the Eclipse OSGI classloader

There is no such " Eclipse OSGI classloader" Eclipse uses Equinox and Equinox resolves packages according to the OSGi Specification. This explicitly allows (conflicting) dependencies of different modules/libraries if declared properly.

It doesn't seem to me to be something that is the fault of the Groovy Eclipse plugin

Your description states "Groovy Eclipse Plugin does not work with logback" so it obviously is a fault of the Groovy Eclipse plugin to "stop working" if something unrelated is present...

I wonder if m2e would be able to do something to mitigate the issue on its side as well

If any analysis reveals that m2e should use more specific version ranges or alike yes otherwise not... but embedding a dependency is clearly not the solution especially on APIs as slf4j as this completely contradicts the point of APIs...

@eric-milles
Copy link

One of the problems that makes this difficult to solve for all parties involved is that the ch.qos.logback.classic_1.2.3 bundle is missing a version restriction on one of its package imports. Most groovy imports in that bundle have a range [2.4,3) which comes from the original logback-classic bundle. The import groovy.lang is missing a range restriction. Also the groovy support provided by logback-classic is optional and I cannot find a way to make equinox stop making a link between org.codehaus.groovy and ch.qos.logback.classic.

@mickaelistria
Copy link
Contributor

@eric-milles Thanks for this analysis. Please report issue about ch.qos.logback.classic bundle to the producer, which I guess is Orbit?

@laeubi
Copy link
Member

laeubi commented Jul 19, 2021

This problems are best solved ad the producer-side (e.g. logback and alike) andmake them supply a fix.

If you really want to support people using such wrecked osgi definitions (e.g. as part of the Eclipse-Groovy) there is a powerful tool in OSGI the Resolver Hook Service Specification just make sure your bundle starts at the very beginning (e.g. start-level 1) has no dependencies to anything you like to intercept, use the raw OSGi API (no injection frameworks like DS...) and keep in mind that with great power comes great responsibility.

You will the be able to e.g. hide org.codehaus.groovy from the ch.qos.logback.classic bundle and blacklist certain version from being resolved to specific "bad bundles"...

@reckart
Copy link
Author

reckart commented Jul 20, 2021

So... I opened an issue against Orbit: https://bugs.eclipse.org/bugs/show_bug.cgi?id=574922

I found a git repo at Eclipse for Orbit bundles, but no corresponding GitHub project which would facilitate submitting a PR.

@guw
Copy link
Contributor

guw commented Jul 20, 2021

@reckart the Orbit project uses Gerrit (see https://projects.eclipse.org/projects/tools.orbit/developer for more info). There is a osgi.bnd which is used for tweaking the OSGi info. It has an entry for org.codehaus.groovy.* to set the version range but lacks one for groovy.*.

However, I'm not sure it's possible to prevent this connection. If an optional import can be satisfied Equinox will connect the dots.

@reckart
Copy link
Author

reckart commented Jul 20, 2021

@guw so you say the problem cannot be fixed at the level of the bundle producer and needs to be fixed in other places (m2e, groovy-eclipse, etc.) instead?

@laeubi
Copy link
Member

laeubi commented Jul 20, 2021

However, I'm not sure it's possible to prevent this connection. If an optional import can be satisfied Equinox will connect the dots.

What is perfectly fine and should not create any issue...

so you say the problem cannot be fixed at the level of the bundle producer and needs to be fixed in other places

There is nothing that needs to be "fixed" as its not the opportunity of unrelated projects to do anything here that involves standard OSGi behaviour

@eric-milles
Copy link

@guw The osgi.bnd linked above makes mention of groovy-all. I just wanted to note that Groovy stopped shipping a groovy-all jar from 2.5 onwards. I'm not sure if this is important to the support for groovy config in logback. I say this because the version range that gets written is [2.4.0,3) and maybe it could be [2.4.0,2.5). If this were the case, there should be no groovy bundle that satisfies the dependency.

@guw
Copy link
Contributor

guw commented Jul 20, 2021

@eric-milles That's a discussion for the Logback project. However, it does look like Groovy is disabled in the last version due to JDK 9 issues. It would be great if Logback can ship a separate jar without the Groovy stuff. We could consume this one then.

https://github.com/qos-ch/logback/blob/61ffd3aba042bbb22f588599ff5574d3b9667d22/logback-classic/src/main/java/ch/qos/logback/classic/util/ContextInitializer.java#L73

@reckart
Copy link
Author

reckart commented Jul 21, 2021

Ok... I have opened an issue at Logback as well now (updated the description above with the link too):

@reckart
Copy link
Author

reckart commented Jul 22, 2021

The logback maintainer has created a groovy-less branch of logback 1.2.4 and asks if somebody could test if this solves the problem - https://bugs.eclipse.org/bugs/show_bug.cgi?id=574922

@reckart
Copy link
Author

reckart commented Jul 29, 2021

The Groovy Eclipse plugin has switched to using the groovy-less logback version (@eric-milles correct me if I this is wrong) and removed the conflict declaration with the m2e slf4j-over-logback plugin. That means, the Groovy Eclipse plugins and the m2e slf4j-over-logback plugin can now be installed together again.

It might be a sensible idea for m2e to also switch to a groovy-less logback unless Groovy support is essential for m2e. I'll leave it to the m2e team to decide if this issue can be closed now or if any action in this project should be taken under it.

@laeubi
Copy link
Member

laeubi commented Jul 30, 2021

afaik m2e does not ship/require logback in any way and just uses slf4j-api.

@eric-milles
Copy link

@reckart Just to sum up the change in groovy-eclipse, I was able to use the advice of @laeubi and break the optional link between org.codehaus.groovy and ch.qos.logback.classic. groovy/groovy-eclipse@c0907a1#diff-e4a261ab8c27546221fb3396dd7ee7acabf3212c9e3a1d67e610202bb1c5bbce

With that link broken, I was able to remove the incompatibility declaration. Thus users will not see the dreaded dialog box during install that requests them to remove any feature that uses logback-classic under the covers. This includes "m2e - slf4j over logback logging" which directs maven output to the Maven Console.

You may inform the PMD project that they could restore logback to a normal bundle dependency. Although I would advise waiting until logback-classic 1.2.4 is available via Eclipse Orbit. Starting with 1.2.4, logback-classic will only link to Groovy 2.5. (I looked for the change that resulted in a version restriction in the deployed jar's manifest but couldn't find it. Might be the new tooling versions that did this. I was able to confirm the manifest for logback-classic-1.2.4 contains: groovy.lang;resolution:=optional;version="[2.4,3)")

@eric-milles
Copy link

Long story short, Groovy-Eclipse 4.2.0 declares logback-classic incompatibility regardless of m2e (or whomever) making use of logback-classic 1.2.4 or 1.2.4-groovyless. Groovy-Eclipse 4.3.0 breaks the links to logback-classic regardless of m2e (or whomever) making use of logback-classic 1.2.x or 1.2.4-groovyless. So I would consider this issue ticket resolved.

@mickaelistria
Copy link
Contributor

Thanks all for discussion and related action in Groovy-Eclipse.

@HannesWell
Copy link
Contributor

Maybe m2e can also be adjusted such that the separate m2e slf4j-over-logback bundle is no longer required and thus also removing the problems with Groovy / Eclipse classloading?

M2E now uses logback from Maven-Cenntral: #931
Furthermore the logback.configuration plugin was merged into the logback.appender plugin (and the result was just renamed to slf4j.logback: #943

Just in case those changes are relevant for this issue.

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

No branches or pull requests

6 participants