search

takari / polyglot-maven

Support alternative markup for Apache Maven POM files
Eclipse Public License 1.0
893 stars 101 forks source link

Update to maven 3.9.0 #256

Closed laeubi closed 1 year ago

laeubi commented 1 year ago

Maven 3.9.0 has some new required components that polyglot currently fail with a NPE.

laeubi commented 1 year ago

@jvanzyl @gnodet can you help here?

cstamas commented 1 year ago

For this to work, migration from plexus to jsr330 is needed, see https://github.com/takari/polyglot-maven/pull/257

laeubi commented 1 year ago

For this to work, migration from plexus to jsr330 is needed, see #257

Don't understand this, compilation succeeds so why any migration is required for maven 3.9?

cstamas commented 1 year ago

Reason is plexus vs sisu (JSR330 implementation in Maven): plexus will create component descriptor at build time (using build time dependencies), while sisu merely just creates index of components, and calculates injections at runtime.

What happens here, that if at runtime your component (or it's superclass) changes, plexus -- as it expects "ready to consume" descriptor -- will fail to properly inject the component, while sisu will just happily do it.

Simply put, by use of plexus, you infer injected fields of a component across all supported versions as well, while this is NOT the case with sisu.

This is somewhat another Plexus limitation consequence: as if plexus would be able to use ctor injection, then compilation would fail against 3.9.0, but currently plexus supports field injection only, and that merely "hides" the problem at compile time, and becomes apparent at runtime only.

laeubi commented 1 year ago

3.9 do not use constructor injection so (as an intermediate solution) a recompile should fix the issue.

cstamas commented 1 year ago

No, it will not solve anything, you most probably miss the point how Plexus IoC works....

laeubi commented 1 year ago

Currently the problem is that a field is added in 3.9 that is (as you explained) is not injected because polyglot contains an old xml... if one rebuilds, the field will be added to xml an problem is solved.

Maybe its better to convert it to whatever is "current" in maven today, so if you can provide a patch that is even better that would be great (and this one can be closed then).

cstamas commented 1 year ago

Currently the problem is that a field is added in 3.9 that is (as you explained) is not injected because polyglot contains an old xml... if one rebuilds, the field will be added to xml an problem is solved.

Maybe its better to convert it to whatever is "current" in maven today, so if you can provide a patch that is even better that would be great (and this one can be closed then).

And this kind of "fix" would lock down build time Maven (would make polyglot usable only with build time Maven), while use of jsr330 makes it work still with 3.6.3, 3.8.7 (having no such field present) but also with 3.9.0.

Could you provide a patched XML instead on classpath?

laeubi commented 1 year ago

Well polyglot is a build extension and as those heavily depends on maven version anyways, it has literally zero maintenance, so for me it would be just fine to lock it to 3.9 as currently it works for 3.6.3, 3.8.7 but simply blow up for 3.9.0 ... so bump the version to 0.5.x if that makes things better :-)

Beside that 3.9.x is the new mainline so why not support this from now on only? Probably even upgrade to Java 8 (oficially 7 is supported).

Could you provide a patched XML instead on classpath?

Don't know if that would work, also seems quite annoying for users, for Tycho I'm currently trying this approach: https://github.com/eclipse-tycho/tycho/pull/2084/commits/232335945ed4bf26128d28ccf5fadf8303ebd2ab

laeubi commented 1 year ago

Closing in favor of:

but I can confirm that the Tycho-Hack works.