RFR: 8080608: Missing archive name from jdeps -v -e output if no dependency on other JAR

[Daniel Fuchs]

On 20/05/15 01:44, Mandy Chung wrote:
> On 05/19/2015 10:02 AM, Daniel Fuchs wrote:
>> Hi,
>>
>> Please find below a patch for jdeps:
>>
>> http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8080608
>>
>> :
>>
>> The fix will make sure that jdeps prints instead:
>>
>> indirect2.jar -> dist/unsafe.jar
>>    use.indirect2.UseUnsafeIndirectly2 -> use.unsafe.UseUnsafeClass
>> unsafe.jar
>> unsafe.jar -> dist/unsafe.jar
>>    use.unsafe.UseClassWithUnsafe -> use.unsafe.UseUnsafeClass unsafe.jar
>
> 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...).

>
> 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.


>   This long line should be broken into multiple lines.
>
> Maybe good to initialize Stream<Archive> to handle the empty and
> non-empty case to avoid duplicate code line 148-149:
>     final ArchiveDeps result = results.get(source);
>     Stream<Archive> reqs = result.requires().stream();
>     if (result.requires().isEmpty()&& hasDependences(source)) {
>         reqs = Stream.of(source);
>     }
>     reqs.sorted(.....

Right. Good idea.

New webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8080608/webrev.01/

> I haven't reviewed the tests.  They are named as closure.  Are these
> tests for the new jdeps enhancement that you are thinking?

Hmmm... well - yes and now. The 'closure' directory name might be
a misnomer... Maybe 'VerboseFormat' would be a better name.
But the test tests the verbose format by performing a closure...

There is only one test: JdepsDependencyClosure (containing several
test cases). The rest are just dummy java files on which the test
performs its analysis.

The test accept arguments in the form of either --test:<nb> (where
<nb> is the test case to run and is a number in [0..3]), or
-e <pattern> -v <jars...>

This means that you can use the test as a means to get the closure
on all classes depending on a particular class, where only the
classes in the given <jars...> are analyzed.

So yes - it could serve as a basis to develop the tests that will
come with the new enhancement I was thinking about, but I'm not
sure it will be directly usable for that in this form.

In the mean time, someone who would need to get the closure of his
own classes depending on an unsupported usage of a private sun.*
API could also use that test as if it where a stand alone tool (no 
guarantee made that it won't break on some untested formatting though).

best regards,

-- daniel

>
> Thanks
> Mandy