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

org.eclipse.tycho.surefire.junit* libs should still be Java 1.8 compatible #70

Closed
sewe opened this issue Apr 15, 2021 · 7 comments · Fixed by #71
Closed

org.eclipse.tycho.surefire.junit* libs should still be Java 1.8 compatible #70

sewe opened this issue Apr 15, 2021 · 7 comments · Fixed by #71
Assignees
Milestone

Comments

@sewe
Copy link
Contributor

sewe commented Apr 15, 2021

Since Tycho 2.0, the Tycho maven plugins require Java 11. That’s fine. Unfortunately, since Tycho 2.1.0, your tests now also require Java 11, even if they themselves have a Bundle-RequiredExecutionEnvironment of JavaSE-1.8 and a toolchain is configured to launch the tests with a Java 1.8 JVM.

Ultimately, the test runtime fails to launch with an error message like this one:

!ENTRY org.eclipse.tycho.surefire.junit57 2 0 2021-04-15 12:11:12.112
!MESSAGE Could not resolve module: org.eclipse.tycho.surefire.junit57 [200]
  Unresolved requirement: Require-Capability: osgi.ee; filter:="(&(osgi.ee=JavaSE)(version=11))"

And this is unfortunate, as the org.eclipse.tycho.surefire.junit* libraries really don’t require Java 11. The only version 55.0 (aka Java 11) class file in the org.eclipse.tycho.surefire.junit57-2.3.0.jar, for example, is module-info.class. And then there are these two classes:

META-INF/versions/9/org/junit/platform/commons/util/ModuleUtils$ModuleReferenceScanner.class
META-INF/versions/9/org/junit/platform/commons/util/ModuleUtils.class

But they are, as befits a multi-version JAR, hidden away in META-INF/versions/9, so all that prevents the test runtime from still supporting Java 1.8 is a too restrictive Require-Capability declaration.

I hence propose to still declare the org.eclipse.tycho.surefire.junit*libraries as compatible with Java 1.8 – which they are, in fact. This makes it possible, using toolchains, to build your plugins with Java 11, but still test them against Java 1.8, if that’s what they need to run against in the wild.

@sewe
Copy link
Contributor Author

sewe commented Apr 15, 2021

FWIW, I am willing to create a pull request.

@laeubi
Copy link
Member

laeubi commented Apr 15, 2021

Then go ahaed I don't see any reason not to merge it if all tests still pass :-)

The problem could really be the module-info.class as the manifest is generated using BND and this uses the highest classversion by default.

@mickaelistria
Copy link
Contributor

Just to make sure: are you certain it's not profitable for your use-case to just fully use Java 11 ? Java 8 compatibility is now an extra cost, so fully getting rid of it could be beneficial for you.
If you want to build a PR for that, please make sure there is some test explicitly covering the Java 8 compatibility story, otherwise, versions may be moved again to Java 11 again later without notice.

sewe added a commit to sewe/tycho that referenced this issue Apr 15, 2021
…test runtime

The test runtime (Surefire Booter + JUnit/TestNG Runners) only contains
classes still compatible with Java 8, but unnecessarily declares a Java
11 requirement. This prevents using toolchains to test against older
Java versions.
sewe added a commit to sewe/tycho that referenced this issue Apr 15, 2021
Uses the maven-enforcer-plugin to enforce that all dependencies part of
the Surefire Booter and JUnit/TestNG Runners do not contain class files
requiring Java 9 or later. (META-INF/versions/**.class and
module-info.class are safe and ignored.)
sewe added a commit to sewe/tycho that referenced this issue Apr 15, 2021
@sewe
Copy link
Contributor Author

sewe commented Apr 15, 2021

Just to make sure: are you certain it's not profitable for your use-case to just fully use Java 11 ? Java 8 compatibility is now an extra cost, so fully getting rid of it could be beneficial for you.

We build a plug-in using Tycho that has to support older Eclipse versions as well (well, really old ones like Luna). In particular, we have to support Eclipse versions which do not even launch on Java 11. Hence, we would like to test against such older versions (easily done using an appropriate eclipse-target-definition) on old JREs (using toolchains).

Of course, as soon as JUnit drops Java 8 support, we will have to move on with our tests, at least for the toolchain part, but that hasn’t happened yet. 🤞

The problem could really be the module-info.class as the manifest is generated using BND and this uses the highest classversion by default.

I use the <enforceBytecodeVersion> enforcer rule in my pull request. It will notify you as soon as one of the upstream dependencies rebundled as part of the Surefire Booter or JUnit/TestNG Runner contains bytecode from a more recent Java version (module-info.class and multi-version JAR files’ META-INF/version/**.class excluded). You can then update the execution environment requirement accordingly.

If you want to build a PR for that, please make sure there is some test explicitly covering the Java 8 compatibility story, otherwise, versions may be moved again to Java 11 again later without notice.

Is this enough of a test case? I must admit the whole tycho-its project is a bit convoluted; in particular, I couldn’t find an integration test using a toolchain which I could use for inspiration.

@mickaelistria
Copy link
Contributor

Is this enough of a test case?

It's more up to you to consider. I know most of us won't care if this is back to Java 11, so it's all about how much this check makes you feel safe ;)

@sewe
Copy link
Contributor Author

sewe commented Apr 16, 2021

It's more up to you to consider. I know most of us won't care if this is back to Java 11, so it's all about how much this check makes you feel safe ;)

Alright. I added a comment to the bnd.bnd files about this (bf97f0b). That should prevent someone from accidentally switching back to Java 11.

And if JUnit ever decides to switch to a newer bytecode version, the maven-enforcer-plugin will warn us that we will need to update the declared execution-environment requirement.

So from my point of view, pull request #71 is good to go.

mickaelistria pushed a commit that referenced this issue Apr 16, 2021
The test runtime (Surefire Booter + JUnit/TestNG Runners) only contains
classes still compatible with Java 8, but unnecessarily declares a Java
11 requirement. This prevents using toolchains to test against older
Java versions.
mickaelistria pushed a commit that referenced this issue Apr 16, 2021
Uses the maven-enforcer-plugin to enforce that all dependencies part of
the Surefire Booter and JUnit/TestNG Runners do not contain class files
requiring Java 9 or later. (META-INF/versions/**.class and
module-info.class are safe and ignored.)
@mickaelistria
Copy link
Contributor

PR merged, thanks @sewe

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 a pull request may close this issue.

3 participants