On Thu, 17 Apr 2025 20:51:10 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:
> 
>   Review remarks

Hello Chen,

> Unfortunately, I don't think there is a convenient way to make a jar with 
> jtreg tests.

There are several tests in the JDK repo which create a JAR file of their choice 
within the test code and then run some tests against the JAR file. In fact, 
existing tests for `jdeps` tool already has similar tests. For example, there's 
a `JdepsUtil` test library which has a `createJar(...)` utility method which 
some of these tests use. This new test can use that utility if necessary.

In addition to testing this change against a JAR file, I think the test could 
also include running `jdeps` against a directory which contains more than one 
class file with one of the class file exhibiting the reported issue. That will 
then be able to reproduce the issue and verify the fix when running `jdeps` 
against a directory.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24604#issuecomment-2817647358

Reply via email to