On 08/25/2012 06:15 PM, Cyril Brulebois wrote:
> Hi Tony,
> 
> tony mancill <tmanc...@debian.org> (24/08/2012):
>> Please consider an unblock for package tomcat-native due to #685516.
>> The current version in wheezy (1.1.23) is incompatible with the
>> version of tomcat7-7.0.28 in wheezy.
> 
> that's the kind of things which would have been nice to have in the
> changelog, we have:
> |  * New upstream release (closes: #685516)
> 
> instead, that'd be better:
> |  * New upstream release:
> |    - tomcat7 is wheezy (7.0.28) is incompatible with tomcat-native 1.1.23,
> |      it requires 1.1.24 or higher (Closes: #685516).
> 
> That would save everyone a BTS lookup.

No argument there.

>> tomcat-native version 1.1.24 is compatible with and has been tested
>> with both tomcat6-6.0.35 and tomcat7-7.0.28.  We recognize that the
>> requesting a new upstream version is typically not permitted during
>> the freeze, but in this case the differences between upstream versions
>> aren't extensive and the package is intended for use in conjunction
>> with tomcat6 and tomcat7 (I am not aware of other uses), so it seems a
>> shame to release with a version incompatible with tomcat7.
> 
> Yep, this one doesn't look too insane. It would have been nice to
> include a source debdiff, stripping out the *.html and configure
> changes.
> 
> ((
> I hope the latter really is due to just that:
> -# Generated by GNU Autoconf 2.59.
> +# Generated by GNU Autoconf 2.68
> ))

Well, I kind of assumed that folks would look at the debdiff between the
packages (which will necessarily include all of this) anyway.

> Out of curiosity, if the primary use is with tomcat6 and tomcat7, how
> come it wasn't spotted earlier? What if that version brings subtle
> regressions? Will anyone notice/fix in a timely fashion?

The primary (only?) use of tomcat-native is to increase the performance
of tomcat - that is, running some function in native code in the APR
library via a JNI interface.  It isn't required for tomcat to function,
but is used in many environments.  (I don't use it mine.)  As for why it
wasn't spotted earlier, that's an issue with team-maintenance.  We're
learning, we're trying, and in the future I'll know to test these as a
suite.  If there are subtle regressions, then they'll mostly likely be
reported via the Apache site (or in Debian).  I think the possibility of
a regression is preferable to being completely incompatible with
tomcat7.  For the lifetime of wheezy, tomcat7 should (will hopefully) be
the predominate server choice.  (There was actually talk of dropping
tomcat6 for wheezy for security reason, but we're optimistic that Ubuntu
LTS support will help with the maintenance.)

> Looking at the diff (please bear in mind I know little to nothing in
> java/jni, so those are naïve questions):
>  - is tcn_socket_t for internal use only or is it exposed outside the
>    library?
>  - same question for tcn_pollset_t.
>  - is it ok to get rid of some functions, like update_ttl?
> 
> If answers are 2*internal+yes, we should be able to unblock it.

Taking a close look at the upstream source diff, I think the changes are
only additive:

These 2 are equivalent:

> +TCN_IMPLEMENT_CALL(jint, Poll, add)(TCN_STDARGS, jlong pollset,
> +                                    jlong socket, jint reqevents)

> -TCN_IMPLEMENT_CALL(jint, Poll, add)(TCN_STDARGS, jlong pollset,
> -                                    jlong socket, jint reqevents)

These are new:

> +TCN_IMPLEMENT_CALL(jint, Poll, addWithTimeout)(TCN_STDARGS, jlong pollset,
> +                                               jlong socket, jint reqevents,
> +                                               jlong socket_timeout)

> +static apr_status_t do_add(tcn_pollset_t *p, tcn_socket_t *s,
> +                           apr_int16_t reqevents,
> +                           apr_interval_time_t socket_timeout) {


Here is the change you're referring to:

> -static void update_ttl(tcn_pollset_t *p, const apr_pollfd_t *fd, apr_time_t 
> t)
> +static void update_last_active(tcn_pollset_t *p, const apr_pollfd_t *fd, 
> apr_time_t t)

update_ttl() (and it's replacement, update_last_active()) are only used
in poll.c and is declared as static, so I don't see how that would
change users of the library.

Thank you for your consideration!
tony

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to