hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Oleg Kalnichevski <ol...@apache.org>
Subject Re: [VOTE] HttpComponents Core 4.2 release based on RC1
Date Wed, 25 Apr 2012 08:36:56 GMT
On Wed, 2012-04-25 at 01:23 +0100, sebb wrote:
> 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.
> 

As I said it is just not worth the trouble.

Oleg



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


Mime
View raw message