tycho icon indicating copy to clipboard operation
tycho copied to clipboard

Reproducer for 'tycho-compiler-jdt unable to compile test code when module-info.java is present' / Issue 230

Open jason-faust opened this issue 2 years ago • 52 comments

Demonstration of the problem reported in issue #230

jason-faust avatar Mar 27 '22 04:03 jason-faust

@jason-faust this change seems to include a lot of unrelated changes, can you rebase it and only submit relevant changes?

laeubi avatar Apr 08 '22 07:04 laeubi

@laeubi Which branch do you want it to target? 2.7 or master?

jason-faust avatar Apr 09 '22 17:04 jason-faust

master please, if suitable we can backport it later on.

laeubi avatar Apr 09 '22 17:04 laeubi

Hopefully this works.

jason-faust avatar Apr 10 '22 00:04 jason-faust

Is this the error you are expecting? https://ci.eclipse.org/tycho/job/tycho-github/job/PR-809/2/testReport/org.eclipse.tycho.test.compiler/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/ according to the debug output java 1.7 level is used: -target, 1.7, -source, 1.7

laeubi avatar Apr 10 '22 05:04 laeubi

That is the error, but a little lower down in the output. There may also be some follow on issues with the compiler plugin passing along the source and target options when it should only be passing the release option, but that is unrelated to the current issue.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  3.099 s
[INFO] Finished at: 2022-04-10T01:12:18Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:testCompile (default-testCompile) on project c.m.m: Fatal error compiling: Unrecognized option : --patch-module -> [Help 1]

jason-faust avatar Apr 10 '22 15:04 jason-faust

Can you try to force target/source to 11 just to see if this changes anything?

laeubi avatar Apr 10 '22 16:04 laeubi

Testing locally has the same error when using source, test, and release at 11.

jason-faust avatar Apr 10 '22 16:04 jason-faust

Unit Test Results

151 files  151 suites   1h 1m 28s :stopwatch: 259 tests 255 :heavy_check_mark: 3 :zzz: 0 :x: 1 :fire:

For more details on these errors, see this check.

Results for commit 8d696d61.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Apr 10 '22 18:04 github-actions[bot]

@mickaelistria @akurtakov any one has an idea whats wrong here? Is this an EJC issue?

laeubi avatar Apr 13 '22 08:04 laeubi

Indeed, it looks like the tycho-compiler-jdt doesn't support the --patch-module option that the maven-compiler-plugin is passing it. I'm not sure what's the "wronger" here: maven-compiler-plugin passing this option, or tycho-compiler-plugin not reading it.

mickaelistria avatar Apr 13 '22 09:04 mickaelistria

So any jdt / maven compiler expert who can help here?

laeubi avatar Apr 18 '22 06:04 laeubi

For the record, here is the full commandline passed to jdt and the error message:

[INFO] Compiling 1 source file to /home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/test-classes [DEBUG] JDT compiler args: [--patch-module, ex=/home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/classes:/home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/src/test/java:/home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/generated-test-sources/test-annotations:, --add-reads, ex=ALL-UNNAMED, -s, /home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/generated-test-sources/test-annotations, -d, /home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/test-classes, -classpath, /home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/target/test-classes, /home/runner/work/tycho/tycho/tycho-its/target/projects/MavenCompilerPluginTest/testMavenCompilerPluginModuleWithTests/compiler.mavenCompilerPlugin.moduleWithTests/src/test/java/ex/ATest.java, -g, -nowarn, -target, 11, -source, 11, --release, 11] [DEBUG] Original compiler output: Unrecognized option : --patch-module

laeubi avatar Apr 18 '22 06:04 laeubi

@iloveeclipse can you tell if tycho is doing anything wrong here or if this is a user error?

laeubi avatar Aug 10 '22 04:08 laeubi

I'm not a module expert. Spec is here: https://openjdk.org/jeps/261

If you would attach source code in a project with configuration as it is expected to work plus the actual command line that triggers ECJ, it would simify debugging.

iloveeclipse avatar Aug 10 '22 05:08 iloveeclipse

The code is here: https://github.com/eclipse-tycho/tycho/tree/8d696d614a95000438b379bdeb41f2e0d9f322e7/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests The JDT compiler arguments generated by Tycho is here: https://github.com/eclipse-tycho/tycho/pull/809#issuecomment-1101136523 Original compiler output: Unrecognized option : --patch-module

If I understand @jason-faust correctly javac can compile this?

laeubi avatar Aug 10 '22 05:08 laeubi

If you would attach source code in a project with configuration as it is expected to work

I really meant exact that above. Not some random files somewhere, but as a proect with compiler settings.

iloveeclipse avatar Aug 10 '22 05:08 iloveeclipse

I'm not sure if one can transform this into an eclipse project, this uses maven to replace the javac compiler by using ejc (see here https://github.com/eclipse-tycho/tycho/blob/8d696d614a95000438b379bdeb41f2e0d9f322e7/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/pom.xml) so not a "traditional" eclipse project.

I think one can emulate this by

  1. Download the zip: https://github.com/eclipse-tycho/tycho/archive/8d696d614a95000438b379bdeb41f2e0d9f322e7.zip and extract it to e.g. /tmp/tycho-bug
  2. Create a Java main file (I just put it into the jdt.core bundle)
public class CallCompiler {

