Reproduce JavaSE-17 issues on JDK 11 #951
Conversation
|
As mentioned in the discussion, this story requires a toolchains.xml to define where to find Java 17. The test would need to include that. |
|
where would I the jdkHome in the toolchains.xml point to? Locally I have this one, but the path will obviously not be valid for CI: <?xml version="1.0" encoding="UTF8"?>
<toolchains>
<toolchain>
<type>jdk</type>
<provides>
<version>1.8</version>
<id>JavaSE-1.8</id>
</provides>
<configuration>
<jdkHome>/usr/lib/jvm/java-8-openjdk-amd64</jdkHome>
<bootClassPath>
<includes>
<include>jre/lib/*</include>
<include>jre/lib/ext/*</include>
<include>jre/lib/endorsed/*</include>
</includes>
</bootClassPath>
</configuration>
</toolchain>
<toolchain>
<type>jdk</type>
<provides>
<version>17</version>
<id>JavaSE-17</id>
</provides>
<configuration>
<jdkHome>/home/aleaum/.sdkman/candidates/java/17.0.3-tem</jdkHome>
</configuration>
</toolchain>
</toolchains>I'll try and find one in the other integration tests, if there are obvious other examples I hope I find them... |
|
I forgot to re-ask / mention I did not find a similar toolchains setup. Can someone point me to a similar test (or mention the path I need to put into the toolchains file?) |
We use this approach at work:
I'm not sure what would be the best way to inject/overwrite the JDK path here with the Github actions. If all else fails, one could still write such a toolchains file via antcall or similar from within the build. |
Toolchains support property expansion, so there should be no need to modify the file itself and one can simply use ENV variables like here: https://github.com/eclipse/tycho/blob/master/toolchains-gh.xml |
|
@akurtakov as you are dedicated to Java 17 support, can you take a look here what might be the problem? |
Tycho GH actions already use the following toolchain: https://github.com/eclipse/tycho/blob/master/toolchains-gh.xml |
|
I honestly don't understand what the issue is here. Toolchain for Java-17 must be setup in order to compile using Java 17 APIs while running on Java 11 . Tycho itself will require Java 17 (#983) soon so I strongly advice switching the JVM driving the build to Java 17 and configure Java 11 in your toolchain. Announced at https://www.eclipse.org/lists/tycho-dev/msg02086.html and https://www.eclipse.org/lists/cross-project-issues-dev/msg19262.html . |
|
@akurtakov as far as I understand the issue arise when tycho is running on a lower JVM (11 here) but code should be compiled with a higher one (17 here). Maybe @aumann can give better details. |
|
/rebase |
Yes, exactly. I know that this can be worked around - still I expected this to work, and @mickaelistria confirmed my expectations here: #938 (comment) - that's why I've openend the issue. |
|
So you are running Tycho with Java 11, have toolchains configuration for Java17, tycho uses Java 17 and build fails ? |
|
Just to be sure, does it happen with Tycho 3.0.0-SNAPSHOT? I ask that question as if you're using old tycho version it will come with old ecj/jdt.core version which may not support Java 17 constructs yet. |
Yes, that's right.
I can try today in the evening - until now, I've only tested upto |
|
/rebase |
|
@aumann can you please give exact steps to reproduce:
|
I'm using the project as committed here, and execute maven from the it-project root: Full output
|
|
@laeubi is this one fixed now? |
|
@aumann Would you please sign https://www.eclipse.org/legal/ECA.php ? Without it we can not accept patch from you. |
|
I don't think so @aumann has provided steps to reproduce but I can't tell if this is an issue in Tycho or EJC ... |
I've re-signed with the correct mail address. |
akurtakov
force-pushed
the
reproduceBree17On11Bug
branch
from
d734cf2
to
f6e82c6
Compare
|
@akurtakov any chance we can finish this for 3.0? Is this an issue with Tycho or with EJC? |
Change-Id: Ib9180671e66d5c004b3b1083d0436cfcbae4d625
laeubi
force-pushed
the
reproduceBree17On11Bug
branch
from
f6e82c6
to
2fe1a59
Compare
|
Well, it's probably time to move Tycho to require Java 17 actually. The jdt bug that forced this revert is supposed to be fixed now. |
I think we should first fix all Java 17 related issue to be sure people can still compile for older versions and never versions... |
|
This one happens when running on Java 11 but targetting Java 17 which would no longer be relevant if Tycho requires Java 17 as runtime JVM. |
But Java 19 and so on will be, so this should be fixed first before we move on to a never JVM that possible hides an issue with compiling with higher JVMs thath those the Tycho build is run with. |
|
I highly doubt that is true (esp. without a reproducer). My look into jdt.core points to isJRE12Plus as the most likely cause so if requiring Java 17 I think this bug will not be hidden but the combination it happens to be no longer supported. |
|
So if its a JDT Bug we should report it there. |
|
@iloveeclipse can you help investigate if this should be supported by EJC or Tycho si doing something wrong? |
What I don't get is that the code should actually be compiled with the Java 17 JVM so why should it matter what is the "Tycho jvm"? Because I can happily run my eclipse with java 11 and use j17 jvm to compile ... |
|
@aumann can you rebase this on top of current master branch? |
Forget about that... I have already rebased this :-) But it seems the test now succeeds, so if jenkins also succeeds I think we can merge this as fixed? |
I'm not a maven expert and have no idea how translate maven config to the ECJ command line. Can you please update bug description and explain what is the problem and in particular, what part of the problem is related to ECJ and how? Ideally with a simple example input arguments / ecj command line / observed output / expected output etc. |
There is no test run. This PR is just a project to use while testing locally. |
|
IMHO compiling Java 17 while running Tycho with Java 11 shouldn't even be smth that we consider supporting. If one needs newer JVM features, one should run Tycho with that or newer. I at least don't plan to spend time on it. |
Why not? Maven even support compiling Java 17 when running with Java 8, so I don't see any relation here... And as @mickaelistria also indicated it "should" work, there must be something wrong in the setup of the project (I don't see any?) or a bug in Tycho, or maybe something wrong in EJC ... |
|
@aumann can you integrate the example into the integration test suite? |
It just doesn't climb enough on the important bug/feature scale. If your code requires newer JVM features it should be fine to run your tool with that version too. |
|
It looks like the JDT compiler doesn't support this case anyway: @iloveeclipse is this intended? I personally don't mind if it's a rule that to compile for X, you need the compiler to run a Y>=X, it makes a lot of sense. |
|
I honestly don't know, but I would expect that ecj should be able to compile with Java 17 target if running on 11 if it would know where Java 17 SDK files are. Other way around it works because modern Java 9+ SDK bring stubs of all older releases in ct.sym file, so compiler knows where to find "Record" etc. |
|
Thanks for the hint @iloveeclipse , I found that passing |
At least as mentioned I'm running my IDE with Java 11 and can compile for Java 17 and Java 19 without a problem
That's what the mentioned |
|
By the way, Tycho has even two modes:
so even if ecj would not work for JVMs < running JVM Tycho could still fall back to "ompile in extra process" with the given (higher) JVM from toolchains and I think that is how maven-compiler works and why (maven) people complain that it is slow to use toolchains :-) S the ability of JDT to "cross-compile" using different higher/lower jvms is really a great benefit! |
|
Unfortunately, the compile in extra process mode (fork=true on tycho-compiler-plugin) does not work. However, a custom JAVA_HOME is supposed to be provided for that case, and should result into something similar. So I tried something So this demonstrates that -unlike javac- the JAVA_HOME setting is not sufficient for ECJ to use a different
|
Yes I think that's missing and we had some strange behavior in the past, so I think this actually is a good show-case to improve Tycho then! |
|
unfortunately, this test cannot be easily made to execute properly on CI, so I'm closing it as not actionable. |
This is only reproducing issue #950 when run locally with JAVA_HOME set to a JDK-11.
If you point me to a similar test I can try and complete the test setup at least (i.e. prepare for this test running, and running on Java 11).