On Mon, 21 Apr 2025 14:59:59 GMT, Chen Liang <li...@openjdk.org> wrote:
>> When jdeps was migrated from old classfile to ClassFile API, the parsing >> semantic changed - error checks are now made lazily, and nested crashes from >> malformed signature or other problems is now latent, after a `ClassModel` >> instance is available. (The old error check existed only for constructing a >> `ClassModel`) >> >> To address this issue, I have updated the way of iterating class files to be >> handler/consumer based like in the ClassFile API. This has the advantage >> that when one invocation of the handler fails of a `ClassFileError`, other >> invocations for other class files can proceed, and the exception handler has >> sufficient information to report a useful message indicating the source of >> error. >> >> For the particular example of examining a proguard processed >> `dummy-scala.jar`, here is the new output of `jdeps dummy-scala.jar`: >> >> Warning: com.sun.tools.jdeps.Dependencies$ClassFileError: Unexpected >> character ; at position 59, expected an identifier: >> Lscala/collection/immutable/TreeMap$TreeMapBuilder<TA;TB;>.;: >> scala/collection/immutable/TreeMap$TreeMapBuilder.class (dummy-scala.jar) >> Warning: com.sun.tools.jdeps.Dependencies$ClassFileError: Unexpected >> character ; at position 49, expected an identifier: >> Lscala/collection/parallel/mutable/ParArray<TT;>.;: >> scala/collection/parallel/mutable/ParArray.class (dummy-scala.jar) >> >> >> Now, jdeps shows the bad class files. Inspection into the files reveal that >> proguard incorrectly deleted the simple class names with trailing `$`, for >> example, the original signature of the broken ParArray was >> `Lscala/collection/parallel/mutable/ParArray<TT;>.ParArrayIterator$;`, so >> the `ParArrayIterator$` part was incorrectly dropped by proguard. >> >> Testing: langtools/tools/jdeps. > > Chen Liang has updated the pull request incrementally with one additional > commit since the last revision: > > Add a test Thank you Chen for adding the test. The test looks good to me. I verified locally that it does indeed reproduce the original issue without the fix. I've just one final request - since the `DirectoryIterator` has been updated in this PR, I think it would be good to include in your test, a `jdeps` invocation against a directory just like against you do it for the JAR file. ------------- PR Review: https://git.openjdk.org/jdk/pull/24604#pullrequestreview-2781628423
