Multiple `--add-exports` options fail with JDK 17

[parttimenerd]

Passing multiple --add-exports to the tycho-compiler fails on JDK 17 (but works on JDK 11, related to https://github.com/eclipse/tycho/commit/c53c47ffe3e39f7d3ceb7f89f6712b33d2a25662). This error can be observed when running the modified test project compiler.extraExports (#618 ), it contains

     <compilerArgs>
         <arg>--add-exports</arg>
         <arg>java.desktop/com.sun.imageio.plugins.gif=ALL-UNNAMED</arg>
         <arg>--add-exports</arg>
         <arg>java.desktop/com.sun.imageio.plugins.png=ALL-UNNAMED</arg>
      </compilerArgs>

and a class that has a property of a type from package com.sun.imageio.plugins.png.

The second --add-exports is ignored (but only on JDK 17, not on JDK 11). This can be circumvented by inserting any option between the two --add-exports options.

I observed this problem while trying to build the OpenJDK Mission Control on JDK 17.

[laeubi]

@mickaelistria @akurtakov something for 2.7 release?

[mickaelistria]

something for 2.7 release?

I wouldn't make it a blocker for 2.7 as it's not a regression. But if it's fixed soon, there is no reason why not including it in next release.

[parttimenerd]

The CI currently does run the tests on JDK 17 (as far as I can see) and therefore bugs with JDK 17 do not appear there.

[laeubi]

@parttimenerd we can add aJ17 JDK, but you should first change your test to require such one (e.g. by BREE requirement) if I remember right for example the test https://github.com/eclipse/tycho/tree/master/tycho-its/projects/compiler.jdt.bree.java8 requires a real java 8 JDK, so you might take a look there how to enforce this?

[parttimenerd]

I can look into this. But two things:

1. I think that my pull request is still valid as the current test does not properly test what it says it tests (regardless of JDK version)
2. I ran into problems running the normal test suite with maven on JDK 17 because the JDK 11 seems to be hard coded in some places

[laeubi]

You should be able to run with J11 and habe a J17 jdk in the toolchains.xml, I'll try to prepare a change to enable this for the github actions.

[parttimenerd]

I can run all tests on JDK 11.

But I do not see why we should enable my PR only on JDK 17, it is still valid with JDK 11 (and runs successfully there).

[laeubi]

See https://github.com/eclipse/tycho/pull/625/files

It hard to guess what the test checks if it never fails for the current setup. If there is a problem with Java17 the code should show this with a java17 jdk.

You can still copy the exiting test if you think its worth to test it with J11 + J17

[parttimenerd]

Ah, yes of course. Should I change my pull request accordingly?

[laeubi]

Yeah it would be helpfull, you can wait right until J 17 support is enabled, or you can do it right now to proof Java 17 tests are nt yet supported, :-)

[parttimenerd]

I think the latter would be more of my point :P

[parttimenerd]

I changed my PR accordingly (and hopefully it works)

[laeubi]

I changed my PR accordingly (and hopefully it works)

Isn't the expectation that it fails? ;-)

[parttimenerd]

It works when it fails...

[laeubi]

@parttimenerd I have now enabled a true java 17 JDK and your test case is merged. @mickaelistria @akurtakov could you take a look at https://github.com/eclipse/tycho/tree/master/tycho-its/projects/compiler.extraExports.java17 I don't understand what to do to force tycho use the JDK17 ... The test has <useJDK>BREE</useJDK> and Bundle-RequiredExecutionEnvironment: JavaSE-17

[mickaelistria]

I tried running the test with Java 17 and faced no issue.

[umairsair]

I am using SYSTEM jdk 17 but using source and target 11. Compilation is failing and it seems --add-exports are completely ignored. But if I only change source and target to 17 then it works fine.

Is it related to this issue? I am using tycho 2.7.0.

[laeubi]

@umairsair the best would be if you provide an integration-test to demonstrate the issue or at laest an example project for further investigation.

[umairsair]

project.zip

@laeubi kindly check the attached project.

[umairsair]

@laeubi ,

did you get a chance to try out the attached project?

[laeubi]

Not yet, sorry. But you could try to debug it yourself if you like, e.g. add -Xto see whats passed to the compiler and probably what @parttimenerd has suggested adding another option makes a difference in your case as well.

[akurtakov]

I have reproduced the issue using the attached project.

[akurtakov]

So far calling org.eclipse.jdt.internal.compiler.batch.Main with

[--add-exports, java.desktop/com.sun.imageio.plugins.gif=ALL-UNNAMED, --add-exports, java.desktop/com.sun.imageio.plugins.png=ALL-UNNAMED, -s, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/generated-sources/annotations, -d, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/classes, -classpath, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/classes, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/src/main/java/Foo.java, -g, -nowarn, -target, 17, -source, 17, --release, 17]

works but calling with

[--add-exports, java.desktop/com.sun.imageio.plugins.gif=ALL-UNNAMED, --add-exports, java.desktop/com.sun.imageio.plugins.png=ALL-UNNAMED, -s, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/generated-sources/annotations, -d, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/classes, -classpath, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/target/classes, /home/akurtakov/git/tycho/tycho-its/projects/compiler.extraExports.java17.target11/src/main/java/Foo.java, -g, -nowarn, -target, 11, -source, 11, --release, 11]

fails. The only difference is the target/source/release being set. This puzzles me as it goes into the JDT.core territory. @tsmaeder @rgrunber can you guide me on this one?

[iloveeclipse]

Note: according to https://github.com/eclipse-jdt/eclipse.jdt.core/issues/114#issuecomment-1162071166, combination of --release with --add-exports from a system module is not allowed. So there will be no JDT fix for this issue, only a fix that will print explicit compilation error in such case.

[laeubi]

@iloveeclipse thanks for the finding, just to be clear: Do you think this is a user error or do tycho behaves wrong here?

[iloveeclipse]

I don't know if tycho is adding the arguments automatically. The combination is bad (and if I understand Stephan right, very specific combination - exports on a system module during "release" compilation) , so whoever combined it makes a mistake. If user does it, would be good if tycho could give a hint. If tycho does it, would be good to fix it.

[RealCLanger]

Hi, I just got back here because I wanted to try with latest tycho on OpenJDK JMC project. Now I learned that using --release and --add-exports from system modules is illegal which I wasn't aware of. OK, so be it.

However, to compile successfully, we need --add-exports from system modules. I'm wondering, is there a way to configure tycho to not use the --release option?

Look at this POM

With latest tycho this doesn't build any more since tycho now requires JDK17. But even if I change java.source and java.target settings to 17, it won't compile.

I think in our case it would help if we could leave out --release and just compile with -source and -target.

[RealCLanger]

Little update on that one: With <useJDK>BREE</useJDK>, I can get it to work. I can the probably hit the else statement here