commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Charles Honton <c...@honton.org>
Subject Re: BCEL 6 API breakage
Date Tue, 07 Jun 2016 02:50:13 GMT
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 <loskutov@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 <loskutov@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-discuss@cs.umd.edu
>>> https://mailman.cs.umd.edu/mailman/listinfo/findbugs-discuss
> 
> -- 
> Kind regards,
> google.com/+AndreyLoskutov
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


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


Mime
View raw message