Review request: 8003562: Provide a command-line tool to find static dependencies
Mandy Chung
mandy.chung at oracle.com
Fri Dec 14 20:54:34 UTC 2012
I have cleaned up per your comments, Ulf. Here is the updated webrev:
http://cr.openjdk.java.net/~mchung/jdk8/webrevs/jdeps/webrev.05/
This also cleans up how the inner classes are handled since
Location.getClassName returns a fully-qualified classname.
On 12/14/2012 3:54 AM, Ulf Zibis wrote:
> Some nits again:
>
> _private_ static class Options has _public_ members, why?
>
Fixed.
> 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 ???
>
I tried to be consistent with the javap implementation. It's reasonable
to have a common option parsing library for the command-line tools to
use - for example [1] that we hope to get to it.
> 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.
Fixed.
Thanks
Mandy
>
>
> 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
>>
http://hg.openjdk.java.net/jigsaw/jigsaw/jdk/file/ce66890e6d86/src/share/classes/org/openjdk/internal/joptsimple/
>> 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)
>>>>>>
>>>>
>>
>
More information about the core-libs-dev
mailing list