Evaluate BCEL 6 update strategies #106
Comments
|
I think renaming back BCEL package is a lot of effort. A new major version (4.0) with compatibility breaking changes totally makes sense for Java 9. |
|
Actually I did it in few seconds, this was easy thanks to Eclipse. Unfortunately they also did some other API changes, but also this was easy to fix on FB side. The problem is that the code seem to have some major issues which look like regressions in BCEL6. |
|
Attaching the snapshot jar (github only allows zip's) I've generated from almost unchanges BCEL 6 trunk: iloveeclipse/commons-bcel@5ffd682 |
This is the first commit which only fixes BCEL package move (see https://issues.apache.org/jira/browse/BCEL-222), two commits will follow with fixes for other BCEL API breakage.
#106 StackMapTableEntry also changed signature: getByteCodeOffsetDelta -> getByteCodeOffset
The jar snapshot is created from iloveeclipse/commons-bcel@5ffd682 This is almost unmodified HAED from BCEL6, few small commits after git-svn-id: https://svn.apache.org/repos/asf/commons/proper/bcel/trunk@1746918
|
OK, after full upgrade of FB code to the untouched BCEL6 trunk state I still see same test errors. BCEL bug reported : https://issues.apache.org/jira/browse/BCEL-273. |
|
@amaembo : Tagir, do you have time to look into the regressions with BCEL6, see comment above? |
|
@iloveeclipse nope, sorry. In general I quit active development of Findbugs as I started my own bytecode analyzer. |
|
@amaembo A-ha, good news and bad news at same time. |
|
Not yet/not yet/not yet. It's hard to do in two months everything which was done in FindBugs for 15 years :-) We have draft maven and gradle plugins. Ant is not big priority these days, though I will probably write it. Also we already have a volunteer who wrote draft Eclipse plugin. 99% FB warnings is not a priority: some warnings are deliberately excluded as junk/outdated (see this list). Though indeed most of them must be reimplemented. |
|
OK, an update to the BCEL API story: the package renaming is reverted, and I hope to see other breakages fixed soon. |
|
Attached is the snapshot based on iloveeclipse/commons-bcel@452b5e6 Corresponding FB branch is https://github.com/findbugsproject/findbugs/commits/java9_bcel6_old If I apply iloveeclipse/commons-bcel@25dfa20 on BCEL head, all tests pass now on Java 8. |
This also fixes the StackMapTable misery, see BCEL-92 and BCEL-248 and discussion on http://mail-archives.apache.org/mod_mbox/commons-dev/201606.mbox/ajax/%3C4595124.Qpp5S77DT7%40pinguin%3E. Added StackMapTable/StackMapTableEntry to the FB repository, basically 1:1 copy of StackMap/StackMapEntry. The classes are only there to don't break the clients used them via PreorderVisitor API. Fixed removed constants from Constants (they match those from Const). The tests are still failing due https://issues.apache.org/jira/browse/BCEL-273.
This also fixes the StackMapTable misery, see BCEL-92 and BCEL-248 and discussion on http://mail-archives.apache.org/mod_mbox/commons-dev/201606.mbox/ajax/%3C4595124.Qpp5S77DT7%40pinguin%3E. Added StackMapTable/StackMapTableEntry to the FB repository, basically 1:1 copy of StackMap/StackMapEntry. The classes are only there to don't break the clients used them via PreorderVisitor API. Fixed removed constants from Constants (they match those from Const). The tests are still failing due https://issues.apache.org/jira/browse/BCEL-273.
Created TODO's in the code to avoid analysis errors while visiting INVOKEDYNAMIC instructions.
|
BCEL 6 trunk snapshot with only single patch applied on top: iloveeclipse/commons-bcel@25dfa20 This works now on Java 8/9 without known errors. |
The build uses now BCEL6 snapshot built from BCEL6 trunk + 1 patch, see iloveeclipse/commons-bcel@25dfa20 and https://issues.apache.org/jira/browse/BCEL-273.
|
I think we can close this issue now. BCEL 6.0 will use old namespace and we have the patch to get it working with Findbugs without breaking every client. |
This also fixes the StackMapTable misery, see BCEL-92 and BCEL-248 and discussion on http://mail-archives.apache.org/mod_mbox/commons-dev/201606.mbox/ajax/%3C4595124.Qpp5S77DT7%40pinguin%3E. Added StackMapTable/StackMapTableEntry to the FB repository, basically 1:1 copy of StackMap/StackMapEntry. The classes are only there to don't break the clients used them via PreorderVisitor API. Fixed removed constants from Constants (they match those from Const). The tests are still failing due https://issues.apache.org/jira/browse/BCEL-273.
…oject#106 Created TODO's in the code to avoid analysis errors while visiting INVOKEDYNAMIC instructions.
The build uses now BCEL6 snapshot built from BCEL6 trunk + 1 patch, see iloveeclipse/commons-bcel@25dfa20 and https://issues.apache.org/jira/browse/BCEL-273.
BCEL 6 broke every single API by moving all types in different package structure, see https://issues.apache.org/jira/browse/BCEL-222. No comments about that move.
However, to get up to date with latest Java 9 fixes in BCEL we have either to break every FB plugin/user (because we exposed BCEL in FB API all over the place, see for example use of org.apache.bcel.classfile.JavaClass) and ship totally incompatible FB 4.0, or we need to backport BCEL 6 changes to the "old" package structure.
I'm investigating the later one.
OK, I've managed to rename the packages back (see https://github.com/iloveeclipse/commons-bcel/tree/old_structure) and with few (not committed) FB changes I was able to run FB with that BCEL version. Problem: it simply doesn't work in few cases anymore, neither on Java 8 nor on Java 9. I had few different exceptions, which I'm not sure where the root cause is - new BCEL code or missing adoption on FB side:
bcel_test_result.txt
Now I'm going to break FB API and use BCEL as is, just to test if BCEL 6 "unchanged" will work with FB.
The text was updated successfully, but these errors were encountered: