Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-30 Thread Daniel Fuchs

Hi,

Please find below an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.01/

Instead of removing the CLITest.java - I copied
Process.java into the test hierarchy, renamed it to
ProcessXSLT.java, modified it so that it no longer uses
internal APIs, and changed the CLITest.java to use that.

The only adherence is with
com.sun.org.apache.xml.internal.utils.DefaultErrorHandler
that the new ProcessXSLT class tries to instantiate
through reflection - as that error handler has access to
jaxp internals and can give a better error diagnostic.

However - the ProcessXSLT will use its own dummy ErrorHandler
if it can't managed to instantiate the internal class.

This way - we can keep the test :-)

best regards,

-- daniel

On 29/07/15 17:02, Daniel Fuchs wrote:

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
  com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel




Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-30 Thread huizhe wang

Hi Daniel,

On 7/30/2015 6:38 AM, Daniel Fuchs wrote:

Hi,

Please find below an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.01/


Looks good to me. The only nit is it seems createDefaultErrorHandler 
method is a dup of createDefaultErrorListener?




Instead of removing the CLITest.java - I copied
Process.java into the test hierarchy, renamed it to
ProcessXSLT.java, modified it so that it no longer uses
internal APIs, and changed the CLITest.java to use that.


This is a good idea as it keeps the Process utility around.


The only adherence is with
com.sun.org.apache.xml.internal.utils.DefaultErrorHandler
that the new ProcessXSLT class tries to instantiate
through reflection - as that error handler has access to
jaxp internals and can give a better error diagnostic.

However - the ProcessXSLT will use its own dummy ErrorHandler
if it can't managed to instantiate the internal class.

This way - we can keep the test :-)


Perfect indeed, and if others (Yuri) want to use it, it's readily usable.

best,
Joe



best regards,

-- daniel

On 29/07/15 17:02, Daniel Fuchs wrote:

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
  com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel






Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-30 Thread Daniel Fuchs

On 30/07/15 18:16, huizhe wang wrote:


On 7/30/2015 9:08 AM, Daniel Fuchs wrote:

On 30/07/15 17:55, huizhe wang wrote:

Hi Daniel,

On 7/30/2015 6:38 AM, Daniel Fuchs wrote:

Hi,

Please find below an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.01/


Looks good to me. The only nit is it seems createDefaultErrorHandler
method is a dup of createDefaultErrorListener?


Yes it is - but the return type is different.


Yes, but didn't see it's used.


Good point! Removed.

-- daniel



Joe




Instead of removing the CLITest.java - I copied
Process.java into the test hierarchy, renamed it to
ProcessXSLT.java, modified it so that it no longer uses
internal APIs, and changed the CLITest.java to use that.


This is a good idea as it keeps the Process utility around.


The only adherence is with
com.sun.org.apache.xml.internal.utils.DefaultErrorHandler
that the new ProcessXSLT class tries to instantiate
through reflection - as that error handler has access to
jaxp internals and can give a better error diagnostic.

However - the ProcessXSLT will use its own dummy ErrorHandler
if it can't managed to instantiate the internal class.

This way - we can keep the test :-)


Perfect indeed, and if others (Yuri) want to use it, it's readily
usable.


Thanks Joe!

-- daniel



best,
Joe



best regards,

-- daniel

On 29/07/15 17:02, Daniel Fuchs wrote:

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel












Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-30 Thread Daniel Fuchs

On 30/07/15 17:55, huizhe wang wrote:

Hi Daniel,

On 7/30/2015 6:38 AM, Daniel Fuchs wrote:

Hi,

Please find below an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.01/


Looks good to me. The only nit is it seems createDefaultErrorHandler
method is a dup of createDefaultErrorListener?


Yes it is - but the return type is different.


Instead of removing the CLITest.java - I copied
Process.java into the test hierarchy, renamed it to
ProcessXSLT.java, modified it so that it no longer uses
internal APIs, and changed the CLITest.java to use that.


This is a good idea as it keeps the Process utility around.


The only adherence is with
com.sun.org.apache.xml.internal.utils.DefaultErrorHandler
that the new ProcessXSLT class tries to instantiate
through reflection - as that error handler has access to
jaxp internals and can give a better error diagnostic.

However - the ProcessXSLT will use its own dummy ErrorHandler
if it can't managed to instantiate the internal class.

This way - we can keep the test :-)


Perfect indeed, and if others (Yuri) want to use it, it's readily usable.


Thanks Joe!

-- daniel



best,
Joe



best regards,

-- daniel

On 29/07/15 17:02, Daniel Fuchs wrote:

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
  com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel








Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-30 Thread huizhe wang


On 7/30/2015 9:08 AM, Daniel Fuchs wrote:

On 30/07/15 17:55, huizhe wang wrote:

Hi Daniel,

On 7/30/2015 6:38 AM, Daniel Fuchs wrote:

Hi,

Please find below an updated webrev:

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.01/


Looks good to me. The only nit is it seems createDefaultErrorHandler
method is a dup of createDefaultErrorListener?


Yes it is - but the return type is different.


Yes, but didn't see it's used.

Joe




Instead of removing the CLITest.java - I copied
Process.java into the test hierarchy, renamed it to
ProcessXSLT.java, modified it so that it no longer uses
internal APIs, and changed the CLITest.java to use that.


This is a good idea as it keeps the Process utility around.


The only adherence is with
com.sun.org.apache.xml.internal.utils.DefaultErrorHandler
that the new ProcessXSLT class tries to instantiate
through reflection - as that error handler has access to
jaxp internals and can give a better error diagnostic.

However - the ProcessXSLT will use its own dummy ErrorHandler
if it can't managed to instantiate the internal class.

This way - we can keep the test :-)


Perfect indeed, and if others (Yuri) want to use it, it's readily 
usable.


Thanks Joe!

-- daniel



best,
Joe



best regards,

-- daniel

On 29/07/15 17:02, Daniel Fuchs wrote:

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel










Re: RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-29 Thread Lance Andersen
Looks OK based on the email thread about this proposed change


On Jul 29, 2015, at 11:02 AM, Daniel Fuchs daniel.fu...@oracle.com wrote:

 http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com





RfR - 8130058: jaxp: Investigate removal of com/sun/org/apache/xalan/internal/xslt/Process.java

2015-07-29 Thread Daniel Fuchs

Hi,

Please find below a patch that removes a bunch of unused files
in jdk9/dev/jaxp:

https://bugs.openjdk.java.net/browse/JDK-8130058
8130058: jaxp: Investigate removal of
 com/sun/org/apache/xalan/internal/xslt/Process.java

http://cr.openjdk.java.net/~dfuchs/webrev_8130058/webrev.00/

best regards,

-- daniel