Require Java 17 to run Tycho

[akurtakov]

No description provided.

[github-actions]

Test Results

328 files  328 suites   2h 1m 4s ⏱️
293 tests 287 ✔️ 4 💤 0 ❌ 2 🔥
586 runs  575 ✔️ 9 💤 0 ❌ 2 🔥

For more details on these errors, see this check.

Results for commit c4496e3.

♻️ This comment has been updated with latest results.

[akurtakov]

@laeubi Any idea about CiFriendlyVersionsTest failures? I don't see how they can be related with Java 17 switch.

[laeubi]

Only idea would be to run the test locally (not as an integration test) and look for unusual output ...

e.g. for the error:

java.lang.AssertionError: /home/jenkins/agent/workspace/tycho-github_PR-1365/tycho-its/target/projects/CiFriendlyVersionsTest/testDefaultBuildQualifier/ci-friendly/buildqualifier/bundle/target/bundle-1.0.0.2022.jar is not generated!

one might look at what is generated instead ... maybe there is a message in the log, maybe a warning that something has changed due to the updated project settings?

[laeubi]

I have compile locally and the problem is that the TychoCiFriendlyVersions component seem to be not working anymore.

This component is listed in /META-INF/sisu/javax.inject.Named but it seems "something" is not compatible in Maven for Java 17 there or maybe something else interferes here? I think one might want to try out smaller steps, e.g. upgrading only the minimum java version or only updating the Tycho version (even though it should not matter here)...

Its just hard to find out anything as "maven" and "java 17" are to general terms and all results map to user problems.

I hope this helps looking into it, to debug you can do:

1. Checkout master
2. build tycho-its/projects/ci-friendly/buildqualifier$ mvnDebug clean package -Dtycho.buildqualifier.provider=jgit -Dtycho.buildqualifier.format=yyyyMM and set a breakpoint at TychoCiFriendlyVersions constructor --> will be triggered
3. check out this PR
4. build again --> breakpoint is not triggered.

[laeubi]

I could workaround the issue by using

@Component(role = ModelVersionProcessor.class)

instead of

@Named

but

we should really find out why this do not work @Named is actually a more "modern" approach and used both in maven and other plugins/extensions! So if there is an issue with Java 17 and we force people to use Java 17 because of Tycho, this might break other build setups!

Some documentation https://maven.apache.org/maven-jsr330.html

[akurtakov]

My gut feeling points me in the direction of dependency issue. I'll try to look into it later today.

[akurtakov]

Also https://maven.apache.org/maven-jsr330.html says that sisu-maven-plugin has to be used but I don't see it there .

[laeubi]

Also https://maven.apache.org/maven-jsr330.html says that sisu-maven-plugin has to be used but I don't see it there .

I have no clue, I can only tell that in both cases the required file is generated, and as this is done at build-time I can't guess why it should make a difference at runtime, so for me the only difference seem the java-class version here.

[laeubi]

Alright I tried using

<properties>
      <min.jdk.version>11</min.jdk.version>
</properties>

in tycho-build module and then it seems to work, so it seems something in the JSR-330 can't work with Java 17 binary classes and silently ignores this (maybe because it can't find the annotations on that class anymore?).

[akurtakov]

Are the two tycho-build jars identical when build with Java 11 and 17?

[laeubi]

Are the two tycho-build jars identical when build with Java 11 and 17?

I cant find any obvious difference:

tycho-build-3.0.0-SNAPSHOT_11.zip
tycho-build-3.0.0-SNAPSHOT_17.zip

The only change I do is in this single module (all else is compiled with java 17) and I always run the build with Java 17 ...

<properties>
      <min.jdk.version>11</min.jdk.version>
</properties>

[akurtakov]

Worst case we can make tycho-build be java 11 bytecode.

[laeubi]

Worst case we can make tycho-build be java 11 bytecode.

I think this could work, I still wonder if it makes sense to move do Java 17 if maven self has issues with java 17 :-\

[akurtakov]

Until someone moves, no issue will be discovered or fixed. That's the world we live in. I've went through this when tycho was failing to even run on Java 11, it was a tough time getting things to run on modular VM and no waiting helped even a slightly. So someone has to make a move for the whole ecosystem to progress. E.g. if tm4e/m2e hasn't moved eclipse-jdt/eclipse.jdt.core@36ac8f4 wouldn't have happened and we would still be waiting for things to "improve".

[laeubi]

Problem is just that if we have some code requiring java 11 then, so I think we should stell get an idea what component is the problem here and how it could be fixed for future release.

So any idea? Is it guice? or eclipse-sisu?

[akurtakov]

At least google/guice#1529 seems to point to guice.

[laeubi]

This one indicates that asm might have an influence here? google/guice#1521 as far as I can see guice 4.1.2 is used by maven what might contain outdated asm?

@akurtakov as you are the Java 17 evangelist here, would you like to bring the issue to the maven-dev list and/or bugtracker? As far as I can see maven itself test for 8/11/17 so it is a bit strange and we probably doing something wrong here?

[laeubi]

@michael-o can you probably help with this?

[michael-o]

@gnodet

[mickaelistria]

It looks like Java 17 support comes with Guice 5.1.0: https://github.com/google/guice/wiki/Guice510 while Maven 3.8.5 and 3.8.6 includes Guice 4.2.2.
Since IMO, migration to Java 17 is much more important for the future of Tycho than preferring Guice annotations over Plexus ones and since Plexus seems to be more capable of Java 17 and also since this is the only Maven component for which we use @Named instead of @Component, I would recommend that we adopt the workaround (or even solution until Maven ships a Java 17-compatible guice) suggested in #1365 (comment)

[michael-o]

3.9.0 contains 5.1.0

[akurtakov]

I'll do the chnage to component.

[laeubi]

@akurtakov I'll backport that if thats ok?

[akurtakov]

Please do, thanks!

[laeubi]

3.9.0 contains 5.1.0

Is there any plan for a release of 3.9.0?

than preferring Guice annotations over Plexus ones and since Plexus seems to be more capable of Java 17 and also since this is the only Maven component for which we use @Named instead of @Component

I just wanted to note that JSR-330+Sisu Guice is the recommended approach over plexus actually.

https://cwiki.apache.org/confluence/display/MAVEN/Maven+Ecosystem+Cleanup#MavenEcosystemCleanup-DropPlexusContainerforJSR-330+SisuGuiceextension

[mickaelistria]

I just wanted to note that JSR-330+Sisu Guice is the recommended approach over plexus actually.

OK, interesting and I like the idea of supporting JSR-330 in general; but the lack of forward compatibility with newer Java version makes Guice actually not so cool as it seems. Plexus is actually more powerful than Guice on that matter; and the fact that Java releases are speeding up, forward compatibility with upcoming Java version may become high priority.