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

[Daniel Fuchs]

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?


  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.

best regards,

-- daniel



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
>