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

[Daniel Fuchs]

Hi Joe,

This not an easy patch to review ;-)

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?

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?

  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
>