Review Request: JDK-8171418: Remove jdeps internal --include-system-modules option

[Mandy Chung]

> On Dec 19, 2016, at 10:26 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> On 19/12/16 18:06, Mandy Chung wrote:
>> Updated webrev:
>> 
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.01
> 
> Looks good to me.
> A related question is whether the tokens ALL-SYSTEM, ALL-DEFAULT,
> ALL-MODULE-PATH, should appear in the jdeps help message?
> 

I’ll leave it as is.  Clean it up when we do another pass on the help message.

Mandy

> best regards,
> 
> -- daniel
> 
>> 
>>> On Dec 19, 2016, at 9:54 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
>>> 
>>> Hi Mandy,
>>> 
>>> DepsAnalyzer.java:
>>> 
>>> 232         Collection<Module> modules = configuration.getModules().values();
>>> 
>>> I don't see where modules is used in this method.
>>> Should this line be removed?
>>> 
>> 
>> Removed. Lance also caught that.
>> 
>>> 
>>> 163         if (filter.requiresFilter().isEmpty()) {
>>> 164             return archives.stream()
>>> 165                 .filter(this::include)
>>> ...
>>> 168         } else {
>>> 169             // use the archives that have dependences and not specified in --require
>>> 170             return archives.stream()
>>> 171                 .filter(source -> !filter.requiresFilter().contains(source.getName()))
>>> 172                 .filter(this::include)
>>> ...
>>> 
>>> and later:
>>> 
>>> 266     public boolean include(Archive source) {
>>> 267         Module module = source.getModule();
>>> 268         // skip system module by default
>>> 269         return  !module.isSystem()
>>> 270                     || configuration.rootModules().contains(source)
>>> 271                     || filter.requiresFilter().contains(module.name());
>>> 272     }
>>> 
>>> 
>>> If I look at how JdepsFilter::includes was implemented, and given that
>>> filter.requiresFilter().contains(module.name()) will always be false
>>> if we come from line 165, or from line 172 then I would suggest to
>>> remove line 271: this would ensure that the call at line 95 does
>>> the same than before.
>>> 
>> 
>> That’s a good suggestion.
>> 
>> Mandy
>> 
>>> 
>>> On 19/12/16 05:42, Mandy Chung wrote:
>>>> Webrev at:
>>>>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8171418/webrev.00
>>>> 
>>>> --include-system-modules option was a temporary option added in an early stage of developement.  Time to remove it as a clean up.
>>>> 
>>>> Mandy
>>>> 
>>> 
>> 
>