> On Jun 10, 2016, at 12:20 PM, Jim Laskey (Oracle) <james.las...@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
