Fix for polyglot when in lib/ext

cstamas commented 4 months ago

Originally polyglot worked when loaded via .mvn/extensions.xml but not when created custom distro (put in lib/ext).

Reason was simple, as Maven core beans were not overridden when in "flat space", that causes lib/ext, unlike mvn extensions.

To make it work from lib/ext (when components are placed into maven core), following is needed:

DefaultProjectBuilder is "default" bean, hence we need to override it (using priority)
DefaultModelProcessor is trickier, it uses custom name (despite class name being Default) and uses Typed annotation. Essentially same thing, using priority makes polyglot component "prevail" as otherwise they are same ranked (non-default custom named components).
finally, do not extend component but use JSR330 capability to inject "exact" bean and just compose (possible as all used methods are public on replaced bean as well).

cstamas commented 4 months ago

@headius This PR is needed to make polyglot work when created "custom Maven distro" (repackaged Maven with polyglot in lib/ext). I tested and figured it does not work with current releases. Tested with polyglot ruby, but this is polyglot-common change, not lang specific at all.

cstamas commented 4 months ago

@mkristian ping as well ^^ so my advice, creating "custom maven distro" (unpack mvn zip, put polyglot-ruby w/ deps into lib/ext, repack) will NOT work with current version (tested manually). This PR adds the change that makes it work, but also keeps original capability to work from .mvn/extensions.xml as well.

cstamas commented 4 months ago

We may want 0.7.1 release with this change.

headius commented 4 months ago

@cstamas Thanks for the ping on this one. I tried to integrate polyglot into our ruby-maven gem myself and thought I must be doing something wrong! Glad to hear that it's a pretty simple fix in polyglot. Let me know if you need my help getting this integrated and released (testing etc).

I will open a bug or reference this PR from the ruby-maven side so folks know we are moving forward with a solution.

cstamas commented 4 months ago

I can merge and do 0.7.1 asap. Ok?

headius commented 4 months ago

If you are happy with it, go ahead with release.

I can work with @mkristian this week/end to get it integrated into ruby-maven and let you know if we have any further issues.

cstamas commented 4 months ago

https://github.com/takari/polyglot-maven/releases/tag/polyglot-0.7.1

Sync to central pending...

mkristian commented 4 months ago

@cstamas cool.thanks. yes I am going to work on this integration this weekend.

laeubi commented 3 months ago

This change now likely breaks the whole .mvn/extensions.xml usecase for Tycho:

https://github.com/eclipse-tycho/tycho/pull/4063

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.13.0:compile (default-compile) on project tycho-pomless: Compilation failure
/home/runner/work/tycho/tycho/tycho/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoTeslaProjectBuilder.java:[30,8] constructor TeslaProjectBuilder in class org.sonatype.maven.polyglot.TeslaProjectBuilder cannot be applied to given types;
  required: org.sonatype.maven.polyglot.TeslaModelProcessor,org.apache.maven.project.DefaultProjectBuilder
  found:    no arguments
  reason: actual and formal argument lists differ in length

cstamas commented 3 months ago

See https://github.com/eclipse-tycho/tycho/blob/main/tycho-extras/tycho-pomless/src/main/java/org/eclipse/tycho/pomless/TychoTeslaModelProcessor.java#L18

laeubi commented 3 months ago

Good point :-D

Lets see if it helps:

https://github.com/eclipse-tycho/tycho/pull/4077

laeubi commented 3 months ago

Now it fails with

Caused by: java.lang.NullPointerException: Cannot invoke "java.util.Map.get(Object)" because "options" is null
    at org.sonatype.maven.polyglot.TeslaModelProcessor.getPomXmlFile (TeslaModelProcessor.java:159)
    at org.sonatype.maven.polyglot.TeslaModelProcessor.read (TeslaModelProcessor.java:116)
    at org.sonatype.maven.polyglot.TeslaModelProcessor.read (TeslaModelProcessor.java:97)
    at org.eclipse.tycho.packaging.UpdateConsumerPomMojo.execute (UpdateConsumerPomMojo.java:196)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:903)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:280)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:203)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:569)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)

cstamas commented 3 months ago

As you invoke modelProcessor with null options here: https://github.com/eclipse-tycho/tycho/blob/e1967079439bb9dd6f3e604105cb6f5b118d1fce/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateConsumerPomMojo.java#L196

