Re: Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-28 Thread Mandy Chung

> On Oct 28, 2016, at 3:03 AM, Daniel Fuchs  wrote:
> 
> Hi Mandy,
> 
> Looks good to me in general, but I feel like the new
> option --list-reduced-deps should be better documented:
> 

I agree and this details and example can be covered in the man page.

> jdeps.properties:
> 
> 152 main.opt.list-deps=\
> 153 \  --list-deps\n\
> 154 \  --list-reduced-deps   Lists the dependences and use of JDK 
> internal\n\
> 155 \APIs. --list-reduced-deps lists the 
> dependences\n\
> 156 \after transition reduction.
> 
> #1 - is it 'transition reduction' or 'transitive reduction'?
> (at two places in this file: line 156 & line 135)
> 

Good catch.  typo should be “transitive” and will fix it.
  
> #2 - could the description be made a little more verbose? something
> like:
> 
> --list-reduced-deps   lists the dependences
>  after transitive reduction.
>  Transitive reduction is obtained
>  by removing the dependencies which
>  are already transitively exported
>  by another module in the
>  dependency graph. For instance,
>  if both java.sql and java.logging
>  are included in the dependency
>  graph, then java.logging will be
>  removed because it is already
>  transitively exported by
>  java.sql, and therefore
>  requiring java.sql should be
>  enough.

Good idea.  I will expand the description.

Mandy

> 
> 
> best regards,
> 
> -- daniel
> 
> 
> On 19/10/16 23:19, Mandy Chung wrote:
>> Webrev at:
>>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167057/webrev.00/
>> 
>> This patch enhances jdeps to print the dependences in the format : 
>> $MODULE[/$PACKAGE].
>> 
>> This is intended for analyzing the regression tests we develop and add make 
>> it easy to add the proper @modules.
>> 
>> Mandy
>> 
> 



Re: Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-28 Thread Daniel Fuchs

Hi Mandy,

Looks good to me in general, but I feel like the new
option --list-reduced-deps should be better documented:

jdeps.properties:

 152 main.opt.list-deps=\
 153 \  --list-deps\n\
 154 \  --list-reduced-deps   Lists the dependences and use of 
JDK internal\n\
 155 \APIs. --list-reduced-deps lists 
the dependences\n\

 156 \after transition reduction.

#1 - is it 'transition reduction' or 'transitive reduction'?
 (at two places in this file: line 156 & line 135)

#2 - could the description be made a little more verbose? something
 like:

--list-reduced-deps   lists the dependences
  after transitive reduction.
  Transitive reduction is obtained
  by removing the dependencies which
  are already transitively exported
  by another module in the
  dependency graph. For instance,
  if both java.sql and java.logging
  are included in the dependency
  graph, then java.logging will be
  removed because it is already
  transitively exported by
  java.sql, and therefore
  requiring java.sql should be
  enough.


best regards,

-- daniel


On 19/10/16 23:19, Mandy Chung wrote:

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167057/webrev.00/

This patch enhances jdeps to print the dependences in the format : 
$MODULE[/$PACKAGE].

This is intended for analyzing the regression tests we develop and add make it 
easy to add the proper @modules.

Mandy





Re: Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-19 Thread Mandy Chung

> On Oct 19, 2016, at 4:22 PM, Alexandre (Shura) Iline 
>  wrote:
> 
> Mandy,
> 
> I have a question around line 228 of 
> src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java:
> 224 new Option(false, "--list-deps", "--list-reduced-deps") {
> 225 void process(JdepsTask task, String opt, String arg) {
> 226 task.options.showModulesAddExports = true;
> 227 task.options.reduced = opt.equals("--list-reduced-deps");
> 228 task.options.verbose = PACKAGE;
> 229 }
> 230 },
> 
> What is the expected behavior of jdeps if I use ‘—list-jdeps’ at the same 
> time as, say, ‘-v’? 
> 

-v (or other verbose options) specifies the granularity of analysis.  
-—list-deps only does package-level analysis and so verbose option has 
essentially no effect.


> Should there be more checks similar to these?
> 483 if ((options.findJDKInternals) && (options.hasFilter() || 
> options.showSummary)) {
> 484 showHelp();
> 485 return EXIT_CMDERR;
> 486 }
> 487 if (options.showSummary && options.verbose != SUMMARY) {
> 488 showHelp();
> 489 return EXIT_CMDERR;
> 490 }
> 
> Looking on already existing options, with the current implementation, for 
> ‘-s’ and -v’ options:
> ‘-v' ‘-s’ causes ‘-v' to be ignored

> ‘-s' ‘-v’ is not allowed.
> 

-s and -v are conflicting options and both cases should not be allowed.  It’s a 
bug.

> If used with ‘-jdkinternals', ‘-s’ is not allowed, while ‘-v’ is ignored.

-jdkinternals and -v and -s are conflicting options.  Can you file an issue for 
this bug?  I will need to take a pass on all options and should also consider 
subcommands.

Mandy

Re: Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-19 Thread Alexandre (Shura) Iline
Mandy,

I have a question around line 228 of 
src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java:
 224 new Option(false, "--list-deps", "--list-reduced-deps") {
 225 void process(JdepsTask task, String opt, String arg) {
 226 task.options.showModulesAddExports = true;
 227 task.options.reduced = opt.equals("--list-reduced-deps");
 228 task.options.verbose = PACKAGE;
 229 }
 230 },

What is the expected behavior of jdeps if I use ‘—list-jdeps’ at the same time 
as, say, ‘-v’? 

Should there be more checks similar to these?
 483 if ((options.findJDKInternals) && (options.hasFilter() || 
options.showSummary)) {
 484 showHelp();
 485 return EXIT_CMDERR;
 486 }
 487 if (options.showSummary && options.verbose != SUMMARY) {
 488 showHelp();
 489 return EXIT_CMDERR;
 490 }

Looking on already existing options, with the current implementation, for ‘-s’ 
and -v’ options:
‘-v' ‘-s’ causes ‘-v' to be ignored
‘-s' ‘-v’ is not allowed.

If used with ‘-jdkinternals', ‘-s’ is not allowed, while ‘-v’ is ignored.

Is this the intended behavior?

Shura


> On Oct 19, 2016, at 3:19 PM, Mandy Chung  wrote:
> 
> Webrev at:
>  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167057/webrev.00/
> 
> This patch enhances jdeps to print the dependences in the format : 
> $MODULE[/$PACKAGE].
> 
> This is intended for analyzing the regression tests we develop and add make 
> it easy to add the proper @modules.
> 
> Mandy



Review Request: JDK-8167057 jdeps to list the modules and internal APIs to help find @modules for tests

2016-10-19 Thread Mandy Chung
Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8167057/webrev.00/

This patch enhances jdeps to print the dependences in the format : 
$MODULE[/$PACKAGE].

This is intended for analyzing the regression tests we develop and add make it 
easy to add the proper @modules.

Mandy