Skip to content

Ignore module-info.java if on jdk1.8 or earlier.#2037

Merged
JaroslavTulach merged 3 commits into
apache:masterfrom
errael:master
Mar 31, 2020
Merged

Ignore module-info.java if on jdk1.8 or earlier.#2037
JaroslavTulach merged 3 commits into
apache:masterfrom
errael:master

Conversation

@errael
Copy link
Copy Markdown
Contributor

@errael errael commented Mar 19, 2020

Right now, when the JavaFX archetypes are used on JDK8, the whole source code is rendered as incompatible. The problem is that the bootclasspath is empty on JDK8 if there is module-info.java file. This PR ignores module-info.java with jdk1.8 or earlier. With this PR NetBeans show proper code completion for files App.java and SystemInfo.java.

@errael
Copy link
Copy Markdown
Contributor Author

errael commented Mar 19, 2020

@jtulach could you review?

Comment thread java/maven/src/org/netbeans/modules/maven/classpath/ClassPathProviderImpl.java Outdated
Comment thread java/maven/src/org/netbeans/modules/maven/classpath/ClassPathProviderImpl.java Outdated
@junichi11 junichi11 added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Mar 19, 2020
@errael errael requested a review from junichi11 March 20, 2020 00:10
@JaroslavTulach JaroslavTulach changed the title Ingore module-info.java if on jdk1.8 or earlier. Ignore module-info.java if on jdk1.8 or earlier. Mar 20, 2020
if (found != null) {
String sourceLevel = SourceLevelQuery.getSourceLevel(found);
if (sourceLevel != null && sourceLevel.startsWith("1.")) { //NOI18N
found = null;
Copy link
Copy Markdown

@JaroslavTulach JaroslavTulach Mar 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to cover this change with a test. I am not sure if java.api.common tests are actually executed by travis, but following would be my test:

diff --git a/java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/classpath/ModuleClassPathsTest.java b/java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/classpath/ModuleClassPathsTest.java
index b30f8246bdb7..1c0ba04a674a 100644
--- a/java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/classpath/ModuleClassPathsTest.java
+++ b/java/java.api.common/test/unit/src/org/netbeans/modules/java/api/common/classpath/ModuleClassPathsTest.java
@@ -46,6 +46,7 @@ import java.util.stream.Collectors;
 import java.util.zip.ZipEntry;
 import javax.lang.model.element.ModuleElement;
 import javax.swing.event.ChangeListener;
+import org.junit.Assume;
 import org.netbeans.api.annotations.common.CheckForNull;
 import org.netbeans.api.annotations.common.NonNull;
 import org.netbeans.api.annotations.common.NullAllowed;
@@ -63,11 +64,14 @@ import org.netbeans.junit.NbTestCase;
 import org.netbeans.modules.java.api.common.TestJavaPlatform;
 import org.netbeans.modules.java.api.common.TestProject;
 import org.netbeans.modules.java.api.common.project.ProjectProperties;
+import org.netbeans.modules.java.classpath.SimpleClassPathImplementation;
 import org.netbeans.modules.java.j2seplatform.platformdefinition.Util;
 import org.netbeans.modules.java.source.BootClassPathUtil;
 import org.netbeans.modules.parsing.api.indexing.IndexingManager;
 import org.netbeans.spi.java.classpath.ClassPathFactory;
+import org.netbeans.spi.java.classpath.ClassPathImplementation;
 import org.netbeans.spi.java.classpath.ClassPathProvider;
+import org.netbeans.spi.java.classpath.PathResourceImplementation;
 import org.netbeans.spi.java.queries.CompilerOptionsQueryImplementation;
 import org.netbeans.spi.project.support.ant.AntProjectHelper;
 import org.netbeans.spi.project.support.ant.EditableProperties;
@@ -178,6 +182,44 @@ public class ModuleClassPathsTest extends NbTestCase {
         assertEquals(expectedURLs, resURLs);
     }
 
+    public void testModuleInfoInJDK8Project() throws IOException {
+        assertNotNull(src);
+        createModuleInfo(src, "ModuleInfoDebris"); //NOI18N
+        setSourceLevel(tp, "1.8");   //NOI18N
+        final ClassPath base = systemModules == null ? ClassPath.EMPTY : systemModules;
+        final ClassPathImplementation mcp = ModuleClassPaths.createModuleInfoBasedPath(
+            base,
+            src,
+            base,
+            ClassPath.EMPTY,
+            null,
+            null
+        );
+        List<? extends PathResourceImplementation> resources = mcp.getResources();
+        assertEquals("No resources found as module-info.java is ignored: " + resources, 0, resources.size());
+    }
+
+    public void testModuleInfoInJDK11Project() throws IOException {
+        if (systemModules == null) {
+            System.out.println("No jdk 9 home configured.");    //NOI18N
+            return;
+        }
+
+        assertNotNull(src);
+        createModuleInfo(src, "ModuleInfoUsed"); //NOI18N
+        final ClassPath base = systemModules;
+        final ClassPathImplementation mcp = ModuleClassPaths.createModuleInfoBasedPath(
+            base,
+            src,
+            base,
+            ClassPath.EMPTY,
+            null,
+            null
+        );
+        List<? extends PathResourceImplementation> one = mcp.getResources();
+        assertEquals("One resource found as module-info.java is used: " + one, 1, one.size());
+    }
+
     public void testModuleInfoBasedCp_SystemModules_in_NamedModule() throws IOException {
         if (systemModules == null) {
             System.out.println("No jdk 9 home configured.");    //NOI18N

I was able to successfully run it on JDK8:

java/java.api.common$ JAVA_HOME=$HOME/bin/jdk1.8.0 ant test -Dtest.includes=**/ModuleClassPathsTest.class

as well as on JDK11:

java/java.api.common$ JAVA_HOME=$HOME/bin/jdk-11 ant test -Dtest.includes=**/ModuleClassPathsTest.class -Dtest.nbjdk.home=$HOME/bin/jdk-11 -Dtest.run.args=--limit-modules=java.base,java.logging,java.xml,java.prefs,java.desktop,java.management,java.instrument,jdk.zipfs,java.scripting,java.naming -Dtest.bootclasspath.prepend.args=-Dno.netbeans.bootclasspath.prepend.needed=true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have successfully run the test on JDK8 using the command you show.
But for JDK11 my setup must have problems. I get lots of the following errors (many more of the 2nd)

    [junit] SEVERE: null
    [junit] java.io.IOException: Do not know where to store build.properties; must set netbeans.user!

and

    [junit] Testcase: testPatchModuleWithDuplicates(org.netbeans.modules.java.api.common.classpath.ModuleClassPathsTest):   Caused an ERROR
    [junit] class "com.sun.tools.javac.code.Scope$WriteableScope"'s signer information does not match signer information of other classes in the same package
    [junit] java.lang.SecurityException: class "com.sun.tools.javac.code.Scope$WriteableScope"'s signer information does not match signer information of other classes in the same package
    [junit]     at java.base/java.lang.ClassLoader.checkCerts(ClassLoader.java:1150)
    [junit]     at java.base/java.lang.ClassLoader.preDefineClass(ClassLoader.java:905)

I guessed it wanted a pointer to a userdir (there is a build.properties in there), so I tried the userdir I use when I run the repo build. I did both
-Dnetbeans.user=C:/.../userdir on the command line and netbeans.user=C:/.../userdir in user.build.properties. Neither worked.

For the time being I'm assuming the signature errors are because of netbeans.user not set. If that's not the case, that's the next hurdle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Do not know where to store build.properties error seems harmless on Linux - the execution continues. Doing ant test-generate-html then shows no errors in the test charts.

I haven't seen the SecurityException on Linux. This is where @jlahoda has to step in. The javac situation (e.g. JDK11, JDK14, JDK... and nbjavac) is complicated (as recently discussed on mailing list). Setting things up to run on Linux properly hasn't been not easy for me either. I can only guess what can be wrong on Windows. The only three poorman's ideas I have: debug it, suppress security manager, remove the signing information from the JAR. Possibly also verify how the tests behave on Windows before the applying the changes - if they are broken, then it makes little sense to solve the problem in this issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll investigate the JDK 11 problem later. But I wonder if this change is needed at all? I mean - this is all in a section where system module are available, and the resulting ClassPath should be more or less reasonable even if the module-info is considered, shouldn't it?

@errael errael requested a review from JaroslavTulach March 20, 2020 19:08
@errael errael requested a review from jlahoda March 29, 2020 20:10
Copy link
Copy Markdown
Contributor

@jlahoda jlahoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I guess.

@JaroslavTulach JaroslavTulach merged commit 77bb92d into apache:master Mar 31, 2020
JaroslavTulach pushed a commit that referenced this pull request Apr 1, 2020
Update JavaFX archetypes for recent JavaFX/maven PRs #2036,#2037
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants