You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2017/08/28 18:58:09 UTC

[BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Hi,

Rev. 1782852 [1] has introduced two breaking changes by adding the methods:

public void visitAnnotation(org.apache.bcel.classfile.Annotations)
public void visitAnnotationDefault(org.apache.bcel.classfile.AnnotationDefault)

to the interface org.apache.bcel.classfile.Visitor. The commit comment indicates that these changes are needed by the Tomcat project. How do we want to deal with this for the upcoming 6.1 release? I see several options:

- accept these changes and make it explicit in release notes
- add a new interface which extends from the Visitor interface and add the new methods to that interface
- major version bump (probably not the best idea…)

Thoughts?
Benedikt

[1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <http://svn.apache.org/viewvc?view=revision&revision=1782852>

Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by Gary Gregory <ga...@gmail.com>.
On Aug 31, 2017 23:41, "Benedikt Ritter" <br...@apache.org> wrote:

Hi again,

> Am 31.08.2017 um 01:38 schrieb sebb <se...@gmail.com>:
>
> On 30 August 2017 at 20:11, Benedikt Ritter <britter@apache.org <mailto:
britter@apache.org>> wrote:
>> Hi,
>>
>> looking through the JavaDoc comments in the Visitor interface, it looks
like we added additional methods in the 6.0 release. 6.0 was a major
release but without package and maven coords change. Seems like we accepted
this kind of changes for 6.0. I’m not sure what point I’m trying to make
here… But the whole versioning/api evolution thingy looks pretty messy in
BCEL.
>
> A major version bump does not require changes to package names/maven
coords.
> It just means that there have been major changes.
>
> However changing package/maven coords is a major change and so
> requires a major version bump.
>
> AFAIK 6.0 was binary compatible with the previous release.
> That was certainly the intention.

Looking through the 6.0 release notes [1] I’m thinking about going the same
way again. It seems to be necessary to extend the Visitor interface to
implement new features. Introducing a new VisitorN extends VisitorN-1 very
release seems like overkill. So maybe the best solution to get more
opinions on this is to roll an RC and see what people think.


If it was OK for 6.0, then let's use the same strategy for 6.1 and get
cooking.

Gary


Regards,
Benedikt

[1] https://www.apache.org/dist/commons/bcel/RELEASE-NOTES.txt <
https://www.apache.org/dist/commons/bcel/RELEASE-NOTES.txt>
>
>> Cheers,
>> Benedikt
>>
>>> Am 30.08.2017 um 16:37 schrieb Gary Gregory <ga...@gmail.com>:
>>>
>>> We recently dealt with this kind of compatibility issue in Log4j 2 and
we
>>> decided to subclass the interface in question. I suggest we do the same
>>> here. Not pretty but it is type safe and bullet-proof.
>>>
>>> Gary
>>>
>>> On Tue, Aug 29, 2017 at 5:17 AM, sebb <se...@gmail.com> wrote:
>>>
>>>> On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com>
wrote:
>>>>> The question is whether this breaks binary compatibility. If it does,
>>>> this
>>>>> needs fixing. Unless there are other important changes that warrant a
>>>> major
>>>>> version, I would go the sub-interface route; ugly, but workable within
>>>> the
>>>>> same major release.
>>>>
>>>> According to [1], it seems it does not.
>>>>
>>>> However it will break source compatibility if a 3rd party implements
>>>> the interface.
>>>>
>>>> Also I seem to remember there were some concerns about the downstream
>>>> effect for 3rd party visitor implementations.
>>>>
>>>> IIRC it was that BCEL will expect to be able to invoke the new
>>>> methods; this will fail.
>>>> There may be ways around this by catching the exception.
>>>>
>>>> The dev list should have the details.
>>>>
>>>> [1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html
>>>>
>>>>> Gary
>>>>>
>>>>> On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
>>>>> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Rev. 1782852 [1] has introduced two breaking changes by adding the
>>>> methods:
>>>>>>
>>>>>> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
>>>>>> public void visitAnnotationDefault(org.apache.bcel.classfile.
>>>>>> AnnotationDefault)
>>>>>>
>>>>>> to the interface org.apache.bcel.classfile.Visitor. The commit
comment
>>>>>> indicates that these changes are needed by the Tomcat project. How
do we
>>>>>> want to deal with this for the upcoming 6.1 release? I see several
>>>> options:
>>>>>>
>>>>>> - accept these changes and make it explicit in release notes
>>>>>> - add a new interface which extends from the Visitor interface and
add
>>>> the
>>>>>> new methods to that interface
>>>>>> - major version bump (probably not the best idea…)
>>>>>>
>>>>>> Thoughts?
>>>>>> Benedikt
>>>>>>
>>>>>> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
>>>>>> http://svn.apache.org/viewvc?view=revision&revision=1782852>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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 <mailto:
dev-unsubscribe@commons.apache.org>
>> For additional commands, e-mail: dev-help@commons.apache.org <mailto:
dev-help@commons.apache.org>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org <mailto:
dev-unsubscribe@commons.apache.org>
> For additional commands, e-mail: dev-help@commons.apache.org <mailto:
dev-help@commons.apache.org>

Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by Benedikt Ritter <br...@apache.org>.
Hi again,

> Am 31.08.2017 um 01:38 schrieb sebb <se...@gmail.com>:
> 
> On 30 August 2017 at 20:11, Benedikt Ritter <britter@apache.org <ma...@apache.org>> wrote:
>> Hi,
>> 
>> looking through the JavaDoc comments in the Visitor interface, it looks like we added additional methods in the 6.0 release. 6.0 was a major release but without package and maven coords change. Seems like we accepted this kind of changes for 6.0. I’m not sure what point I’m trying to make here… But the whole versioning/api evolution thingy looks pretty messy in BCEL.
> 
> A major version bump does not require changes to package names/maven coords.
> It just means that there have been major changes.
> 
> However changing package/maven coords is a major change and so
> requires a major version bump.
> 
> AFAIK 6.0 was binary compatible with the previous release.
> That was certainly the intention.

Looking through the 6.0 release notes [1] I’m thinking about going the same way again. It seems to be necessary to extend the Visitor interface to implement new features. Introducing a new VisitorN extends VisitorN-1 very release seems like overkill. So maybe the best solution to get more opinions on this is to roll an RC and see what people think.

Regards,
Benedikt

[1] https://www.apache.org/dist/commons/bcel/RELEASE-NOTES.txt <https://www.apache.org/dist/commons/bcel/RELEASE-NOTES.txt>
> 
>> Cheers,
>> Benedikt
>> 
>>> Am 30.08.2017 um 16:37 schrieb Gary Gregory <ga...@gmail.com>:
>>> 
>>> We recently dealt with this kind of compatibility issue in Log4j 2 and we
>>> decided to subclass the interface in question. I suggest we do the same
>>> here. Not pretty but it is type safe and bullet-proof.
>>> 
>>> Gary
>>> 
>>> On Tue, Aug 29, 2017 at 5:17 AM, sebb <se...@gmail.com> wrote:
>>> 
>>>> On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com> wrote:
>>>>> The question is whether this breaks binary compatibility. If it does,
>>>> this
>>>>> needs fixing. Unless there are other important changes that warrant a
>>>> major
>>>>> version, I would go the sub-interface route; ugly, but workable within
>>>> the
>>>>> same major release.
>>>> 
>>>> According to [1], it seems it does not.
>>>> 
>>>> However it will break source compatibility if a 3rd party implements
>>>> the interface.
>>>> 
>>>> Also I seem to remember there were some concerns about the downstream
>>>> effect for 3rd party visitor implementations.
>>>> 
>>>> IIRC it was that BCEL will expect to be able to invoke the new
>>>> methods; this will fail.
>>>> There may be ways around this by catching the exception.
>>>> 
>>>> The dev list should have the details.
>>>> 
>>>> [1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html
>>>> 
>>>>> Gary
>>>>> 
>>>>> On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
>>>>> wrote:
>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> Rev. 1782852 [1] has introduced two breaking changes by adding the
>>>> methods:
>>>>>> 
>>>>>> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
>>>>>> public void visitAnnotationDefault(org.apache.bcel.classfile.
>>>>>> AnnotationDefault)
>>>>>> 
>>>>>> to the interface org.apache.bcel.classfile.Visitor. The commit comment
>>>>>> indicates that these changes are needed by the Tomcat project. How do we
>>>>>> want to deal with this for the upcoming 6.1 release? I see several
>>>> options:
>>>>>> 
>>>>>> - accept these changes and make it explicit in release notes
>>>>>> - add a new interface which extends from the Visitor interface and add
>>>> the
>>>>>> new methods to that interface
>>>>>> - major version bump (probably not the best idea…)
>>>>>> 
>>>>>> Thoughts?
>>>>>> Benedikt
>>>>>> 
>>>>>> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
>>>>>> http://svn.apache.org/viewvc?view=revision&revision=1782852>
>>>> 
>>>> ---------------------------------------------------------------------
>>>> 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 <ma...@commons.apache.org>
>> For additional commands, e-mail: dev-help@commons.apache.org <ma...@commons.apache.org>
>> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org <ma...@commons.apache.org>
> For additional commands, e-mail: dev-help@commons.apache.org <ma...@commons.apache.org>

Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by sebb <se...@gmail.com>.
On 30 August 2017 at 20:11, Benedikt Ritter <br...@apache.org> wrote:
> Hi,
>
> looking through the JavaDoc comments in the Visitor interface, it looks like we added additional methods in the 6.0 release. 6.0 was a major release but without package and maven coords change. Seems like we accepted this kind of changes for 6.0. I’m not sure what point I’m trying to make here… But the whole versioning/api evolution thingy looks pretty messy in BCEL.

A major version bump does not require changes to package names/maven coords.
It just means that there have been major changes.

However changing package/maven coords is a major change and so
requires a major version bump.

AFAIK 6.0 was binary compatible with the previous release.
That was certainly the intention.

> Cheers,
> Benedikt
>
>> Am 30.08.2017 um 16:37 schrieb Gary Gregory <ga...@gmail.com>:
>>
>> We recently dealt with this kind of compatibility issue in Log4j 2 and we
>> decided to subclass the interface in question. I suggest we do the same
>> here. Not pretty but it is type safe and bullet-proof.
>>
>> Gary
>>
>> On Tue, Aug 29, 2017 at 5:17 AM, sebb <se...@gmail.com> wrote:
>>
>>> On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com> wrote:
>>>> The question is whether this breaks binary compatibility. If it does,
>>> this
>>>> needs fixing. Unless there are other important changes that warrant a
>>> major
>>>> version, I would go the sub-interface route; ugly, but workable within
>>> the
>>>> same major release.
>>>
>>> According to [1], it seems it does not.
>>>
>>> However it will break source compatibility if a 3rd party implements
>>> the interface.
>>>
>>> Also I seem to remember there were some concerns about the downstream
>>> effect for 3rd party visitor implementations.
>>>
>>> IIRC it was that BCEL will expect to be able to invoke the new
>>> methods; this will fail.
>>> There may be ways around this by catching the exception.
>>>
>>> The dev list should have the details.
>>>
>>> [1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html
>>>
>>>> Gary
>>>>
>>>> On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Rev. 1782852 [1] has introduced two breaking changes by adding the
>>> methods:
>>>>>
>>>>> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
>>>>> public void visitAnnotationDefault(org.apache.bcel.classfile.
>>>>> AnnotationDefault)
>>>>>
>>>>> to the interface org.apache.bcel.classfile.Visitor. The commit comment
>>>>> indicates that these changes are needed by the Tomcat project. How do we
>>>>> want to deal with this for the upcoming 6.1 release? I see several
>>> options:
>>>>>
>>>>> - accept these changes and make it explicit in release notes
>>>>> - add a new interface which extends from the Visitor interface and add
>>> the
>>>>> new methods to that interface
>>>>> - major version bump (probably not the best idea…)
>>>>>
>>>>> Thoughts?
>>>>> Benedikt
>>>>>
>>>>> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
>>>>> http://svn.apache.org/viewvc?view=revision&revision=1782852>
>>>
>>> ---------------------------------------------------------------------
>>> 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


Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by Benedikt Ritter <br...@apache.org>.
Hi,

looking through the JavaDoc comments in the Visitor interface, it looks like we added additional methods in the 6.0 release. 6.0 was a major release but without package and maven coords change. Seems like we accepted this kind of changes for 6.0. I’m not sure what point I’m trying to make here… But the whole versioning/api evolution thingy looks pretty messy in BCEL.

Cheers,
Benedikt

> Am 30.08.2017 um 16:37 schrieb Gary Gregory <ga...@gmail.com>:
> 
> We recently dealt with this kind of compatibility issue in Log4j 2 and we
> decided to subclass the interface in question. I suggest we do the same
> here. Not pretty but it is type safe and bullet-proof.
> 
> Gary
> 
> On Tue, Aug 29, 2017 at 5:17 AM, sebb <se...@gmail.com> wrote:
> 
>> On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com> wrote:
>>> The question is whether this breaks binary compatibility. If it does,
>> this
>>> needs fixing. Unless there are other important changes that warrant a
>> major
>>> version, I would go the sub-interface route; ugly, but workable within
>> the
>>> same major release.
>> 
>> According to [1], it seems it does not.
>> 
>> However it will break source compatibility if a 3rd party implements
>> the interface.
>> 
>> Also I seem to remember there were some concerns about the downstream
>> effect for 3rd party visitor implementations.
>> 
>> IIRC it was that BCEL will expect to be able to invoke the new
>> methods; this will fail.
>> There may be ways around this by catching the exception.
>> 
>> The dev list should have the details.
>> 
>> [1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html
>> 
>>> Gary
>>> 
>>> On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
>>> wrote:
>>> 
>>>> Hi,
>>>> 
>>>> Rev. 1782852 [1] has introduced two breaking changes by adding the
>> methods:
>>>> 
>>>> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
>>>> public void visitAnnotationDefault(org.apache.bcel.classfile.
>>>> AnnotationDefault)
>>>> 
>>>> to the interface org.apache.bcel.classfile.Visitor. The commit comment
>>>> indicates that these changes are needed by the Tomcat project. How do we
>>>> want to deal with this for the upcoming 6.1 release? I see several
>> options:
>>>> 
>>>> - accept these changes and make it explicit in release notes
>>>> - add a new interface which extends from the Visitor interface and add
>> the
>>>> new methods to that interface
>>>> - major version bump (probably not the best idea…)
>>>> 
>>>> Thoughts?
>>>> Benedikt
>>>> 
>>>> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
>>>> http://svn.apache.org/viewvc?view=revision&revision=1782852>
>> 
>> ---------------------------------------------------------------------
>> 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


Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by Gary Gregory <ga...@gmail.com>.
We recently dealt with this kind of compatibility issue in Log4j 2 and we
decided to subclass the interface in question. I suggest we do the same
here. Not pretty but it is type safe and bullet-proof.

Gary

On Tue, Aug 29, 2017 at 5:17 AM, sebb <se...@gmail.com> wrote:

> On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com> wrote:
> > The question is whether this breaks binary compatibility. If it does,
> this
> > needs fixing. Unless there are other important changes that warrant a
> major
> > version, I would go the sub-interface route; ugly, but workable within
> the
> > same major release.
>
> According to [1], it seems it does not.
>
> However it will break source compatibility if a 3rd party implements
> the interface.
>
> Also I seem to remember there were some concerns about the downstream
> effect for 3rd party visitor implementations.
>
> IIRC it was that BCEL will expect to be able to invoke the new
> methods; this will fail.
> There may be ways around this by catching the exception.
>
> The dev list should have the details.
>
> [1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html
>
> > Gary
> >
> > On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
> > wrote:
> >
> >> Hi,
> >>
> >> Rev. 1782852 [1] has introduced two breaking changes by adding the
> methods:
> >>
> >> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
> >> public void visitAnnotationDefault(org.apache.bcel.classfile.
> >> AnnotationDefault)
> >>
> >> to the interface org.apache.bcel.classfile.Visitor. The commit comment
> >> indicates that these changes are needed by the Tomcat project. How do we
> >> want to deal with this for the upcoming 6.1 release? I see several
> options:
> >>
> >> - accept these changes and make it explicit in release notes
> >> - add a new interface which extends from the Visitor interface and add
> the
> >> new methods to that interface
> >> - major version bump (probably not the best idea…)
> >>
> >> Thoughts?
> >> Benedikt
> >>
> >> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
> >> http://svn.apache.org/viewvc?view=revision&revision=1782852>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by sebb <se...@gmail.com>.
On 28 August 2017 at 20:07, Gary Gregory <ga...@gmail.com> wrote:
> The question is whether this breaks binary compatibility. If it does, this
> needs fixing. Unless there are other important changes that warrant a major
> version, I would go the sub-interface route; ugly, but workable within the
> same major release.

According to [1], it seems it does not.

However it will break source compatibility if a 3rd party implements
the interface.

Also I seem to remember there were some concerns about the downstream
effect for 3rd party visitor implementations.

IIRC it was that BCEL will expect to be able to invoke the new
methods; this will fail.
There may be ways around this by catching the exception.

The dev list should have the details.

[1] https://docs.oracle.com/javase/specs/jls/se8/html/jls-13.html

> Gary
>
> On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
> wrote:
>
>> Hi,
>>
>> Rev. 1782852 [1] has introduced two breaking changes by adding the methods:
>>
>> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
>> public void visitAnnotationDefault(org.apache.bcel.classfile.
>> AnnotationDefault)
>>
>> to the interface org.apache.bcel.classfile.Visitor. The commit comment
>> indicates that these changes are needed by the Tomcat project. How do we
>> want to deal with this for the upcoming 6.1 release? I see several options:
>>
>> - accept these changes and make it explicit in release notes
>> - add a new interface which extends from the Visitor interface and add the
>> new methods to that interface
>> - major version bump (probably not the best idea…)
>>
>> Thoughts?
>> Benedikt
>>
>> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
>> http://svn.apache.org/viewvc?view=revision&revision=1782852>

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


Re: [BCEL] Breaking changes in 6.1 needed by Tomcat (?)

Posted by Gary Gregory <ga...@gmail.com>.
The question is whether this breaks binary compatibility. If it does, this
needs fixing. Unless there are other important changes that warrant a major
version, I would go the sub-interface route; ugly, but workable within the
same major release.

Gary

On Mon, Aug 28, 2017 at 12:58 PM, Benedikt Ritter <br...@apache.org>
wrote:

> Hi,
>
> Rev. 1782852 [1] has introduced two breaking changes by adding the methods:
>
> public void visitAnnotation(org.apache.bcel.classfile.Annotations)
> public void visitAnnotationDefault(org.apache.bcel.classfile.
> AnnotationDefault)
>
> to the interface org.apache.bcel.classfile.Visitor. The commit comment
> indicates that these changes are needed by the Tomcat project. How do we
> want to deal with this for the upcoming 6.1 release? I see several options:
>
> - accept these changes and make it explicit in release notes
> - add a new interface which extends from the Visitor interface and add the
> new methods to that interface
> - major version bump (probably not the best idea…)
>
> Thoughts?
> Benedikt
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=1782852 <
> http://svn.apache.org/viewvc?view=revision&revision=1782852>