On Wed, 2012-04-25 at 01:23 +0100, sebb wrote: > On 24 April 2012 21:25, Oleg Kalnichevski <[email protected]> wrote: > > On Tue, 2012-04-24 at 21:10 +0100, sebb wrote: > >> On 24 April 2012 20:48, Oleg Kalnichevski <[email protected]> wrote: > >> > On Tue, 2012-04-24 at 20:30 +0100, sebb wrote: > >> >> On 24 April 2012 19:33, Oleg Kalnichevski <[email protected]> wrote: > >> >> > On Tue, 2012-04-24 at 19:03 +0100, sebb wrote: > >> >> >> On 24 April 2012 18:28, Oleg Kalnichevski <[email protected]> wrote: > >> >> >> > On Tue, 2012-04-24 at 18:07 +0100, sebb wrote: > >> >> >> >> On 24 April 2012 17:18, Oleg Kalnichevski <[email protected]> > >> >> >> >> wrote: > >> >> >> >> > On Tue, 2012-04-24 at 16:59 +0100, sebb wrote: > >> >> >> >> >> On 24 April 2012 16:03, Oleg Kalnichevski <[email protected]> > >> >> >> >> >> wrote: > >> >> >> >> >> > On Tue, 2012-04-24 at 15:18 +0100, sebb wrote: > >> >> >> >> >> >> On 24 April 2012 12:13, Oleg Kalnichevski > >> >> >> >> >> >> <[email protected]> wrote: > >> >> >> >> >> >> > On Tue, 2012-04-24 at 02:48 +0100, sebb wrote: > >> >> >> >> >> >> >> On 23 April 2012 14:33, sebb <[email protected]> wrote: > >> >> >> >> >> >> >> > On 21 April 2012 12:21, Oleg Kalnichevski > >> >> >> >> >> >> >> > <[email protected]> wrote: > >> >> >> >> >> >> >> >> Please vote on releasing these packages as > >> >> >> >> >> >> >> >> HttpComponents Core 4.2. The > >> >> >> >> >> >> >> >> vote is open for the at least 72 hours, and only > >> >> >> >> >> >> >> >> votes from > >> >> >> >> >> >> >> >> HttpComponents PMC members are binding. The vote > >> >> >> >> >> >> >> >> passes if at least > >> >> >> >> >> >> >> >> three binding +1 votes are cast and there are more +1 > >> >> >> >> >> >> >> >> than -1 votes. > >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> Packages: > >> >> >> >> >> >> >> >> http://people.apache.org/~olegk/httpcore-4.2-RC1/ > >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> Release notes: > >> >> >> >> >> >> >> >> http://people.apache.org/~olegk/httpcore-4.2-RC1/RELEASE_NOTES.txt > >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> Maven artefacts: > >> >> >> >> >> >> >> >> https://repository.apache.org/content/repositories/orgapachehttpcomponents-078/org/apache/httpcomponents/ > >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> SVN tag: > >> >> >> >> >> >> >> >> https://svn.apache.org/repos/asf/httpcomponents/httpcore/tags/4.2-RC1/ > >> >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> -------------------------------------------------------------------------- > >> >> >> >> >> >> >> >> Vote: HttpComponents Core 4.2 release > >> >> >> >> >> >> >> >> [ ] +1 Release the packages as HttpComponents Core > >> >> >> >> >> >> >> >> 4.2. > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> Sorry, I'm changing my vote: > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> >> [X] -1 I am against releasing the packages (must > >> >> >> >> >> >> >> >> include a reason). > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> I've just noticed that Clirr reports several > >> >> >> >> >> >> >> compatibility issues against 4.1. > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> I've not investigated in any detail, but it looks as > >> >> >> >> >> >> >> though at least > >> >> >> >> >> >> >> some of these are binary compatibility issues, and they > >> >> >> >> >> >> >> appear to be > >> >> >> >> >> >> >> in public APIs. > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> It may be that these are not actually a problem, but I > >> >> >> >> >> >> >> think they need > >> >> >> >> >> >> >> to be investigated further. > >> >> >> >> >> >> >> If the errors are harmless - or perhaps only affect > >> >> >> >> >> >> >> source builds - it > >> >> >> >> >> >> >> would be helpful to update the site (and ideally release > >> >> >> >> >> >> >> notes) > >> >> >> >> >> >> >> accordingly. > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> [No need to cancel the vote just yet, in case I'm wrong.] > >> >> >> >> >> >> >> > >> >> >> >> >> >> >> BTW, we recently added test jars to the Commons Maven > >> >> >> >> >> >> >> output. > >> >> >> >> >> >> >> This should make it easier to run old tests against new > >> >> >> >> >> >> >> releases. > >> >> >> >> >> >> >> > >> >> >> >> >> >> > > >> >> >> >> >> >> > Sebastian > >> >> >> >> >> >> > > >> >> >> >> >> >> > The reported differences in public APIs reported by Clirr > >> >> >> >> >> >> > are due to two > >> >> >> >> >> >> > things (1) upgrade from Java 1.3 to Java 1.5 (2) removal > >> >> >> >> >> >> > of code > >> >> >> >> >> >> > deprecated between 4.0-beta1 and 4.0 (that is, before 4.0 > >> >> >> >> >> >> > GA, more than > >> >> >> >> >> >> > two years ago) > >> >> >> >> >> >> > > >> >> >> >> >> >> > We had a discussion about pros and cons of upgrading to > >> >> >> >> >> >> > Java 1.5 and if > >> >> >> >> >> >> > I remember it correctly you were in favor of that idea > >> >> >> >> >> >> > [1]. The changes > >> >> >> >> >> >> > have also been announced early enough (several releases > >> >> >> >> >> >> > in advance) [2]. > >> >> >> >> >> >> > They do make 4.1 and 4.2 not fully binary compatible but > >> >> >> >> >> >> > I seriously > >> >> >> >> >> >> > doubt there will be a single user affected by > >> >> >> >> >> >> > incompatibility. > >> >> >> >> >> >> > > >> >> >> >> >> >> > I hope you will change your mind. > >> >> >> >> >> >> > >> >> >> >> >> >> I've been looking further at the changes. > >> >> >> >> >> >> The changes to NIO are all removals of deprecated methods, > >> >> >> >> >> >> so not a > >> >> >> >> >> >> problem (or at least, not our problem). > >> >> >> >> >> >> > >> >> >> >> >> >> The removed methods in HttpCore are also deprecated > >> >> >> >> >> >> methods, so not a problem. > >> >> >> >> >> >> > >> >> >> >> >> > > >> >> >> >> >> > Not only were they deprecated, they are deprecated two > >> >> >> >> >> > release cycles > >> >> >> >> >> > back (before 4.0 official release). > >> >> >> >> >> > > >> >> >> >> >> >> Not sure why the value definitions of > >> >> >> >> >> >> HTTP.DEFAULT_CONTENT_CHARSET and > >> >> >> >> >> >> DEFAULT_PROTOCOL_CHARSET were changed. > >> >> >> >> >> >> Given that they are now deprecated, I would have thought > >> >> >> >> >> >> the values > >> >> >> >> >> >> could have been left untouched. > >> >> >> >> >> > > >> >> >> >> >> > I think the case changed (by mistake). I'll fix it right > >> >> >> >> >> > away. > >> >> >> >> >> > > >> >> >> >> >> >> However AFAICT that does not affect compatibility. > >> >> >> >> >> >> > >> >> >> >> >> >> [BTW, in future we ought to document in which release items > >> >> >> >> >> >> are deprecated] > >> >> >> >> >> >> > >> >> >> >> >> >> That just leaves the changed method signatures, which are > >> >> >> >> >> >> due to > >> >> >> >> >> >> adding generics to Iterator in o.a.h.message.Basic*Iterator > >> >> >> >> >> >> and to > >> >> >> >> >> >> AbstractMessageParser. > >> >> >> >> >> >> In the case of the MessageParser subclasses, these were > >> >> >> >> >> >> also changed > >> >> >> >> >> >> to use more specific subclasses: > >> >> >> >> >> >> HttpRequest and HttpResponse instead of their common > >> >> >> >> >> >> super-interface HttpMessage > >> >> >> >> >> >> > >> >> >> >> >> >> It's not obvious to me if these methods are likely to be > >> >> >> >> >> >> called by 3rd > >> >> >> >> >> >> party code or whether they are only likely to be used > >> >> >> >> >> >> internally. > >> >> >> >> >> >> > >> >> >> >> >> > > >> >> >> >> >> > You see, in any sane use case scenarios, especially as far > >> >> >> >> >> > as iterators > >> >> >> >> >> > are concerned, the type returned from those methods would > >> >> >> >> >> > always be cast > >> >> >> >> >> > to the expected subtype. In almost all cases regardless of > >> >> >> >> >> > how those > >> >> >> >> >> > methods are being used the changes will have no effect at > >> >> >> >> >> > the runtime > >> >> >> >> >> > behavior. > >> >> >> >> >> > >> >> >> >> >> The problem is that the return type of a method is part of the > >> >> >> >> >> signature. > >> >> >> >> >> Java won't find the method at runtime if the signature changes > >> >> >> >> >> between > >> >> >> >> >> compilation and run-time. > >> >> >> >> >> > >> >> >> >> >> This generally does not affect source compatibility, but it > >> >> >> >> >> does > >> >> >> >> >> affect binary compat. > >> >> >> >> >> > >> >> >> >> >> We had this exact problem in Commons IO > >> >> >> >> >> We wanted to change a method return from void to something > >> >> >> >> >> else; > >> >> >> >> >> however testing against pre-existing binaries showed that this > >> >> >> >> >> broke > >> >> >> >> >> binary compat. > >> >> >> >> >> > >> >> >> >> > > >> >> >> >> > All right. I'll revert those changes. > >> >> >> >> > >> >> >> >> We are already making the assumption breaking the API is OK for > >> >> >> >> long-deprecated methods, i.e. that user applications have migrated > >> >> >> >> away from the deprecated methods. > >> >> >> >> > >> >> >> >> So if the methods in question are not likely to be used by 3rd > >> >> >> >> party > >> >> >> >> applications - are they effectively internal ? - we could consider > >> >> >> >> releasing with such breaks in compat, provided that such changes > >> >> >> >> are > >> >> >> >> clearly documented. > >> >> >> >> > >> >> >> > > >> >> >> > It is almost as easy just to deprecate the affected classes. > >> >> >> > >> >> >> Which classes need to be deprecated? > >> >> >> > >> >> >> > What is done is done. > >> >> >> > >> >> >> Not sure I follow what you mean here. > >> >> >> > >> >> > > >> >> > I already reverted the changes that made 4.2 binary incompatible with > >> >> > 4.1 with exception of removal of deprecated code. > >> >> > > >> >> >> >> > I always thought the return type did not matter for binary > >> >> >> >> > method calls. Obviously I was wrong. > >> >> >> >> > >> >> >> >> I originally thought the same. It was one of the long-time Commons > >> >> >> >> devs who pointed out the problem. > >> >> >> >> > >> >> >> >> It's particularly strange that changing void to non-void matters, > >> >> >> >> but it does. > >> >> >> >> [Perhaps it was easier than making an exception for that > >> >> >> >> particular case] > >> >> >> >> > >> >> >> > > >> >> >> > I am not sure I understand the point of including the return type > >> >> >> > in the > >> >> >> > method signature since there will always be ambiguity in case > >> >> >> > there is > >> >> >> > no assignment of the method return to a variable. > >> >> >> > >> >> >> Not possible, see below. > >> >> >> > >> >> >> > int i = obj.dostuff(); // returns int > >> >> >> > double d = obj.dostuff(); // returns double > >> >> >> > >> >> >> That's not possible; obj.dostuff() can only have a single return > >> >> >> type (or void). > >> >> >> > >> >> >> Compiler complains about a "duplicate method" otherwise. > >> >> >> > >> >> > > >> >> > Precisely. So, what is the point of including the return type in the > >> >> > method signature? > >> >> > >> >> [I'm guessing here] > >> >> > >> >> The compiler checks that the caller of an API method is using the > >> >> correct return type. > >> >> It's obviously important that the same type is provided at run-time. > >> >> > >> >> If the return type were ignored by the loader when resolving method > >> >> references, the JVM would have to do additional run-time checks on the > >> >> return types. > >> >> This allows the error to be detected earlier (and it's cheaper). > >> >> > >> > > >> > That sounds plausible. Performance does seem to be a factor here. > >> > > >> >> >> > obj.dostuff(); // trouble > >> >> >> > > >> >> >> > That begs the question: what is the point of making things more > >> >> >> > complex > >> >> >> > than necessary. > >> >> >> > >> >> >> I don't think they did make things more complex. > >> >> >> > >> >> > > >> >> > See above. > >> >> > > >> >> >> > Anyway, as soon as you are happy with the content of the release > >> >> >> > notes, > >> >> >> > I'll cut another RC and call a vote. > >> >> >> > >> >> >> I've made some fixes to the parent pom, because unfortunately the > >> >> >> buildNumber plugin with javasvn implementation does not work with SVN > >> >> >> 1.7+ clients. > >> >> >> > >> >> > > >> >> > Yes, I am still on version 1.6.x > >> >> > > >> >> >> I assume you have not yet upgraded, because the > >> >> >> "Implementation-Build" > >> >> >> headers are OK in the Manifests, so that can be fixed later. > >> >> >> > >> >> > > >> >> > Shall I go ahead and cut a new RC? > >> >> > >> >> OK. > >> >> > >> >> The remaining Clirr errors are for deprecated methods only, so I no > >> >> longer have objections on that score. > >> >> > >> >> I see you have created new versions of the AbstractMessageParser > >> >> sub-classes, so the old ones can be kept and deprecated. > >> >> Solves the compat. problem without losing the type improvements. > >> >> Excellent. > >> >> > >> >> It would be great if there were a similar solution for the Iterator > >> >> classes that were reverted, but unfortunately that does not look > >> >> possible, and the classes might well have been used/extended by 3rd > >> >> parties. At least there are type-safe nextEntry() methods available > >> >> for use. > >> >> > >> > > >> > I think similar approach could easily be used for iterator classes. > >> > >> I had a look, and the problem is that at least some of the interfaces > >> are used elsewhere as return types. > >> I don't think it's possible to provide a parallel set of interfaces > >> and still maintain binary compat. elsewhere. > >> > > > > I can be wrong again, but as far as interfaces are concerned we would > > not need to replace them as their generic types get erased anyway. We > > would only have to avoid using old implementations that have Objects as > > the return type embedded in their method signatures. In fact as long as > > the old iterator classes were used through the Itertor<Stuff> interface > > there would be no binary compatibility issues at all. > > I've looked again at the Clirr report and it only complains about the > concrete classes, for example: > > Class: org.apache.http.message.BasicHeaderElementIterator > Return type of method 'public java.lang.Object next()' has been > changed to org.apache.http.HeaderElement > > It would be possible to create an alternate version of > BasicHeaderElementIterator that has a different return type for the > next() method. > Similarly for the other implementations. > > It's not possible to compile the old classes against anything but > Iterator<Object>. > This means that we would need to continue to use casts when using the > Iterator. > I don't know (yet) how this will affect 3rd party code. >
As I said it is just not worth the trouble. Oleg --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
