Review request: 8003562: Provide a command-line tool to find static dependencies (original) (raw)

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 <files....> where possible options include: -version Version information -classpath Specify where to find class files -summary Print dependency summary only -v:class Print class-level dependencies -v:package Print package-level dependencies -p Restrict analysis to classes in this package (may be given multiple times) -e 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 (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