You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2020/12/17 15:23:10 UTC

[GitHub] [maven-compiler-plugin] bmarwell commented on a change in pull request #36: [MCOMPILER-447] Unable to compile modularized test for a multi-version jar that is not a java module by default

bmarwell commented on a change in pull request #36:
URL: https://github.com/apache/maven-compiler-plugin/pull/36#discussion_r545168225



##########
File path: src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java
##########
@@ -27,11 +27,13 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.Iterator;

Review comment:
       Please fix this breaking issue: 
   `Error:  src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java:[30,8] (imports) UnusedImports: Unused import - java.util.Iterator.`
   

##########
File path: src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java
##########
@@ -319,7 +352,42 @@ else if ( Double.valueOf( getTarget() ) < Double.valueOf( MODULE_INFO_TARGET ) )
         if ( testModuleDescriptor != null )
         {
             modulepathElements = testPath;
-            classpathElements = Collections.emptyList();
+            classpathElements = null;
+            for ( String path : modulepathElements )
+            {
+                File file = new File( path );
+                if ( file.exists() && ! file.isDirectory() && file.getName().endsWith( ".jar" ) )
+                {
+                    // check if the jar file contains the module-info.class
+                    try
+                    {
+                        JarFile jarFile = new JarFile( file );
+                        if ( jarFile.getJarEntry( "module-info.class" ) == null )
+                        {
+                            if ( getLog().isDebugEnabled() )
+                            {
+                                getLog().debug( "Add non-module jar dependency " + path
+                                        + " to class path." );
+                            }
+                            if ( classpathElements ==  null )
+                            {
+                                classpathElements = new ArrayList<String>();

Review comment:
       Please use the diamond operator available in Java 7.

##########
File path: src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java
##########
@@ -319,7 +352,42 @@ else if ( Double.valueOf( getTarget() ) < Double.valueOf( MODULE_INFO_TARGET ) )
         if ( testModuleDescriptor != null )
         {
             modulepathElements = testPath;
-            classpathElements = Collections.emptyList();
+            classpathElements = null;
+            for ( String path : modulepathElements )
+            {
+                File file = new File( path );
+                if ( file.exists() && ! file.isDirectory() && file.getName().endsWith( ".jar" ) )
+                {
+                    // check if the jar file contains the module-info.class
+                    try
+                    {
+                        JarFile jarFile = new JarFile( file );
+                        if ( jarFile.getJarEntry( "module-info.class" ) == null )
+                        {
+                            if ( getLog().isDebugEnabled() )
+                            {
+                                getLog().debug( "Add non-module jar dependency " + path
+                                        + " to class path." );
+                            }
+                            if ( classpathElements ==  null )
+                            {
+                                classpathElements = new ArrayList<String>();
+                            }
+                            // add the path to the classpathElements list.
+                            classpathElements.add( path );
+                        }

Review comment:
       Can you refactor the code a bit? Too many levels of indentation.

##########
File path: src/main/java/org/apache/maven/plugin/compiler/TestCompilerMojo.java
##########
@@ -227,6 +229,37 @@ protected void preparePaths( Set<File> sourceFiles )
             }
         }
 
+        // It is possible that we are building a multi-version jar file in which the main classes are not module based.
+        if ( ! mainModuleDescriptorClassFile.exists() && release != null )
+        {
+            int version = Integer.valueOf( release );
+            while ( version >= 9 )
+            {
+                String releaseOutputDirectory = String.format( "%s%sMETA-INF%sversions%s%d",
+                        getProject().getBuild().getOutputDirectory(), File.separator, File.separator,
+                        File.separator, version );

Review comment:
       Please use `File metaInfDir = new File(getProject().getBuild().getOutputDirectory(), "META-INF/versions/" + version)`. It will do the conversion  to system-specific separators for you.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org