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

Daniel Fuchs daniel.fuchs at oracle.com
Wed May 20 10:09:15 UTC 2015

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/webrev8080608/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

->

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 to handle the empty and non-empty case to avoid duplicate code line 148-149: final ArchiveDeps result = results.get(source); Stream 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: (where is the test case to run and is a number in [0..3]), or -e -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

More information about the core-libs-dev mailing list