	public static void main(String[] args) {
		String basePath = "/tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/";
		Main.compile(
				"--patch-module ex=" + basePath
		                + "target/classes:" + basePath + "src/test/java:" + basePath
		                + "target/generated-test-sources/test-annotations: --add-reads ex=ALL-UNNAMED -s "
		                + basePath
		                + "target/generated-test-sources/test-annotations -d "
		                + basePath
		                + "target/test-classes -classpath " + basePath
		                + "target/test-classes " + basePath
		                + "src/test/java/ex/ATest.java -g -nowarn -target 11 -source 11 --release 11");

	}

}

laeubi avatar Aug 10 '22 05:08 laeubi

By the way javac says:

javac --patch-module ex=/tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/target/classes:/tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/src/test/java:/tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/target/generated-test-sources/test-annotations: --add-reads ex=ALL-UNNAMED -s /tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/target/generated-test-sources/test-annotations -d /tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/target/test-classes -classpath /tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/target/test-classes /tmp/tycho-bug/tycho-its/projects/compiler.mavenCompilerPlugin.moduleWithTests/src/test/java/ex/ATest.java -g -nowarn -target 11 -source 11 --release 11
error: option --source cannot be used together with --release
error: option --target cannot be used together with --release
Usage: javac <options> <source files>
use --help for a list of possible options

If I remove those, ejc still complains Unrecognized option : --patch-module but javac fails with;

error: module not found: ex
1 error

So I maybe messed up the call a bit but maybe it still helps as ejc is not getting over the parsing options stage....

laeubi avatar Aug 10 '22 05:08 laeubi

@iloveeclipse is the provided example suitable? Is this a bug I should report to JDT?

laeubi avatar Aug 28 '22 10:08 laeubi

Would you please fix the conflict with master?

akurtakov avatar Sep 15 '22 15:09 akurtakov

By the way, the documentation of maven-compiler-plugin and other discussions recommends

      <plugin>
        <artifactId>maven-compiler-plugin</artifactId>
        <configuration>
          <compilerId>eclipse</compilerId>
        </configuration>
        <dependencies>
          <dependency>
            <groupId>org.codehaus.plexus</groupId>
            <artifactId>plexus-compiler-eclipse</artifactId>
            <version>...</version>
          </dependency>
          <dependency>
            <groupId>org.eclipse.jdt</groupId>
            <artifactId>ecj</artifactId>
            <version>...</version>
          </dependency>
        </dependencies>
      </plugin>

So if this works, then I don't think Tycho should support the use-case you describe here and the jdt compiler should be treated as an internal artifact of Tycho, not to be mixed with the maven-compiler-plugin.

mickaelistria avatar Sep 15 '22 20:09 mickaelistria

So if this works, then I don't think Tycho should support the use-case you describe here and the jdt compiler should be treated as an internal artifact of Tycho, not to be mixed with the maven-compiler-plugin.

Probably its the other way round, if plexus-compiler-eclipse is a standard compiler API interface maybe Tycho should then drop its own wrapper to the compiler and use the same path as maven-compiler-plugin? If not it seems there are benefits of the Tycho one and is an equally valid choice than the suggested approach.

laeubi avatar Sep 16 '22 05:09 laeubi

@jason-faust can you please resolve the conflict and rebase on current master?

laeubi avatar Sep 16 '22 05:09 laeubi

The tycho-compiler-plugin exists because it has finer-grained access to access rules and other things that are only useful in the case of Tycho. It's not really meant to replace ecj and existing standard approach. We shouldn't support in Tycho something that is already better supported upstream.

mickaelistria avatar Sep 16 '22 07:09 mickaelistria

Probably its the other way round, if plexus-compiler-eclipse is a standard compiler API interface maybe Tycho should then drop its own wrapper to the compiler and use the same path as maven-compiler-plugin?

Ideally, yes. However, back then (and maybe not now) Tycho required some access to non-API of the compiler plugin that made it easier for it to kind of form the maven-compiler-plugin.

mickaelistria avatar Sep 16 '22 07:09 mickaelistria

@laeubi Should be in sync again.

jason-faust avatar Sep 17 '22 22:09 jason-faust

Test Results

362 files  ±0  362 suites  ±0   2h 38m 25s :stopwatch: - 4m 31s 336 tests +1  325 :heavy_check_mark: ±0  10 :zzz: ±0  0 :x: ±0  1 :fire: +1  672 runs  +2  649 :heavy_check_mark: ±0  21 :zzz: ±0  0 :x: ±0  2 :fire: +2 

For more details on these errors, see this check.

Results for commit 6a3c5868. ± Comparison against base commit afbdc648.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Sep 18 '22 14:09 github-actions[bot]

Indeed, it looks like the tycho-compiler-jdt doesn't support the --patch-module option that the maven-compiler-plugin is passing it. I'm not sure what's the "wronger" here: maven-compiler-plugin passing this option, or tycho-compiler-plugin not reading it.

@stephan-herrmann @iloveeclipse any idea on this? This seems to be plain maven with plain compiler so no special tycho involved, using mvn clean install -X should show all details like compiler arguments.

laeubi avatar Jan 22 '24 13:01 laeubi

@jason-faust sorry to ask again but now there are conflicts that need to be resolved :-\

If someone has an idea how to address this please let me know so it could be included in the upcoming bugfix release.

laeubi avatar Jan 22 '24 13:01 laeubi