RFR JDK-8159206 - All jlink tests failing

[Mandy Chung]

> On Jun 10, 2016, at 12:20 PM, Jim Laskey (Oracle) <james.laskey at oracle.com> wrote:
> 
> http://cr.openjdk.java.net/~jlaskey/8159206/webrev/index.html <http://cr.openjdk.java.net/~jlaskey/8159206/webrev/index.html>https://bugs.openjdk.java.net/browse/JDK-8159206 <https://bugs.openjdk.java.net/browse/JDK-8159206>
> 


ResourceFilter.java - since you are in this file, we could take the opportunity to improve the code.

54     public ResourceFilter(String[] patterns, boolean exclude) throws IOException {

ResourceFilter is constructed by several plugins with this pattern:
   predicate = new ResourceFilter(Utils.listParser.apply(exclude), true);

or IncludeLocalesPlugin creates the filter with exclude=false:
   predicate = new ResourceFilter(Utils.listParser.apply(value), false);

I suggest to change ResourceFilter to define static factory methods such as:
   static ResourceFilter excludeFilter(List<String> patterns)
   static ResourceFilter includeFilter(List<String> patterns)

Alss Util::listParser returns a List<String> instead of String[] and return an empty list when argument == null.

This pattern is used in multiple places.
private static final FileSystem JRT_FILE_SYSTEM = Utils.jrtFileSystem();
PathMatcher matcher = Utils.getPathMatcher(JRT_FILE_SYSTEM, line);

You can use Utils::getPathMatcher(String pattern) and cache JRT_FILE_SYSTEM in Utils class.

  62                     if (file.exists()) {

Should it fail if the file does not exist?

test/tools/jlink/plugins/CompressorPluginTest.java
 106                 "*Exception.class,^*IOException.class");

The original test has includes and excludes.  Perhaps you can use regex syntax here.  New test cases with regex: and glob: would be good to add (the existing tests cover the default without specifying the syntax).

I also reviewed this patch together with JDK-8158402 which causes this regression:
  http://cr.openjdk.java.net/~jlaskey/8158402/webrev

My suggestion on the help message 

$ jmod —-help

  --exclude <pattern>                Exclude files, given as a PATTERN    
  --hash-modules <pattern> 
 
The default for exclude pattern is glob-syntax pattern whereas hash-modules is a regex pattern of module names.  It may be okay to overload —-exclude option to accept “regex:<pattern>” and the description needs to be made clear.  Alternative is to have a different option to take regex. 

Maybe 
—-exclude [syntax:]<pattern> 
--hash-modules <pattern>

and specify the valid syntax.

$ jlink —-list-plugins

Plugin Name: exclude-files
Option: --exclude-files=<files to exclude | files of excluded files>
Description: Specify files to exclude. e.g.: *.diz, /java.base/native/client/*

Plugin Name: exclude-resources
Option: --exclude-resources=<resources to exclude | file of excluded resources>
Description: Specify resources to exclude. e.g.: *.jcov, */META-INF/*

Plugin Name: order-resources
Option: --order-resources=<paths in priority order | file with resource paths>
Description: Order resources. e.g.: */module-info.class,/java.base/java/lang/*

The option and examples need update to use syntax:pattern and states what are valid syntax and the default.

$ jimage —-help

  --filter                          Filter entries for list or extract
                                    Ex. /java.base/*, */module-info.class

should be 
   —-filter [syntax:]pattern>

I assume you meant to give example “/java.base/**” to list all entries in java.base.  Maybe show a regex example.

Notice that jlink uses “=“ as the option separator whereas jmod and jimage use space - just to mention it and CLI option consistency is on the list to be examined.

Mandy