> On May 20, 2015, at 3:09 AM, Daniel Fuchs <daniel.fu...@oracle.com> wrote:
>
> On 20/05/15 01:44, Mandy Chung wrote:
>>
>> A dependency from unsafe.jar to itself is redundant and thus was
>> excluded from the returned value of the requires method. Maybe better
>> just to output without any dependence like this:
>>
>> unsafe.jar
>> use.unsafe.UseClassWithUnsafe -> use.unsafe.UseUnsafeClass unsafe.jar
>
> I'm not sure it's redundant because the dependency written
>
> <archive-name> -> <path-to-the-archive>
>
> This is the only place were the path to the archive appears - and
> I believe it's good to keep it (+ it makes the output more regular
> and thus easier to parse with simple greps etc…).
Good point. Let’s keep the dependency output consistent format.
>
>>
>> Analyzer.java
>>
>> 147 if (result.requires.isEmpty() && type == Type.VERBOSE &&
>> hasDependences(source)) {
>> Is type == Type.VERBOSE necessary? If hasDependences(source)
>> returns true,
>> "unsafe.jar" is missing for non-verbose mode I assume.
>
> Yes - for summary mode, having a line that tells you
> unsafe.jar -> dist/unsafe.jar
> and nothing else is not very helpful.
>
> Actually - I think the test should be type != Type.SUMMARY rather
> than type == Type.VERBOSE.
jdeps has -filter:none option.
$ jdeps -s -filter:none -e use.unsafe.UseUnsafeClass *.jar
When -filter:none is used, I think it’s right for the summary page should
include unsafe.jar in that case. The default will filter out same package
dependency and hasDependency returns false in that case.
>
>
> New webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.01/
> <http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.01/>
Analyzer.java looks good except that I think it doesn’t need type !=
Type.SUMMARY check.
Thanks for adding the great tests. The test can be extracted as a jdeps test
library. Do you have the cycle to refactor it so that it can be used in
future jdeps tests?
TestCaseData now takes 3 String[][] to list classes, dependencies and archives.
You can consider making it a jdeps data Builder that will make the data more
explicit expected dependencies of each class.
Can you run all test cases in the same VM? The test calls
com.sun.tools.jdeps.Main directly and it’d be good to avoid running each test
case as a separate VM unless it’s necessary. Can you also add @modules
jdk.dev/com.sun.tools.jdeps following @summary as it uses internal API.
Mandy
