Review Request for 8027481: jdeps to handle classes with the same package name ...

[Daniel Fuchs]

On 10/30/13 3:06 AM, Mandy Chung wrote:
> I have added the test cases to verify dependency on javax.crypto and
> also a class in a SE package (javax.activity in the test case).  The
> updated webrev:
> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027481/webrev.01/

Looks good. What's the purpose of the split() method in Basic.java?
It looks as if it's not used. Is it a trap for reviewers? ;-)

best regards,

-- daniel

>
> Mandy
>
> On 10/29/2013 2:37 PM, Mandy Chung wrote:
>> On 10/29/13 2:19 PM, Daniel Fuchs wrote:
>>> Hi Mandy,
>>>
>>> The changes look good to me - though I am a novice in this area:
>>> so I could have missed a few things.
>>>
>>> As a general comment I would say that this code could benefit
>>> from some more commenting - if you don't know what it's supposed
>>> to do then it is a bit difficult to reverse-engineer :-)
>>>
>>
>> Thanks for the review, Daniel.  I have added some more comments and
>> updated the webrev per the questions you asked at IM.
>>
>> Mandy
>>
>>> best regards,
>>>
>>> (not a reviewer)
>>>
>>> -- daniel
>>>
>>>
>>> On 10/29/13 8:30 PM, Mandy Chung wrote:
>>>> Webrev at:
>>>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/8027481/webrev.00/
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8027481
>>>>
>>>> This patch fixes a couple of small jdeps issues:
>>>>
>>>> 1) jdeps doesn't handle the same package name being used in more than
>>>> one JAR files.  It currently incorrectly shows the dependencies for
>>>> example if two JAR files have classes in the unnamed package.
>>>>
>>>> The fix is to change the Analyzer class to maintain the target name
>>>> together the Archive where the dependence comes from (the new
>>>> ArchiveDeps.Dep class in this patch).
>>>>
>>>> 2) javax.crypto.* packages are in compact1 but shown as "JDK internal
>>>> API" because they are not in rt.jar and thus not included in ct.sym.
>>>> The
>>>> simple fix is to special it in jdeps rather than changing ct.sym to
>>>> work
>>>> with multiple jars (which will impact javac)
>>>>
>>>> 3) -verbose prints all class dependencies including those within the
>>>> same JAR file. Having the summary.dot file to show package-level
>>>> summary
>>>> would be more useful for analysis when -verbose is specified.
>>>>
>>>> I also added an experimental option to label the edges in the
>>>> summary.dot with the dependencies to make it easier to view the graph.
>>>> It's useful when it has managable amount of dependencies displayed in
>>>> the graph and not sure how well the dot tool can render. So I keep it
>>>> experimental for now.
>>>>
>>>> thanks
>>>> Mandy
>>>
>>
>