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)
