Bug in ClassPathContext?

[Mandy Chung]

On 10/11/2012 8:25 AM, mark.reinhold at oracle.com wrote:
> 2012/10/10 20:13 -0700, mandy.chung at oracle.com:
>> On 10/10/2012 4:38 PM, mark.reinhold at oracle.com wrote:
>>> Now that I've studied your recent URLClassPath changes, I think what's
>>> happening is that if a $BUILD/classes directory doesn't exist then the
>>> tools and ext modules are marked as bootstrap modules, so they're loaded
>>> by the bootstrap loader rather than a URLClassPath.ModuleLibraryLoader
>>> as you probably intended.
>> No that's not my intent.  If there is no $BUILD/classes directory
>> (not a development build), tools/ext modules should not be loaded by
>> the bootstrap loader.
>>
>> The comment is correct and the check should be (in both toolsModule()
>> and extModules() methods):
>>
>> -        return file("classes") != null ? modules : new String[0];
>> +        return file("classes") == null ? modules : new String[0];
>>
>> This tries to retain the classpath mode behavior as with a legacy
>> development build.  In a legacy jdk build, all classes including tools
>> are loaded by the bootstrap loader.  Perhaps this isn't highly
>> necessary with module library.
> In the legacy build it was an expedient hack to load tool classes via the
> bootstrap class loader in a development build.  I don't see any reason to
> preserve that behavior, especially when we now have a chance to reduce
> the difference between development and image builds.
>
> FYI, I tried changing the lines in question to return the array of module
> names unconditionally, but that didn't work.  Haven't yet figured out why.

It doesn't work because the development build doesn't have tools.jar
which is another piece of the puzzle.  In a legacy image, the tools
classes are loaded only if tools.jar is on the classpath.
If we always load the tools classes in a module image even if
-cp tools.jar is not specified, it will be a change in behavior.
In addition, there are also existing applications that checks if
tools.jar exists and loads the javac main class using a URLClassLoader.
For compatibility, we decided to create an empty tools.jar in the
module image to retain the same behavior when running in classpath mode.
There is no tools.jar in the development build and thus it doesn't work.

ClasssPathContext.LoaderType.isDefaultPath method tests if
tools.jar exists (that's the default path for system class loader).

URLClassPath.getLoader(URL) will determine if the given URL
is the default path for the classpath, extension, or bootclasspath
and then searches classes/resources from the appropriate modules.

I agree it's a good chance to reduce the difference between development
and image builds.  I think a good time to do that is when we add
the runtime modulepath (aka exploded module) support (Alan and I
briefly talked about the exploded modules during the development
of classpath mode support).  With the exploded modules, the development
build would not need to have a module library. We'll follow up on
this.

The JPRT job almost completes and tests passed on most builds
except solaris-sparcv9 and solaris-x64 since there is a bug
in the makefile that didn't create tools.jar (solaris 64-bit
build logic is different than linux/windows/macosx 64-bit).
I'll fix the makefile and push the changeset.

Mandy