Review request: 8003562: Provide a command-line tool to find static dependencies

[Ulf Zibis]

Some nits again:

_private_ static class Options has _public_ members, why?

IMHO, defining a distinct inner class for each Option is a little bit oversized. I also do not see 
the need for class Options, as it is only instantiated once. Instead you could define:
     enum Options
Anyway, isn'd there still an options parser from other java langtools, which could be reused ???

In many places the source is exhausting to read, e.g
         if (f.exists()) {
             ClassFileReader reader = ClassFileReader.newInstance(f);
             Archive archive = new Archive(f, reader);
             result.add(archive);
         }
could simply be:
         if (f.exists())
             result.add(new Archive(f, ClassFileReader.newInstance(f)));

... also spreading around the variable definitions at different places.

-Ulf


Am 14.12.2012 08:22, schrieb Mandy Chung:
> Updated webrev:
> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/webrev.04/
>
> The binary name of an inner class has a '$' and so ClassFileReader.java:74 should do it.  Once the 
> jdeps change is pulled down to the profiles forest, I may make the change there to use the API as 
> javac uses.
>
> thanks
> Mandy
>
> On 12/13/2012 6:15 PM, Mandy Chung wrote:
>> On 12/13/2012 4:06 PM, Jonathan Gibbons wrote:
>>> Mandy,
>>>
>>> Mostly good -- and nice to see Dependencies getting the full tool treatment.
>>>
>>
>> Thanks for the review, Jon.
>>
>>> -- Jon
>>>
>>>
>>> Archive.java:128
>>> non-localized text: "not found"
>>>
>>
>> Oh I missed that - will fix it.
>>
>>> ClassFileReader.java:74, ditto similar code in JarFileReader
>>> Will not work for inner classes.
>>>
>>
>> That's right.  What's the best way handling inner classes besides retrying?
>>
>>> ClassFileReader.java:various
>>> Langtools is (for a while longer) still using -source 6. This is to allow NetBeans to build and 
>>> use langtools. I guess the code here is OK as long as NB don't try and build all of langtools.
>>>
>>
>> We talked about this and I hope that some part of langtools could be built with -source 7 as they 
>> are not needed in the bootstrap phase.  I will fix it since it's close to integration.   That'll 
>> help when you use NB to open langtools repo; otherwise, NB will indicate the files with syntax 
>> error which is kind of annoying.
>>
>>
>>> JDepsTask:111
>>> Did you mean --summary?
>>
>> Yes will fix it.
>>
>>> If you're wanting to emulate the GNU-style --options, these options normally use =, as in 
>>> --name=value, so you might want to update handleOption.
>>>
>>
>> That's right - will fix it.
>>
>>> PlatformClassPath
>>> The API you are looking for is now available in the profiles forest, in 
>>> langtools/src/classes/com/sun/tools/javac/sym
>>
>> Good - I'll check that out and send out a new webrev.
>>
>> Thanks
>> Mandy
>>
>>>
>>> -- Jon
>>>
>>> On 12/10/2012 02:30 AM, Erik Joelsson wrote:
>>>> Looks good.
>>>>
>>>> /Erik
>>>>
>>>> On 2012-12-07 22:55, Mandy Chung wrote:
>>>>> This is the updated webrev. It includes Erik's makefile changes and
>>>>> small change - be consistent with javap, jdeps can take classnames as
>>>>> the input argument or wildcard "*" to analyze all class files that
>>>>> replaces the "-all" option.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/webrev.03/
>>>>>
>>>>> Mandy
>>>>>
>>>>> On 12/5/12 5:36 PM, Mandy Chung wrote:
>>>>>> This review request (add a new jdeps tool in the JDK) include changes in
>>>>>> langtools and the jdk build.  I would need reviewers from the langtools
>>>>>> and the build-infra team.
>>>>>>
>>>>>> Webrev for review:
>>>>>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/langtools-webrev.02/
>>>>>> http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/jdk-webrev.02/
>>>>>>
>>>>>> The jdeps source is in the langtools repo.  The change in the jdk repo is
>>>>>> to add the new jdeps executable.  I made change in the old and new build
>>>>>> for the addition of this new jdeps tool.  I discussed with Jon whether I
>>>>>> should modify langtools/make/build.xml to add a new jdeps target. Since
>>>>>> the old build will be replaced by the new build soon, I simply add
>>>>>> com/sun/tools/jdeps as part of javap.includes.
>>>>>>
>>>>>> Alan,
>>>>>>
>>>>>> The -d option is kept as a hidden option and use as implementation. We
>>>>>> can remove it when it's determined not useful in the future. I also
>>>>>> rename -v:summary to -summary.
>>>>>>
>>>>>> Thanks
>>>>>> Mandy
>>>>>>
>>>>>> ----------------------------
>>>>>>
>>>>>> $ jdep -h
>>>>>> Usage: jdeps <options> <files....>
>>>>>> where possible options include:
>>>>>>   -version                 Version information
>>>>>>   -classpath <path>        Specify where to find class files
>>>>>>   -summary                 Print dependency summary only
>>>>>>   -v:class                 Print class-level dependencies
>>>>>>   -v:package               Print package-level dependencies
>>>>>>   -p <package name>        Restrict analysis to classes in this package
>>>>>>                            (may be given multiple times)
>>>>>>   -e <regex>               Restrict analysis to packages matching pattern
>>>>>>                            (-p and -e are exclusive)
>>>>>>   -P  --profile            Show profile or the file containing a package
>>>>>>   -R  --recursive          Traverse all dependencies recursively
>>>>>>   -all                     Process all classes specified in -classpath
>>>>>>
>>>>>> $ jdep Notepad.jar Ensemble.jar
>>>>>> Notepad.jar -> D:\tools\devtools\jdk8\windows-i586\jre\lib\rt.jar
>>>>>> <unnamed> (Notepad.jar)
>>>>>>       -> java.awt
>>>>>>       -> java.awt.event
>>>>>>       -> java.beans
>>>>>>       -> java.io
>>>>>>       -> java.lang
>>>>>>       -> java.net
>>>>>>       -> java.util
>>>>>>       -> java.util.logging
>>>>>>       -> javax.swing
>>>>>>       -> javax.swing.border
>>>>>>       -> javax.swing.event
>>>>>>       -> javax.swing.text
>>>>>>       -> javax.swing.tree
>>>>>>       -> javax.swing.undo
>>>>>>
>>>>>> Ensemble.jar -> D:\tools\devtools\jdk8\windows-i586\jre\lib\jfxrt.jar
>>>>>> Ensemble.jar -> D:\tools\devtools\jdk8\windows-i586\jre\lib\rt.jar
>>>>>>    com.javafx.main (Ensemble.jar)
>>>>>>       -> java.applet
>>>>>>       -> java.awt
>>>>>>       -> java.awt.event
>>>>>>       -> java.io
>>>>>>       -> java.lang
>>>>>>       -> java.lang.reflect
>>>>>>       -> java.net
>>>>>>       -> java.security
>>>>>>       -> java.util
>>>>>>       -> java.util.jar
>>>>>>       -> javax.swing
>>>>>>       -> sun.misc                                 JDK internal API (rt.jar)
>>>>>
>>>
>