Which in case polyglot is installed is invalid. Polyglot is envisioned to be "installed" or "not installed" (is Maven extension), and when installed it "takes over some key components". Am unsure what this "pomless tycho extras" is doing and why it uses Polyglot at all...

laeubi commented 3 months ago

As you invoke modelProcessor with null options here: Which in case polyglot is installed is invalid.

Where is this documented? It obviously has worked in 0.7.0 (and previous versions) but now fails with 0.7.1

Polyglot is envisioned to be "installed" or "not installed" (is Maven extension),

Tycho is a Maven extension as well and uses (and extends) polyglot since years and actually was the reason for changes/bugfixes I proposed here over the time

Am unsure what this "pomless tycho extras" is doing and why it uses Polyglot at all...

Tycho extends Polyglot in a way that it can use Eclipse Metadata files as build input (like yaml or json or ... in this repository): https://tycho.eclipseprojects.io/doc/latest/StructuredBuild.html

laeubi commented 3 months ago

Beside that, the javadoc clearly states that null is allowed, so this is a bug of polyglot-common.

  /**
     * Reads the model from the specified file.
     *
     * @param input The file to deserialize the model from, must not be {@code null}.
     * @param options The options to use for deserialization, may be {@code null} to use the default values.
     * @return The deserialized model, never {@code null}.
     * @throws IOException If the model could not be deserialized.
     * @throws ModelParseException If the input format could not be parsed.
     */
    Model read(File input, Map<String, ?> options) throws IOException, ModelParseException;

laeubi commented 3 months ago

Passing an empty map results in

[ERROR] Failed to execute goal org.eclipse.tycho:tycho-packaging-plugin:5.0.0-SNAPSHOT:update-consumer-pom (default-update-consumer-pom) on project bundle1: Execution default-update-consumer-pom of goal org.eclipse.tycho:tycho-packaging-plugin:5.0.0-SNAPSHOT:update-consumer-pom failed: Unable to determine model input format; options={} -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.eclipse.tycho:tycho-packaging-plugin:5.0.0-SNAPSHOT:update-consumer-pom (default-update-consumer-pom) on project bundle1: Execution default-update-consumer-pom of goal org.eclipse.tycho:tycho-packaging-plugin:5.0.0-SNAPSHOT:update-consumer-pom failed: Unable to determine model input format; options={}
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:333)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:903)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:280)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:203)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:569)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default-update-consumer-pom of goal org.eclipse.tycho:tycho-packaging-plugin:5.0.0-SNAPSHOT:update-consumer-pom failed: Unable to determine model input format; options={}
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:133)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:903)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:280)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:203)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:569)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)
Caused by: java.lang.RuntimeException: Unable to determine model input format; options={}
    at org.sonatype.maven.polyglot.PolyglotModelManager.getReaderFor (PolyglotModelManager.java:63)
    at org.sonatype.maven.polyglot.TeslaModelProcessor.read (TeslaModelProcessor.java:153)
    at org.sonatype.maven.polyglot.TeslaModelProcessor.read (TeslaModelProcessor.java:97)
    at org.eclipse.tycho.packaging.UpdateConsumerPomMojo.execute (UpdateConsumerPomMojo.java:199)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:903)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:280)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:203)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at jdk.internal.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:77)
    at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:569)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:255)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:201)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:361)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:314)

laeubi commented 3 months ago

@cstamas @gnodet any chance to look into this? This currently completely breaks Tycho integration and polyglot is a very important part of it.

gnodet commented 3 months ago

There's not many code changes, why is that broken ? Maybe you should raise a different issue.

laeubi commented 3 months ago

I can only tell that it worked in 0.7.0 but fails in 0.7.1 and this one looks like it is changing some fundamental things I have now created

https://github.com/takari/polyglot-maven/issues/321

headius commented 3 months ago

@cstamas @mkristian I am just checking in to see how we're coming with this work and the integration into JRuby's ruby-maven gem. Let me know how I can help!

gnodet commented 3 months ago

@laeubi this issue is close, can you open a new one ? Is there a fix available yet ?

cstamas commented 3 months ago

So the timeline of events:

