Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-20 Thread Paul Sandoz

On Jan 17, 2014, at 10:10 PM, Mandy Chung mandy.ch...@oracle.com wrote:

 Paul,
 
 Here is the updated webrev:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.01/
 
 This cleans up GetResource.sh and merges with GetResource2.sh.  Also fixed 
 if (pos - lastPoc  0) line and javadoc typo.
 

Looks good.

--

In GetResource.sh:
  89 GetResource $expected || exit 10
Should it be exit $count ?, i suppose it does not matter. Regardless no need 
for another review round for that.

Paul.


Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-16 Thread Paul Sandoz

On Jan 16, 2014, at 2:27 AM, Mandy Chung mandy.ch...@oracle.com wrote:

 There is an inconsistency in searching classes vs resources if bootclasspath 
 contains an empty path.  Empty path on bootclasspath is skipped by the 
 bootstrap class loader when searching classes while it defaults to current 
 working directory when searching resources as the application class loader.

Just curious, where is the code that skips empty paths in the bootclasspath 
when loading classes? Is that in the VM?


  This fixes sun.misc.Launcher to skip empty path when constructing the paths 
 from bootclasspath for resource lookup.
 
 Webrev at:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.00/
 

Perhaps it is better to merge GetResource2.sh into GetResource.sh.

I am so tempted to change this too :-)

 423 if (pos - lastPos  0) {

Paul.


 There is some incompatibility risk that may impact existing code depending on 
 this behavior to search resources from the current working directory if empty 
 path (rather than explicit) is set.  I think most application using 
 bootclasspath is to add their paths to load their classes and likely expect 
 the resources are looked in the consistent way (i.e. skips the empty path).  
 So I expect the incompatibility risk is low.
 
 thanks
 Mandy



Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-16 Thread Paul Sandoz

On Jan 16, 2014, at 4:51 PM, Mandy Chung mandy.ch...@oracle.com wrote:

 
 On 1/16/2014 7:39 AM, Paul Sandoz wrote:
 On Jan 16, 2014, at 2:27 AM, Mandy Chung mandy.ch...@oracle.com wrote:
 
 There is an inconsistency in searching classes vs resources if 
 bootclasspath contains an empty path.  Empty path on bootclasspath is 
 skipped by the bootstrap class loader when searching classes while it 
 defaults to current working directory when searching resources as the 
 application class loader.
 Just curious, where is the code that skips empty paths in the bootclasspath 
 when loading classes? Is that in the VM?
 
 Yes it's in the VM which implements the class loading.   I don't know the 
 history and I guess splitting the implementation of class loading vs resource 
 lookup in the VM and jdk could be due to no actual ClassLoader implementation 
 (null instead).
 

OK.


  This fixes sun.misc.Launcher to skip empty path when constructing the 
 paths from bootclasspath for resource lookup.
 
 Webrev at:
 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.00/
 
 Perhaps it is better to merge GetResource2.sh into GetResource.sh.
 
 I was considering if merging them would be better and separating it makes it 
 clear for what it tests for.  On the other hand, I don't like duplication and 
 I probably can merge them and clean
 

OK, the echo statements make things quite clear IMHO.


 I am so tempted to change this too :-)
 
  423 if (pos - lastPos  0) {
 
 Me too and time to do it.  Will post another webrev.
 

While you are at it, there is a JavaDoc error for the method ref:

/**
 * This class loader supports dynamic additions to the class path
 * at runtime.
 *
 * @see 
java.lang.instrument.Instrumentation#appendToSystemClassPathSearch
 */
private void appendToClassPathForInstrumentation(String path) {

Even though it is a private method that JavaDoc is useful since one can click 
through via the IDE.

Paul.


Re: Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-16 Thread Mandy Chung


On 1/16/2014 7:39 AM, Paul Sandoz wrote:

On Jan 16, 2014, at 2:27 AM, Mandy Chung mandy.ch...@oracle.com wrote:


There is an inconsistency in searching classes vs resources if bootclasspath 
contains an empty path.  Empty path on bootclasspath is skipped by the 
bootstrap class loader when searching classes while it defaults to current 
working directory when searching resources as the application class loader.

Just curious, where is the code that skips empty paths in the bootclasspath 
when loading classes? Is that in the VM?


Yes it's in the VM which implements the class loading.   I don't know 
the history and I guess splitting the implementation of class loading vs 
resource lookup in the VM and jdk could be due to no actual ClassLoader 
implementation (null instead).



  This fixes sun.misc.Launcher to skip empty path when constructing the paths 
from bootclasspath for resource lookup.

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.00/


Perhaps it is better to merge GetResource2.sh into GetResource.sh.


I was considering if merging them would be better and separating it 
makes it clear for what it tests for.  On the other hand, I don't like 
duplication and I probably can merge them and clean



I am so tempted to change this too :-)

  423 if (pos - lastPos  0) {


Me too and time to do it.  Will post another webrev.

Mandy


Paul.



There is some incompatibility risk that may impact existing code depending on 
this behavior to search resources from the current working directory if empty 
path (rather than explicit) is set.  I think most application using 
bootclasspath is to add their paths to load their classes and likely expect the 
resources are looked in the consistent way (i.e. skips the empty path).  So I 
expect the incompatibility risk is low.

thanks
Mandy




Review request for JDK-6760902: inconsistent behavior in bootstrap class loader for classes and resources

2014-01-15 Thread Mandy Chung
There is an inconsistency in searching classes vs resources if 
bootclasspath contains an empty path.  Empty path on bootclasspath is 
skipped by the bootstrap class loader when searching classes while it 
defaults to current working directory when searching resources as the 
application class loader.  This fixes sun.misc.Launcher to skip empty 
path when constructing the paths from bootclasspath for resource lookup.


Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/6760902/webrev.00/

There is some incompatibility risk that may impact existing code 
depending on this behavior to search resources from the current working 
directory if empty path (rather than explicit) is set.  I think most 
application using bootclasspath is to add their paths to load their 
classes and likely expect the resources are looked in the consistent way 
(i.e. skips the empty path).  So I expect the incompatibility risk is low.


thanks
Mandy