On Tue, Jun 7, 2016 at 12:15 AM, William A Rowe Jr <[email protected]> wrote:
> It seems the most critical point of my earlier post was missed... > > On Mon, Jun 6, 2016 at 10:08 AM, <[email protected]> wrote: > >> Author: wrowe >> Date: Mon Jun 6 15:08:01 2016 >> New Revision: 1747027 >> >> URL: http://svn.apache.org/viewvc?rev=1747027&view=rev >> Log: >> Showstopper to patch adoption, along with a more general objection >> >> Modified: >> httpd/httpd/branches/2.4.x/STATUS >> >> Modified: httpd/httpd/branches/2.4.x/STATUS >> URL: >> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1747027&r1=1747026&r2=1747027&view=diff >> >> ============================================================================== >> --- httpd/httpd/branches/2.4.x/STATUS (original) >> +++ httpd/httpd/branches/2.4.x/STATUS Mon Jun 6 15:08:01 2016 >> @@ -203,7 +203,20 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: >> 2.4.x patch: >> http://home.apache.org/~jailletc36/apr_cstr_casecmp_v2.diff > > Note that this is not the apr_cstr_ implementation, so the entire proposal seems a little borked to me. If you are going to borrow the name, why not the implementation? I'm going back to the earlier comparisons to benchmark these two against one another before proposing an alternate patch. jailletc36: v2: update as per discussion on dev@. Do not mix ap_ and >> apr_ >> namespaces + tweak DOXYGEN comments. >> - +1: jailletc36, icing >> + +1: jailletc36, icing >> + -0: wrowe [Prefer that we pre-@deprecate this API and encourage >> users >> + to adopt the apr_ convention, reversing the #defines, >> being >> + ready for the httpd 2.next minor release.] >> + -1: wrowe [Notes this patch creates binary incompatible versions of >> httpd >> + with this change; there must be #else stubs in util.c >> sources >> + compiled against APR 1.6+ for binary compatibility, even >> if >> + these are simply; >> + AP_DECLARE(int) ap_cstr_casecmpn(const char *s1, >> + const char *s2, >> apr_size_t n) >> + { >> + return apr_cstr_casecmpn(s1, s2, n); >> + } >> + ] >> > > You cannot have httpd build in such a way that when one exigent > circumstance > changes, the resulting httpd binary is now a different binary with > different > exported symbols. > > E.g. > > 1. Install apr-1.5 > 2. Build httpd 2.4.recent > 3. Build thirdparty mod_foo, referencing ap_cstr_casecmp() > 4. Pick up, build and install apr-1.6 > 5. Pick up httpd 2.4.latest, build and install over 2.4.recent > 6. Start httpd. LoadModule mod_foo fails, ap_cstr_casecmp() unresolved > 7. httpd fails to start. > > We have a contract with third party modules that, once built against 2.4.x, > the will continue to work unmodified when loaded in 2.4.x+1. > Here's a counter-question I'd like to raise... Right now we are *not* demanding users pick up apr-1.6, we see no reason to force them to during the lifespan of 2.4.x. In httpd 2.6 or 3.0, released after apr 1.6 (or 2.0) has been released, that becomes a completely reasonable requirement. If we stub ap_cstr_casecmp() as apr_cstr_casecmp() when apr 1.6 is detected (keeping an ap_cstr_casecmp() stub for binary compatibility with modules built against apr 1.5)... do we have any concerns that the module compiled against apr 1.6 would then be loaded into an apr 1.5-based build of httpd? Based on the consensus answer to that question, the final patch is straightforward. Bill
