RFR 8061297 : sun/reflect/CallerSensitive/CallerSensitiveFinder.java should use the JRT FileSystem

[Chris Hegarty]

Felix,

On 15 Jan 2015, at 07:15, FELIX YANG <felix.yang at oracle.com> wrote:

> Chris, 
>      excuse me,  I'm not a reviewer but have some questions.

All comments and questions are welcome.

>      at line 209, classes is actually trying to resolve a path like "c:\jdk1.9.0\modules", which never exists in JDK directory. It should be "c:\jdk1.9.0\lib\modules”.  

The 'jdk/modules' directory will only exist when running with an exploded build. It is useful to be able to run tests against both exploded builds and images.

>  207         if (home.resolve("lib").toFile().exists()) {
>  208             // either an exploded build or an image

The check for 'lib' is not strictly necessary, I will remove it, as it is confusing.

> 209             File classes = home.resolve("modules").toFile();
> 
>  210             if (classes.isDirectory()) {
>  211                 return Stream.of(classes.toPath());
>  212             } else {
>  213                 return JrtPaths();
>  214             }
> 
> The test works because it always go to JrtPaths method. The jimages have been mounted at initializing of Jrt file system. 

When running with a binary image, then yes it will always use the JRT code path.

> There may be two issues here:
> 1. the naming 
> JrtPaths doesn't follow common rules. Is it more suitable with j
> rtPaths()?

I’ll make this change before pushing.

> 2. Even when changing classes to "lib/modules", it is still probably unable to process class files. I tried last code and failed.
> 
> File classes = home.resolve("lib/modules").toFile();

‘lib/modules’ is not a path that is search for classes by the runtime. Previous comments should have already covered the requirement for ‘modules’

Thanks for looking at this felix.,
-Chris.

> Because, com.sun.tools.jdeps.ClassFileReader hasn't been upgraded to correctly handle Jrt image files.
> 
> So this logic is either unnecessary (as stated above, the jimages have been mounted at initializing of Jrt file system) or unable to work now.
> 
> Thanks,
> 
> -Felix
> On 1/15/2015 12:31 AM, Chris Hegarty wrote:
>> On 14/01/15 16:18, Mandy Chung wrote: 
>>> On 1/14/2015 8:04 AM, Chris Hegarty wrote: 
>>>> 
>>>> http://cr.openjdk.java.net/~chegar/8061297/webrev.01/webrev/ 
>>> 
>>> Looks okay to me. 
>>> 
>>> In CallerSensitiveFinder.java line 57, 80, 137 - not sure if you were 
>>> thinking to pass "pool" to the CallerSensitiveFinder constructor. Looks 
>>> like some editing you meant to cleanup. 
>>> 
>>>    nit: line 76 a space between "classes" and "==" 
>>>    line 207 can be removed. 
>> 
>> D'oh! Fixed both of these. Webrev updated in place. 
>> 
>> -Chris. 
>> 
>>> Mandy 
>>> 
>>> 
>