compiler release option

[LorenzoBettini]

Quoting from https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html

The --release option ensures that the code is compiled following the rules of the programming language of the specified release, and that generated classes target the release as well as the public API of that release. This means that, unlike the -source and -target options, the compiler will detect and generate an error when using APIs that don't exist in previous releases of Java SE.

This means that we can forget about compiling with Java 11 and safely compile with Java 17 always. If we tried to use API not present in Java 11 we would get a compilation error.

This would allow us to switch to Tycho 3 for good (xtext/xtext-monorepo#15)

I added the property in the parent POM. I did NOT remove the source and target options, which would be redundant, because as far as I know our Maven plugins read those properties and not the release property.

Most of our bundles already had org.eclipse.jdt.core.compiler.release=enabled in org.eclipse.jdt.core.prefs, so in Eclipse the option was already enabled. I add that option in the remaining bundles that we publish. I skipped the test bundles since I wouldn't worry about them concerning this issue.

[cdietrich]

does this mean also test run just with j17?
then we need to check that we dont have any java version guards in code anymore.
(JavaRuntimeVersion class)

org.eclipse.xtext.xbase.ui.testing.AbstractXbaseContentAssistTest
at least is one example for such a runtme guard

[cdietrich]

do we have a task for xtext and xtend maven plugin?

[LorenzoBettini]

@cdietrich xtext and xtend maven plugins are executed in standard builds.
Or did you mean something else?

[cdietrich]

@LorenzoBettini i mean two things

I added the property in the parent POM. I did NOT remove the source and target options, which would be redundant, because as far as I know our Maven plugins read those properties and not the release property.

which code are you referring to.

with tests i mean all tests

[LorenzoBettini]

@cdietrich I meant something else: if you look at our Maven plugins, they choose the Java compatibility level by looking at the properties maven.compiler.source and maven.compiler.target they simply ignore maven.compiler.release https://github.com/eclipse/xtext/blob/main/org.eclipse.xtext.maven.plugin/src/main/java/org/eclipse/xtext/maven/AbstractXtextGeneratorMojo.java#L93 https://github.com/eclipse/xtext/blob/main/org.eclipse.xtend.maven.plugin/src/main/java/org/eclipse/xtend/maven/AbstractXtendCompilerMojo.java#L52
that's why in the parent POM I set maven.compiler.source and maven.compiler.target besides maven.compiler.release

[cdietrich]

yes so do we need a follow up task to handle this better?

[szarnekow]

does this mean also test run just with j17?

I think it's absolutely important to still run the tests against Java11 if we claim that we are compatible to Java11. Otherwise some random 3rd party dep might sneak into our TP that is compiled to Java17 bytecode and that would go unnoticed.

[LorenzoBettini]

@szarnekow I see. However, this PR is unrelated to that. This PR only adds additional checks concerning the wrong use of API that is more recent than Java 11. Even in Eclipse, if you tried to use API from Java 17, you would immediately get a compilation error.

Additionally, I'd want to start using Tycho 3 by default. We could still configure the build job using Java 11 to override the Tycho version to 2.7.5.

[HannesWell]

Quoting from https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html

This is right for 'plain' Maven project with packaging type jar, but for projects with type eclipse-plugin the tycho-compiler-plugin usually derives this setting from the BREE or osgi.ee requirement:
tycho-compiler-plugin:deriveReleaseCompilerArgumentFromTargetLevel.

So unless there are projects with maven packaging type jar in xtext you can just remove all maven.compiler.source/target/release properties.

Additionally, I'd want to start using Tycho 3 by default. We could still configure the build job using Java 11 to override the Tycho version to 2.7.5.

It is great that you already consider to move to Tycho 3, I wanted to suggest that too. :)
But instead of changing to another Tycho version I suggest to use Maven-Toolchains instead:
https://maven.apache.org/guides/mini/guide-using-toolchains.html

For example for SWT this is set up (altough SWT is about to move to Java-17 too):

• Tycho 3.0 now requires Java 17 always eclipse-platform/eclipse.platform.swt#404
• https://github.com/eclipse-platform/eclipse.platform.swt/blob/master/.github/toolchains.xml

Something similar is probably also possible for a Jenkins workflow if you want that there too.
I can assist in setting that up.

[LorenzoBettini]

@HannesWell thanks for the feedback!

We also have pure jar projects, so the release configuration is required for those projects.

Moreover, my PR was also meant to let jdt check that in Eclipse, not only during the build.

I'll investigate also the toolchain, which I never had a chance to explore. My proposal for switching to an older Tycho when using java 11 was based on the fact that we also have a few checks in Jenkins depending on the job parameters. The GitHub actions jobs are already using java 17 only.

[HannesWell]

@HannesWell thanks for the feedback!

We also have pure jar projects, so the release configuration is required for those projects.

Yes, then this setting is totally reasonable. 👍🏽

Moreover, my PR was also meant to let jdt check that in Eclipse, not only during the build.

Yes, that is good either way :)

I'll investigate also the toolchain, which I never had a chance to explore. My proposal for switching to an older Tycho when using java 11 was based on the fact that we also have a few checks in Jenkins depending on the job parameters. The GitHub actions jobs are already using java 17 only.

Understand. Yes, I noticed that already.
Depending on what is simpler to set up, you could also consider to run the other java versions and Eclipse target-platforms in GH workflows. It could even be considered to always run all suitable combinations of TP and java version as matrix (as GH workflow or Jenkins pipeline matrix). But that's far from the topic of this PR. Lets discuss that better in a dedicated issue for that. Is there already one in this repo?
In general This is a really advanced pipeline that you created for xtext and it is really great what you did with the repo merge. Before it was (probably, I never tried it) hard to understand the xtext build. Now it is definitely possible. Great job!

[LorenzoBettini]

@HannesWell I created a dedicated issue. If, in the meantime, could you please point me to the relevant parts needed for the toolchain? I saw the XML file and it's simple, but I don't see in the SWT repository how to activate a specific toolchain. Moreover, who provides the locations for the actual JDK installations? If you have time, please comment on the other issue.

@szarnekow can we merge this one? As I said, it's just about additional checks that were missing in some projects to make sure we don't use Java API greater than 11.

[LorenzoBettini]

@cdietrich OK to merge?