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

Update to maven 3.9.0 #256

Closed
wants to merge 1 commit into from
Closed

Update to maven 3.9.0 #256

wants to merge 1 commit into from

Conversation

laeubi
Copy link
Contributor

@laeubi laeubi commented Feb 10, 2023

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

@laeubi
Copy link
Contributor Author

laeubi commented Feb 10, 2023

@jvanzyl @gnodet can you help here?

laeubi added a commit to laeubi/tycho that referenced this pull request Feb 13, 2023
Currently polyglot do not work with maven 3.9 because it extends an
internal component ... this extends the polyglot component to enbedd
updated metadata in tycho itself.

See:
- takari/polyglot-maven#256
@cstamas
Copy link
Member

cstamas commented Feb 13, 2023

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

@laeubi
Copy link
Contributor Author

laeubi commented Feb 13, 2023

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
Copy link
Member

cstamas commented Feb 13, 2023

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
Copy link
Contributor Author

laeubi commented Feb 13, 2023

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

@cstamas
Copy link
Member

cstamas commented Feb 13, 2023

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

@laeubi
Copy link
Contributor Author

laeubi commented Feb 13, 2023

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
Copy link
Member

cstamas commented Feb 13, 2023

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
Copy link
Contributor Author

laeubi commented Feb 13, 2023

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:
eclipse-tycho/tycho@2323359

@laeubi
Copy link
Contributor Author

laeubi commented Feb 13, 2023

Closing in favor of:

but I can confirm that the Tycho-Hack works.

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

Successfully merging this pull request may close these issues.

2 participants