8336695: Update Commons BCEL to Version 6.10.0 #27459

JoeWang-Java · 2025-09-23T21:11:07Z

Update Commons BCEL to Version 6.10.0.

Test: T1-3 passed

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8336695: Update Commons BCEL to Version 6.10.0 (Enhancement - P4)

Reviewers

Lance Andersen (@LanceAndersen - Reviewer)
Naoto Sato (@naotoj - Reviewer)
Iris Clark (@irisclark - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27459/head:pull/27459
$ git checkout pull/27459

Update a local copy of the PR:
$ git checkout pull/27459
$ git pull https://git.openjdk.org/jdk.git pull/27459/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27459

View PR using the GUI difftool:
$ git pr show -t 27459

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27459.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2025-09-23T21:12:08Z

👋 Welcome back joehw! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-09-23T21:13:18Z

@JoeWang-Java This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8336695: Update Commons BCEL to Version 6.10.0

Reviewed-by: lancea, naoto, iris

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 52 new commits pushed to the master branch:

5c596e2: 8360463: Ambiguity in Cipher.getInstance() specification between NoSuchAlgorithmException and NoSuchPaddingException
32ab0db: 8368674: Incremental builds keep rebuilding interim jmod
de1f5a3: 8368468: Split out everything but configure results from spec.gmk
... and 49 more: https://git.openjdk.org/jdk/compare/f1ee1b4a3d7c47b6f61b36b78504e3ec997a925a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2025-09-23T21:14:15Z

@JoeWang-Java The following label will be automatically applied to this pull request:

core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

mlbridge · 2025-09-23T21:18:28Z

Webrevs

00: Full (0e813612)

liach · 2025-09-23T23:30:34Z

Last time I tried to migrate off BCEL to ClassFile API, I noted only xalan xsltc is using it to write class files. Is it possible for us to remove it in the long run?

JoeWang-Java · 2025-09-24T03:49:42Z

Last time I tried to migrate off BCEL to ClassFile API, I noted only xalan xsltc is using it to write class files. Is it possible for us to remove it in the long run?

Possibly, but there is no justification for investing more resources in these libs than keeping them updated as required.

LanceAndersen

Overall, I think this is fine

Can you clarify/confirm that the comment changes are from upstream? I commented on a couple of examples.

Not sure why the change from "Returns" to "Gets" in several places so I am assuming this an upstream change

LanceAndersen · 2025-09-24T12:20:56Z

src/java.xml/share/classes/com/sun/org/apache/bcel/internal/Repository.java


    /**
-     * Lookups class somewhere found on your CLASSPATH, or wherever the repository instance looks for it.
+     * Lookups class somewhere found on your CLASSPATH, or whereever the repository instance looks for it.


The original "wherever" is correct

LanceAndersen · 2025-09-24T12:23:21Z

src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/AccessFlags.java


    /**
-     * Set access flags aka "modifiers".
+     * Sets access flags also known as modifiers.


Why change here, but not other cases such as the one that follows?

JoeWang-Java · 2025-09-24T16:46:52Z

Overall, I think this is fine

Can you clarify/confirm that the comment changes are from upstream? I commented on a couple of examples.

Not sure why the change from "Returns" to "Gets" in several places so I am assuming this an upstream change

Yes, they are from upstream. There were 174 lines/javadoc changes for the class AccessFlags alone. I basically applied them without change, including the ones you mentioned. It's tempting to fix them, but then we'll have to re-visit them the next time we update. We used to do so. We are fortunate we haven't had to go through the exercise for the other libs as Xalan/Xerces for example, haven't had many releases in recent years.

LanceAndersen · 2025-09-24T16:49:05Z

Overall, I think this is fine
Can you clarify/confirm that the comment changes are from upstream? I commented on a couple of examples.
Not sure why the change from "Returns" to "Gets" in several places so I am assuming this an upstream change

Yes, they are from upstream. There were 174 lines/javadoc changes for the class AccessFlags alone. I basically applied them without change, including the ones you mentioned. It's tempting to fix them, but then we'll have to re-visit them the next time we update. We used to do so. We are fortunate we haven't had to go through the exercise for the other libs as Xalan/Xerces for example, haven't had many releases in recent years.

Thank you for the clarification Joe. Just approved your PR

naotoj · 2025-09-24T17:41:25Z

src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/AnnotationEntry.java

 /*
- * reserved comment block
- * DO NOT REMOVE OR ALTER!
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.


IANAL, but I think the original year needs to be here, as apparently it has been in the repository.

Also, I see other files, e.g., ArrayElementValue.java has the same header ("reserved comment block"). Should they also have Oracle copyright?

Oracle copyright is added when we make changes to the upstream source. In this case, I replaced the newly introduced reference to Apache Commons Lang with a Utils method, thus the copyright year 2025 since we haven't made changes to the upstream source until now. Also added is the LastModified tag as required by the license. Note the tag was not there until now.

If Apache's bcel constantly introduce new dependencies, monkey patches like these are no better than migrating the whole xsltc to use ClassFile API for bytecode generation, in my opinion.

Migrating off of BCEL would certainly be great. If you have the time and interest, you're more than welcome to explore it further.

irisclark · 2025-09-25T18:11:27Z

src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/StackMap.java

- * <a href="http://java.sun.com/j2me/"> Java 2 Micro Edition</a> (J2ME). This attribute is used by the
- * <a href="http://java.sun.com/products/cldc/">KVM</a> and contained within the Code attribute of a method. See CLDC
+ * <a href="https://java.sun.com/j2me/"> Java 2 Micro Edition</a> (J2ME). This attribute is used by the
+ * <a href="https://java.sun.com/products/cldc/">KVM</a> and contained within the Code attribute of a method. See CLDC


This change from "http" to "https" highlights the age of these links. Both re-direct to the same page.

True, probably did not check the resulting page. It was a http to https patch at BCEL.

Yea. Not much you can do about without increasing the maintenance cost at our end. I think it's fine to keep what's there.

jdlib · 2025-09-25T18:29:24Z

Last time I tried to migrate off BCEL to ClassFile API, I noted only xalan xsltc is using it to write class files. Is it possible for us to remove it in the long run?

Remove just BCEL or even XSLTC?

liach · 2025-09-25T20:11:15Z

@jdlib If we need to maintain a patched (to remove extra dependencies) BCEL, I believe we are better off maintaining a patched (to use ClassFile API instead of BCEL) xsltc.

jdlib · 2025-09-27T09:28:56Z

@liach Refactoring XSLTC to use the classfile API seems to be a quite straightforward exercise (e.g. moving all use of BCEL classes into xsltc.MethodGenerator, (new) xsltc.InstructionList etc and then redirect their implementation to the classfile API), but not sure if the result can then be easily patched. On the other hand there may not be that much activity on the Apache XSLTC side anymore. I would almost like to volunteer for such a migration.

AlanBateman · 2025-09-27T13:03:56Z

It would be a nice project to do. If you do take it on then I think you'll need to survey the tests for javax.xml.transform and get some sense as to how well the XSLT compiler is tested. Maybe more tests are needed. A cautious approach would be to leave BCEL in place with a system probably or some means to go back to the old implementation until there is confidence that the replacement implementation is good. Also have a means to capture the generated classes from the old and new implementations might help on the confidence too.

8336695: Update Commons BCEL to Version 6.10.0

0e81361

openjdk bot added the core-libs [email protected] label Sep 23, 2025

openjdk bot added the rfr Pull request is ready for review label Sep 23, 2025

LanceAndersen reviewed Sep 24, 2025

View reviewed changes

LanceAndersen approved these changes Sep 24, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Sep 24, 2025

naotoj reviewed Sep 24, 2025

View reviewed changes

naotoj approved these changes Sep 24, 2025

View reviewed changes

irisclark approved these changes Sep 25, 2025

View reviewed changes

8336695: Update Commons BCEL to Version 6.10.0 #27459

Are you sure you want to change the base?

8336695: Update Commons BCEL to Version 6.10.0 #27459

Conversation

JoeWang-Java commented Sep 23, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Sep 23, 2025

Uh oh!

openjdk bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Sep 23, 2025

Uh oh!

mlbridge bot commented Sep 23, 2025

Webrevs

Uh oh!

liach commented Sep 23, 2025

Uh oh!

JoeWang-Java commented Sep 24, 2025

Uh oh!

LanceAndersen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoeWang-Java commented Sep 24, 2025

Uh oh!

LanceAndersen commented Sep 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdlib commented Sep 25, 2025

Uh oh!

liach commented Sep 25, 2025

Uh oh!

jdlib commented Sep 27, 2025

Uh oh!

AlanBateman commented Sep 27, 2025

Uh oh!

Uh oh!

JoeWang-Java commented Sep 23, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Sep 23, 2025 •

edited

Loading