RFR(jdk10/jaxp) 8163121: BCEL: update to the latest 6.0 release
huizhe wang
huizhe.wang at oracle.com
Fri Aug 11 22:00:19 UTC 2017
Thanks Daniel!
Best regards,
Joe
On 8/11/2017 3:31 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> Thanks, looks fine now!
>
> best regards,
>
> -- daniel
>
> On 10/08/2017 04:13, huizhe wang wrote:
>>
>> 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
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list