commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Carman <ja...@carmanconsulting.com>
Subject Re: BCEL 6 API breakage
Date Tue, 07 Jun 2016 10:57:25 GMT
We have to be willing to reevaluate the BC stringency we have had. Is it
working for our users? Is it worth the trouble it causes (people have left
this community over it)? Are there better options? Is it too strict and
could just be relaxed?

Our BC policies sound good on paper and do address things like "jar hell"
very well, but we definitely are out on the fringe. We are the fanatics
when it comes to BC.

On Tue, Jun 7, 2016 at 5:40 AM sebb <sebbaz@gmail.com> wrote:

> On 7 June 2016 at 03:50, Charles Honton <chas@honton.org> wrote:
> > 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.
>
> However, there are ways around this.
> 1) assume code will use the abstract implementations
> 2) use Java 8 default implementations in the interface
> 3) use a sub-interface
> 4) handle the exceptions at run-time
>
> > 5. Whenever binary compatibility is broken, Apache Commons policy is to
> update the Maven GAV to prevent jar hell.
>
> Not exactly, see below.
>
> > 6. Part of updating GAV is to also update the package names.
>
> Avoiding Jar hell for non-Maven users requires package name changes.
> For Maven users the GA coords must correspond 1-1 with Java packages
> otherwise Maven cannot determine which jars can co-exist safely on the
> classpath.
>
> > 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?
>
> Please, no.
>
> > 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.
>
> As I point out above, there are ways to avoid breaking compatibility.
> This requires extra effort, but makes the update much simpler for end
> users.
>
> i.e. it's a trade-off between Commons developer time and everyone else's
> time.
>
> > 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
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message