RFR(jdk10/jaxp) 8163121: BCEL: update to the latest 6.0 release

[huizhe wang]

On 8/9/2017 6:30 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> This not an easy patch to review ;-)

Indeed, took several minutes just to copy the webrevs :-)

>
> Thanks for explaining how you arrived at the final result.
> The method you used as described below seems right.
>
> I haven't clicked through all the files, but instead I had a
> look at  all the JIRA issues revealed by:
>
> hg log -k share/classes/com/sun/org/apache/bcel | grep summary | grep 
> -v Merge
>
> to see if any of them might have been reverted/invalidated.
> So far so good, most of them still seem to be present in
> your changes, but I have a doubt about these two:
>
> https://bugs.openjdk.java.net/browse/JDK-8062608
> https://bugs.openjdk.java.net/browse/JDK-8064516
>
> Has the issue been fixed upstream in a different way, or
> is the issue no longer relevant, or is it going to appear
> again with this patch?

Good catch!  The original patch came from BCEL, so I indeed assumed this 
update would get things the right way. As I double-checked, it turns out 
the additional patches as you listed above fixed issues that BCEL 6.0 
hasn't. These issues were revealed by JRocket, did not affect JAXP, 
however, they are still good changes that we shall keep. I've gone 
through both of the above to put these changes back into the code.

>
> Also looking at the JDK fixes brought my attention to
> java.xml/share/classes/jdk/xml/internal/SecuritySupport.java
>
> Your changes are adding these two methods:
> I don't know what they are for - and I am not sure they would
> work (the brutal cast to ListResourceBundle is dubious).
> Fortunately they don't seem to be called by the new code (I
> grepped the global patch file). Can you remove them?

This is because I removed 
com/sun/org/apache/bcel/internal/util/SecuritySupport.java that is no 
longer used by any classes in our version of BCEL. However, a XPath 
class (XPATHMessages.java) was still using it, I therefore moved the 
getResourceBundle method that was used into the jdk util class before 
removing the one in bcel.

The ListResourceBundle cast was needed since the XPATHMessages variable 
was originally declared ListResourceBundle. I've changed that to 
ResourceBundle and removed the cast.

Here's the updated webrev:
http://cr.openjdk.java.net/~joehw/jdk10/8163121/webrev01/

Thanks,
Joe

>
>  234     /**
>  235      * Gets a resource bundle using the specified base name, the 
> default locale, and the caller's class loader.
>  236      * @param bundle the base name of the resource bundle, a 
> fully qualified class name
>  237      * @return a resource bundle for the given base name and the 
> default locale
>  238      */
>  239     public static ListResourceBundle getResourceBundle(String 
> bundle) {
>  240         return getResourceBundle(bundle, Locale.getDefault());
>  241     }
>  242
>  243     /**
>  244      * Gets a resource bundle using the specified base name and 
> locale, and the caller's class loader.
>  245      * @param bundle the base name of the resource bundle, a 
> fully qualified class name
>  246      * @param locale the locale for which a resource bundle is 
> desired
>  247      * @return a resource bundle for the given base name and locale
>  248      */
>  249     public static ListResourceBundle getResourceBundle(final 
> String bundle, final Locale locale) {
>  250         return 
> AccessController.doPrivileged((PrivilegedAction<ListResourceBundle>) 
> () -> {
>  251             try {
>  252                 return 
> (ListResourceBundle)ResourceBundle.getBundle(bundle, locale);
>  253             } catch (MissingResourceException e) {
>  254                 try {
>  255                     return 
> (ListResourceBundle)ResourceBundle.getBundle(bundle, new Locale("en", 
> "US"));
>  256                 } catch (MissingResourceException e2) {
>  257                     throw new MissingResourceException(
>  258                             "Could not load any resource bundle 
> by " + bundle, bundle, "");
>  259                 }
>  260             }
>  261         });
>  262     }
>
> best regards,
>
> -- daniel
>
> On 09/08/2017 00:43, huizhe wang wrote:
>> Please review an update to BCEL release 6.0 [1].
>>
>> The sources are basically that from the release bundle with the 
>> following exceptions:
>>
>>   * Classes that were not included in the original version are excluded;
>>   * Classes that are deprecated in BCEL 6.0 are removed;
>>   * Classes that are used only for the above classes are also removed;
>>   * Many fields were deprecated in BCEL 6.0 in order to discourage
>>     direct references, in the JDK, I've removed such deprecation and
>>     instead made them private/final.
>>   * BCEL 6.0 applied 'final' to global/local variables pretty
>>     extensively, which were the only changes to many of the classes.
>>   * A few Transform classes were updated to adopt to BCEL 6.0.
>>   * JDK-8162527 [JAXP] XSLT: Investigate why bumping the default class
>>     file version to 49 (Java 5.0) or higher for bytecode generation
>>     doesn't work [2] has not been resolved as of this update. Additional
>>     change [3] is needed before we can move up to possibly Java 5.0.
>>
>>
>> BCEL has been a very old component. This update therefore removed a 
>> large amount of warnings (900+), which is very helpful to the 
>> cleaning effort.
>>
>> All tests passed, including the JCK.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8163121
>> webrevs: http://cr.openjdk.java.net/~joehw/jdk10/8163121/webrev/
>>
>>
>> [1] https://archive.apache.org/dist/commons/bcel/RELEASE-NOTES.txt
>> [2] https://bugs.openjdk.java.net/browse/JDK-8162527
>> [3] https://bugs.openjdk.java.net/browse/JDK-8185991
>>
>> Thanks,
>> Joe
>>
>