RFR: 8164805: Fail to create a MR modular JAR with a versioned entry in base-versioned empty package

[Mandy Chung]

> On Oct 19, 2016, at 11:34 AM, Steve Drach <steve.drach at oracle.com> wrote:
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8164805
> webrev: http://cr.openjdk.java.net/~sdrach/8164805/webrev.00/ 

Issue a warning is good in the case public classes are added in a concealed package.  Some comments:

Main.java

addExtendedModuleAttributes does not seem to be the appropriate method to initialize concealedPackages.  It would be better to set concealedPackages in a separate method that will be called for each modular JAR.

Validator.java
 160                     main.error(Main.formatMsg("error.validator.concealed.public.class", entryName));

This is a warning.  You should add a new warn method to make it clear.
Similiarly,  “error.validator.concealed.public.class” should be renamed to “warn.validator.concealed.public.class”.  I notice there are several keys with “error.validator” prefix that are meant for warnings.  It’d be good to  change them too.  

 247         if (idx > -1) {
 248             String pkgName = internalName.substring(0, idx).replace('/', '.');
 249             if (main.concealedPackages.contains(pkgName)) {
 250                 return true;
 251             }
 252         }

Alternative impl for the above lines:

String pkgName = idx != -1 ? internalName.substring(0, idx).replace('/', '.’)
			   : “”;
return main.concealedPackages.contains(pkgName);

jar.properties

112 error.validator.concealed.public.class=\
 113          warning - entry {0} is a public class in a concealed package, placing this \
 114          jar on the class path will result in incompatible public interfaces

line 113 needs new line ‘\n’.  You may break it into 3 lines
since the entry name will take up some characters.

ConcealedPackage.java test

  60         Path destination = userdir.resolve("classes”);

I suggest to use Paths.get(“classes”) rather than ${user.dir}.  jtreg will clean up the JTwork/scratch directory after the test run.

  63         // add an empty directory
  64         Files.createDirectory(destination.resolve("p").resolve("internal"));

I suggest to take this out.  The test verifies if "jar tf mmr.jar” succeeds.

The test should test both “cf” and “uf” and verify that the ConcealedPackage attributes in module-info.class is updated correctly (one single module-info.class case in the root entry and two module-info.class - root and versioned).

  92     private int jar(String cmd) {
Nit: this can simply take a varargs and no need to split:
  jar(String… options)


  76     private String files(Path start) throws IOException {
Perhaps return Stream<String>.  That can replace the unnecessary concat in a String and then later split to pass to javac.

Mandy