hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [VOTE] HttpComponents Core 4.2 release based on RC1
Date Wed, 25 Apr 2012 00:23:51 GMT
On 24 April 2012 21:25, Oleg Kalnichevski <olegk@apache.org> wrote:
> On Tue, 2012-04-24 at 21:10 +0100, sebb wrote:
>> On 24 April 2012 20:48, Oleg Kalnichevski <olegk@apache.org> wrote:
>> > On Tue, 2012-04-24 at 20:30 +0100, sebb wrote:
>> >> On 24 April 2012 19:33, Oleg Kalnichevski <olegk@apache.org> wrote:
>> >> > On Tue, 2012-04-24 at 19:03 +0100, sebb wrote:
>> >> >> On 24 April 2012 18:28, Oleg Kalnichevski <olegk@apache.org>
wrote:
>> >> >> > On Tue, 2012-04-24 at 18:07 +0100, sebb wrote:
>> >> >> >> On 24 April 2012 17:18, Oleg Kalnichevski <olegk@apache.org>
wrote:
>> >> >> >> > On Tue, 2012-04-24 at 16:59 +0100, sebb wrote:
>> >> >> >> >> On 24 April 2012 16:03, Oleg Kalnichevski <olegk@apache.org>
wrote:
>> >> >> >> >> > On Tue, 2012-04-24 at 15:18 +0100, sebb
wrote:
>> >> >> >> >> >> On 24 April 2012 12:13, Oleg Kalnichevski
<olegk@apache.org> wrote:
>> >> >> >> >> >> > On Tue, 2012-04-24 at 02:48 +0100,
sebb wrote:
>> >> >> >> >> >> >> On 23 April 2012 14:33, sebb
<sebbaz@gmail.com> wrote:
>> >> >> >> >> >> >> > On 21 April 2012 12:21,
Oleg Kalnichevski <olegk@apache.org> 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.

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

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


Mime
View raw message