8154956: Module system implementation refresh (4/2016)

[Claes Redestad]

Lots of good changes here!

I've skimmed the changes and have some comments and minor things (feel 
free to ignore everything for this refresh):

http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/java.base/share/classes/java/lang/module/ModulePath.java.udiff.html

String fn is potentially calculated twice and could be moved to directly 
after the first if (also gets rid of the odd looking "} if" construct):

diff -r 930fcc6b74c8 
src/java.base/share/classes/java/lang/module/ModulePath.java
--- a/src/java.base/share/classes/java/lang/module/ModulePath.java Fri 
Apr 29 18:47:24 2016 -0700
+++ b/src/java.base/share/classes/java/lang/module/ModulePath.java Sat 
Apr 30 16:03:03 2016 +0200
@@ -267,8 +267,9 @@

              if (attrs.isDirectory()) {
                  return readExplodedModule(entry); // may return null
-            } if (attrs.isRegularFile()) {
-                String fn = entry.getFileName().toString();
+            }
+            String fn = entry.getFileName().toString();
+            if (attrs.isRegularFile()) {
                  if (fn.endsWith(".jar")) {
                      return readJar(entry);
                  } else if (fn.endsWith(".jmod")) {
@@ -279,7 +280,6 @@
              }

              // skip hidden files
-            String fn = entry.getFileName().toString();
              if (fn.startsWith(".") || Files.isHidden(entry)) {
                  return null;
              } else {

On line 448 in the same file a Set (serviceNames) is created from a 
stream just to iterate over it and could be replaced by 
distinct().forEach(...)

http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/java.base/share/classes/java/lang/module/ModuleReference.java.udiff.html

hashCode should always store a non-zero value to hash to avoid 
recalculation in degenerate cases, e.g., if the calculated hash is 0, 
store -1.

http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/jdk.jartool/share/classes/sun/tools/jar/Main.java.udiff.html
http://cr.openjdk.java.net/~alanb/8154956/1/jdk/src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java.udiff.html

This adds quite a bit of code duplication between these tools - 
transpose() etc - could (some of) this be refactored to a shared utility?

Thanks!

/Claes

On 2016-04-29 14:38, Alan Bateman wrote:
>
> There have several changes in jake that we need to bring into 
> jdk9/dev. The main changes are:
>
> 1. The policy described in JEP 261 for the root modules to resolve 
> when compiling code in the unnamed module or where the main class is 
> loaded from the class file. This is a disruptive change and we need to 
> get through the transition.
>
> Related is a new token `ALL-DEFAULT` that can be specified to 
> `-addmods` to resolve the same roots when the initial module is a 
> named module. This will eventually replace `ALL-SYSTEM` but we can't 
> remove that just yet. The launchers for javac, javadoc, jlink and a 
> few other tools that load arbitrary code in unnamed modules are now 
> compiled with this option.
>
> 2. The transition to the new form of -Xpatch. This is mostly changes 
> in hotspot but there are changes in other repos too to drop or replace 
> the old form of -Xpatch (boot cycle builds for example).
>
> 3. Removal of the old form of -XaddExports and -XaddReads. This has 
> build + test changes.
>
> 4. The second phase of integrity hashing. With this phase then the 
> build records in java.base the hashes of the non-upgradeable modules. 
> This prevents accidental linking of standard/JDK modules from 
> different JDK builds. The jar and jmod tools have updated options to 
> support this.
>
> 5. Peter Levart's patch to replace the internal WeakSet in jlr.Module 
> with a WeakKeyPair.
>
> 6. Updates to jlink option handling. The reason this went into 
> jdk9/dev is that it is disruptive to FX packager and it's hard to 
> coordinate with FX when it's a separate forest. The packager class 
> that Chris Bensen has added to jlink will allows us to iterate on 
> jlink with reduced risk of breaking FX.
>
> There are several small bug fixes and clean-ups in several areas, 
> we'll have a lot more of these for the next update.
>
> One other thing to mention is that we've bumped the required version 
> of jtreg as jtreg relies on-Xpatch to add test cases into modules and 
> so it needed to be updated too.
>
> The webrevs, all repos are here:
>   http://cr.openjdk.java.net/~alanb/8154956/1/
>
> There are a couple of files in the webrevs that we probably won't 
> bring to JDK 9 in this update:
>
> i. We have a patch to IDL compiler in the corba repo that needs a more 
> extensive fix.
> ii. The javadoc change in ModuleFinder as there are still details to 
> decide on how modular JARs work as multi-release JARs.
>
> One other point is that the webrevs are against jdk-9+116 for now. 
> I've done a test merge + build with the current jdk9/dev forest and 
> there are only a few conflicts to fix. I will re-merge + test with 
> jdk9/dev once we have agreed the changes for this update.
>
> Finally, just to say that we'll probably continue in jake for a while 
> after we get through this update. There are several design issues on 
> the JSR issues list that will likely require a few iterations and a 
> bit of bake time before we bring them to JDK 9.
>
> -Alan