jruby used old maven (3.2.5?) in some docker powered builds where .mvn/extensions.xml was in docker FS root
maven in 3.5.x (or around, long time ago) updated mvn scripts that disregard root path, will not detect .mvn if in FS root
jruby project updated maven from ancient 3.2.x to 3.6.x (or even newer?) and suddenly their docker builds stopped working (as Maven disregarded .mvn/extensions.xml in FS root) -- did not load polyglot
I proposed them that for their docker build they create "custom" mvn distro with polyglot in lib/ext (example https://github.com/takari/polyglot-maven/pull/323)
BUT it turned out that polyglot is NOT capable to work like it (polyglot in extension classloader -- ie loaded via .mvn/extensions.xml worked, but when in same classloader where core is, it did not work, Sisu did not pick it up, hence priorities needed)
hence I created this PR and released 0.7.1 -- this change simply makes sure in whichever classloader polyglot is (is loaded via .mvn/extensions.xml or is present in lib/ext), is active and works as expected. Polyglot was always like this: IF present, it takes over... users never loaded polyglot "by chance" or loaded and did not used it.
in the meantime Maven 3.9.9 went out that fixed initial problem: it will pick up .mvn/extensions.xml from FS root
I have no idea how and for what Tycho uses polyglot, but it seems they had it added to Maven components, and problem for them is now that polyglot becomes "active" while before it was somehow (I assume) dormant (as Sisu did not pick them up)?

cstamas commented 3 months ago

So, for JRuby: @headius @mkristian I think all your issues are resolved, and please go for Maven 3.9.9 as then you can even leave your docker images unchanged, as Maven 3.9.9 (but not any before!) will pick up .mvn/extensions.xml from FS root.

For Tycho: again, am unsure how and for what polyglot is used in there and how it worked before, given the problem is that polyglot became "active" while it was not before. So why is it in classpath then in the first place?

laeubi commented 3 months ago

For Tycho: again, am unsure how and for what polyglot is used in there and how it worked before,

Tycho is providing a polyglot implementation "since ever" (oldest one I can found is form 2015) like polyglot-yaml, or polyglot-ruby, or ... just that is is not part of the polyglot repo and released independent.

given the problem is that polyglot became "active" while it was not before. So why is it in classpath then in the first place?

I never said that anywhere so I'm confused, polyglot is and was always active for Tycho (using .mvn/extensions.xml) and that part is still working. What is no longer working is, that if polyglot is active the Maven Modelreader is broken, thats what I have described here:

https://github.com/takari/polyglot-maven/issues/321

so whatever I pass (null or empty Map) the ModelReader fails to read that model as it should and worked before the micro change in version as used here (the ModelReader is used inside a Mojo!):

https://github.com/eclipse-tycho/tycho/blob/f77e3f951d99ee74d4c49f1d7e723566d767242b/tycho-packaging-plugin/src/main/java/org/eclipse/tycho/packaging/UpdateConsumerPomMojo.java#L194-L199

One special thing to consider is, that polyglot has these options:

There is a polyglot file (like, yaml, rb, ...) then the polyglot implementation is used
There is a pom.xml in wich case this takes precedence over the former and works "like before"

I can only guess that the second case is broken now somehow or there is a different ordering of calls or a missing delegation call as the message says

Unable to determine model input format; options={}

or you get a NPE

Cannot invoke "java.util.Map.get(Object)" because "options" is null

so it seems the modelreader expects a custom model in all cases.

lefou commented 3 months ago

For Tycho: again, am unsure how and for what polyglot is used in there and how it worked before,

Tycho is providing a polyglot implementation "since ever" (oldest one I can found is form 2015) like polyglot-yaml, or polyglot-ruby, or ... just that is is not part of the polyglot repo and released independent.

It's a bit unfortunate, that this never came to the attention of the polyglot maintainers. It would be good, if we add Tycho to the list polyglot dialects, even if it is maintained externally.

mkristian commented 3 months ago

I try to finalise this today or tomorrow with with done by @cstamas

mkristian commented 2 months ago

That is the change how we use all the new stuff from here: https://github.com/jruby/ruby-maven/pull/11

@headius there are still errors in the test which I just discovered this minute. That was totally under my radar.

cstamas commented 2 months ago

Fix found https://github.com/takari/polyglot-maven/pull/326 Tested with tycho and "custom ruby maven distro", both work.

takari / polyglot-maven

Fix for polyglot when in lib/ext #319