Review request JDK-8015912: jdeps support to output in dot file format

[Paul Sandoz]

On Jun 13, 2013, at 6:47 AM, Mandy Chung <mandy.chung at oracle.com> wrote:

> Alan, Paul,
> 
> Thanks for the review.  Here is the revised webrev:
>  http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8015912/webrev.01
> 

Looks OK.


> See my comments inlined below.
> 
> On 6/12/2013 3:48 AM, Paul Sandoz wrote:
>> Hi Mandy,
>> 
>> JdepsTask:
>> 
>> Given that the dot graph output is likely to be just as human readable as your original format it is very tempting just to support that format. In fact it is likely to be more human readable since the output is of a known syntax. So IMHO we don't need a format option and dot graph output is sufficient.
> 
> The package names and classnames in the dot graph output are double-quote strings or the '.'  character has to be escaped.  I think it's more of a noise as the default output.

I am not really convinced the quoting is noisy, i think having two formats is noise :-)

It is still very readable:

digraph "a.jar" {
  "x.y.z.Foo" -> "a.b.c.Bar";
  ....
}

Also, did you also consider using subgraph for multiple archives?


> 
>> 
>>  636             log.format("digraph G {%n");
>> 
>> Perhaps a better name can be chosen for the title of the summary graph?
> 
> Oh yes - I change it to "jdeps summary".  Is it any better?
>> 

Yes.


>> You don't need to pass in the log PrintWriter to the OutputWriter implementations if they are non-static i.e. sometimes you use "out" and sometimes you use "log".
> 
> I agree.  Fixed.
>> 
>> PlatformClassPath:
>> 
>>   53     static boolean contains(Archive archive) {
>>   54         return javaHomeArchives.contains(archive) && !archive.getFileName().equals("jfxrt.jar");
>>   55     }
>> 
>> Just curious: why is the the check for "jfxrt.jar" required?
> 
> If jfxrt.jar is bundled with JRE, it's in the jre/lib/ext directory but it's not part of the JRE and not in any profile.  Thus it should be filtered out.
> 

An outlier! Can other stuff get placed in that dir?

Paul.