How Apache Commons BCEL got to where it is currently.

1. I wanted to release a version of BCEL which would support Java 6 and 7.
2. I updated several classes that handled the new instructions and new code 
attributes.
3. This required new methods on several interfaces.
4. These new methods broke binary compatibility.
5. Whenever binary compatibility is broken, Apache Commons policy is to update 
the Maven GAV to prevent jar hell.
6. Part of updating GAV is to also update the package names.
7. I created a release candidate which was deemed unsuitable for several 
reasons; mostly due to FindBugs warnings.
8. Multiple refactorings were completed (including moving Interface to Class) 
to handle FindBugs warnings.
9. Refactoring died out after no response from users as to Apache direction.
10. Recent new interest has us revisiting these changes.

At this point, we’re somewhat stuck.
1. Do we break Apache Commons Policy regarding binary compatibility GAV and 
package names?
2. Do we ignore the FindBugs warnings?

Personally, I am against either of those.  I also believe that to fix BCEL 
correctly, we’ll end up with an api sufficiently different that users will have 
a non-trivial porting task.  It might be saner if Apache Commons moves BCEL 
into the attic and suggest that our clients to migrate to either ASM or 
JavaAssist.

regards,
chas

> On Jun 6, 2016, at 11:27 AM, Andrey Loskutov <losku...@gmx.de> wrote:
> 
> Hi all,
> 
> this is a follow up on 
> https://mailman.cs.umd.edu/pipermail/findbugs-discuss/2016-June/004282.html.
> 
> I'm cross-posting this to dev@commons.apache.org because the discussion on 
> FindBugs mailing list is related to the BCEL 6 API future, and because I 
> would like to know the opinions from the BCEL community on the upcoming BCEL 
> 6 release compatibility story.
> 
> Please see my answers inline.
> 
> On Monday 06 June 2016 17:30 sebb wrote:
>> On 6 June 2016 at 16:23, Andrey Loskutov <losku...@gmx.de> wrote:
>>> Hi all,
>>> 
>>> here is the current state of FindBugs adoption to Java 9.
>>> 
>>> 1) FindBugs standalone/Eclipse plugin run fine now on/with Java 9, the 
>>> latest code is on java9 branch (not on master yet!), see [0, 1]. If there 
>>> is interest, I can provide binary snapshots.
>>> 
>>> 2)  I have difficulties to use BCEL6 trunk, see [2]. Looks like even after 
>>> fixing all compile errors due the various API breakages in BCEL 6 (see 
>>> [3]), the current BCEL 6 head can't be used directly as a replacement for 
>>> our old BCEL5.2 fork, see [4]. If anyone from FB and/or BCEL gurus could 
>>> check this, this would be really helpful. Either our BCEL 5.2 patches were 
>>> not fully propagated upstream to BCEL or BCEL 6 trunk has few regressions, 
>>> or I missed something during update? I have no idea, because of the next 
>>> point below. The experimental BCEL 6 port is on an extra branch on top of 
>>> Java 9 related changes, see commits prefixed with BCEL6 on java9_bcel6 
>>> branch at [5].
>>> 
>>> 3) I would be very happy if someone (Bill?) would explain how the *current* 
>>> BCEL5.2 fork used by FindBugs was built? It was added in commit [6] but I 
>>> miss instructions how it differs from the original BCEL code and so unable 
>>> to re-build it.
>>> 
>>> 4) Assuming BCEL6 bugs/FB errors would be fixed (see [4]), transition to 
>>> the current BCEL6 head would break each and every FindBugs client, because 
>>> BCEL6 at current state uses different namespace and also added some other 
>>> API breaking changes. If we chose this path, none of the 3rd party 
>>> detectors will work anymore and therefore we must bump FindBugs version to 
>>> 4.0.
>> 
>> This is useful to know.
>> So do the 3rd party detectors depend on the BCEL namespace?
> 
> Yes
> 
>> Or is it because of the BCEL API changes?
> 
> Also yes.
> 
>> If so, which ones?
> 
> The biggest one is the package namespace change, because this affect each 
> existing BCEL class/interface.
> See commit 
> https://github.com/findbugsproject/findbugs/commit/917b692d9a12e048921cd1216102b851866ac3e4
>  which affects ~400 files in FindBugs.
> 
> Much smaller (but still breaking API) changes were class name changes 
> Constants -> Const, StackMapTable[Entry] -> StackMap[Entry] and the move of 
> constants defined in Constants from the interface to the class, thus breaking 
> everyone who implemented the interface and now miss the constants. The rename 
> of StackMapTable/Entry broke also additionally every detector implemented on 
> top of PreorderVisitor. StackMapTableEntry not only changed its name, but 
> also changed signature: getByteCodeOffsetDelta -> getByteCodeOffset which 
> gives you an additional piece of happiness.
> 
> Finally, VisitorSupportsInvokeDynamic interface was removed, which broke all 
> FB visitors based on it via AbstractFrameModelingVisitor and 8 methods were 
> added to the Visitor interface.
> 
> That's all I saw in our FB code (where we have lot of detectors), probably 
> others will report additional API breakage too, I can't say for sure. 
> 
> But main issue is the namespace change - it is really unnecessary and 
> surprising. I've read through the commons mailing list and I was surprised 
> that I saw no real request for it or any discussion about it (I haven't read 
> through all the years but around the namespace change last summer). The only 
> thing I saw was the Jira request 
> https://issues.apache.org/jira/browse/BCEL-222, out of nowhere, and few 
> commits later BCEL 6 API was made incompatible to every existing client. :-(
> 
>> I'm a bit suprised that the BCEL API should affect the detectors, but
>> perhaps there's a good reason for that.
> 
> BF analyses bytecode, and although we have also few recently added ASM based 
> detectors (which are mostly BCEL free), most of the detectors (and 
> unfortunately many places in the FB API) use BCEL data structures. It was a 
> natural choice 15 years ago, where BCEL was the only one bytecode framework...
> 
> One way to "fix" the current FindBugs misery  is to replace BCEL with ASM 
> (asm.tree package &Co) but this requires lot of effort because API and design 
> in ASM do not match BCEL 1:1 - and it will also break every FB client in much 
> harder way BCEL 6 API breakage does it  today. Doing this will effectively 
> mean a complete fork/rewrite of FindBugs code, and no one is willing to spend 
> time for it.
> 
>>> Question is: should we go this way? Alternative is: undo BCEL package 
>>> renaming and revert few API changes were made. This sounds complicated, but 
>>> is doable, see BCEL 6 fork where I renamed all packages back to the old 
>>> namespace in few minutes [7]. Fixing other "smaller" breaking BCEL API 
>>> changes is not that complicated either. However, it is also possible that 
>>> BCEL 6 will be released without breaking API's, if I understood right the 
>>> discussion on the apache commons-dev mailing list [8]. If anyone from BCEL 
>>> is listening to this mailing list, it would be nice to get some feedback on 
>>> BCEL 6 plans.
>> 
>> I have done quite a bit of work trying to eliminate the API breaks
>> without compromising the BCEL 6 updates.
> 
> I really appreciate your effort. Please keep it going.
> 
>> Though I have yet to revert the Java and Maven namespace changes as I
>> wanted agreement with the approach first.
>> 
>> From my point of view I would be happy to see a compatible version of
>> BCEL using the original namespaces.
>> I'm not sure what other Commons devs think.
> 
> I hope to see a binary compatible BCEL 6 release, which is might be not 1:1 
> drop-in replacement of BCEL 5 but at least 99%. Some changes must happen, API 
> must evolve, this is natural and no one can keep on the old API forever.
> But! After walking over the all BCEL renamings etc I do not really see a 
> real, functional reason to break *everything*, and a behavior change with 
> annotations parsing is something one can live with for a major release. Not 
> all detectors rely on annotations and BCEL behavior change can probably be 
> fixed in FB core code (so hidden from 3rd party libraries).
> 
>> There are still some Java8/Java9 features that are not fully supported.
>> This is true regardless of the namespace issue.
> 
> That's absolutely fine and understandable. 
> 
> My main goal it to get rid of private BCEL forks which cannot be 
> rebuilt/updated anymore as we see it today, so that we can compile FB on BCEL 
> head, catching all the fixes you will provide in  future BCEL versions. 
> ...And in my ideal world, the new FindBugs release based on BCEL 6 will not 
> break any existing 3rd party FindBugs detectors library, or eventually only 
> require a few trivial changes.
> 
> Thanks!
> 
>>> [0] https://github.com/findbugsproject/findbugs/commits/java9
>>> [1] https://github.com/findbugsproject/findbugs/issues/105
>>> [2] https://github.com/findbugsproject/findbugs/issues/106
>>> [3] https://issues.apache.org/jira/browse/BCEL-222
>>> [4] https://issues.apache.org/jira/browse/BCEL-273
>>> [5] https://github.com/findbugsproject/findbugs/commits/java9_bcel6
>>> [6] 
>>> https://github.com/findbugsproject/findbugs/commit/f9f46bf97c47f011e3757bf9904249faf1039239
>>> [7] https://github.com/iloveeclipse/commons-bcel/commits/old_structure
>>> [8] http://mail-archives.apache.org/mod_mbox/commons-dev/201606.mbox/thread
>>> 
>>> On Sunday 05 June 2016 12:55 Andrey Loskutov wrote:
>>>> Hi all,
>>>> 
>>>> I've got some free time and now working on Java 9 support for FindBugs,
>>>> the first draft works already, but need some more polishing.
>>>> 
>>>> The main goal is to support FB running on Java 9 JRE, to support reading
>>>> Java 9 class files, and to support FB running on Java 8 but analyzing
>>>> Java 9 code. Nice to have (but not in my scope right now) is to support
>>>> any new Java 8/9 constructs like lambdas, type annotations etc.
>>>> 
>>>> I've documented briefly tasks coming to my mind via [1].
>>>> 
>>>> I plan to push my changes on new java9 branch ASAP.
>>>> 
>>>> Main discussion points I see so far:
>>>> 
>>>> 1) We must bump the required JRE for FB to 1.8. I see no reason trying
>>>> to support obsoleted 1.7 JRE. If someone wants run FB on 1.7, the old FB
>>>> 3.0.1 should be used. Objections?
>>>> 
>>>> 2) Since there are no official releases from ASM/BCEL with Java 9
>>>> support yet, we can release first version based on our own FB private
>>>> snapshot versions. The maven folks will cry but this is a chicken and
>>>> egg problem, so I don't care about maven support for the first round (of
>>>> course any help is welcome). Objections?
>>>> 
>>>> 3) Due the JRE version bump I would propose to bump FB version to 3.1.0.
>>>> Objections?
>>>> 
>>>> Please give your feedback either on the lists or on the github task [1].
>>>> 
>>>> [1] https://github.com/findbugsproject/findbugs/issues/105
>>>> 
>>> 
>>> --
>>> Kind regards,
>>> google.com/+AndreyLoskutov
>>> _______________________________________________
>>> Findbugs-discuss mailing list
>>> findbugs-disc...@cs.umd.edu
>>> https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
> 
> -- 
> Kind regards,
> google.com/+AndreyLoskutov
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to