Fix multi-release support when module-info is not in the base class #948
Conversation
…ersions on both the class-path and module-path for versions before the version wbere `module-info.java` is introduced.
…ath and module-path. Instead, ensure that the `--module-source-path` option is used for compiling modules.
|
There is still a problem in some projects. Once you reach the phase that the compilation is modular, you have to add ALL dependencies on --module-path instead of --class-path. If not, dependent modules are not found. |
|
Do you mean that there is some non-modular dependencies that are on the class-path, and the code in those dependencies are no found? |
No, when you suddenly consider that whole thing as modular project because of the module-info.java, all its module dependencies must be on the --module-path. Currently, all the dependency artifact from the pom.xml are on the --class-path. |
|
Is the following a correct description of the scenario?
|
|
OK, just an example. Look for yourself. Check out https://github.com/eclipse-ee4j/yasson/releases/tag/3.0.4 You will have as a result: |
|
the dependencies of that module will be on --class-path in target/javac.args. Change that to --module-path and the compilation will succeed. |
|
Thanks for the link, I will test right now. What I'm trying to figure out is why the dependency is still on the class-path. |
for `module-info` in previous versions of a multi-release project.
|
Pushed a fix to this pull request. It was indeed a bug in the plugin, which was not detecting that the project is modular when compiling for versions other than the base version, because the Note that this bug existed in the previous approach (putting everything on the class-path) as well, but it was unnoticed. I presume it was because putting everything on the class-path allowed If there is other projects failing, please send me their links too. |
|
Now, with this change, this whole thing is compiling all my test-set correctly. Besides one package, but that is due to the plexus-compiler-api -> javax.tools.JavaCompiler migration and not sure we can do anything useful there. But I see one problem due to the incrementalCompilation. I will expose it in the next comment including the reproducing steps. |
|
Ok, the reproducer that shows problems (that might be not in the scope of this PR) is following: |
|
But, this might not be really the problem of this particular PR. The changes in incremental compilation messed up quite a number of things. |
|
Thank for the link. I have done a step by step debugging. The issue does not seem to come from the incremental compilation. It comes from the execution of the following plugin in the <plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<version>3.0.0</version>
<executions>
<execution>
<phase>test</phase>
<goals>
<goal>add-source</goal>
</goals>
<configuration>
<!-- Necessary to add sources to the source-jar -->
<sources>
<source>src/main/java-mr/9</source>
<source>src/main/java-mr/16</source>
</sources>
</configuration>
</execution>
</executions>
</plugin>Below is the list of steps executed by Maven with my analysis after the ⟵ symbol:
Therefore, the plugin seems to work as instructed. The execution of The only impact of incremental build is that it correctly detects that all the files in the Java 8 directory were already compiler, and think that the files in the Java 9 and 16 directories are new. Therefore, the If this compilation worked with Maven 3 plugin, we would need to run in a debugger in order to understand how it could be. But I suspect that it may be because of some fragile combination of circumstances. |
|
Personally I also think that this is not really maven-compiler-plugin problem. Sometimes you will have lower quality pom.xml files that somehow worked before and now they will not. But there is not a contract of being bug-to-bug compatible. I see from my point of view this PR as pretty mature to get in. Not like my opinion matters :) |
|
Your opinion matter, you identified real bugs and your test cases helped a lot! Regarding the above issue, a workaround is to add the following block in the compiler plugin executions: <execution>
<id>default-compile</id>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<release>8</release>
<compileSourceRoots>
<compileSourceRoot>${project.basedir}/src/main/java</compileSourceRoot>
</compileSourceRoots>
</configuration>
</execution>It causes the plugin to ignore the sources managed by Maven core and use the directory specified in However, when doing that, we hit another issue with the symbolic link created for redirecting |
…ic Maven 3 behaviour). The previous approach using symbolic links caused the same files to appear in two locations, which was another source of confusion.
What I meant is that it does not determine whether something enters or not. |
|
I pushed a commit which replace the symbolic links by a renaming of directories before and after compilation. The symbolic links caused confusion in some circumstances, because of same files visible from two locations. With this commit and the workaround in above comment, the compilation of I will wait a little bit before to merge, in case you have a chance to test. |
|
I will port all the patches and we will know soon. Thank goodness the build is bit-to-bit reproducible and does effective tree pruning :) |
|
My tests are fine all. Everything compiles well. |
|
Thanks! I will merge now then. |
When compiling a multi-release project in the old way, put previous versions on both the class-path and module-path for versions before the version where
module-info.javais introduced.