Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS
Go back through my order of events. If we conditionally compile in
ap_foo() when apr 1.5 is installed, and provide no stub when apr 1.6 is
installed, every module using ap_foo breaks after the user rebuilds httpd
against 1.6.
We promise our users we won't break the binary compatibility of their third
party modules until we ship httpd 2.6 :(
That was the basis of my veto, it can be fixed. My newest question is what
we promise users if the compile their 3rd party mod with httpd 2.4 and apr
1.6... should they expect it to run on the same httpd 2.4 with apr 1.5
installed?
On Jun 8, 2016 1:54 PM, "Ruediger Pluem" wrote:
>
>
> On 06/08/2016 05:31 PM, William A Rowe Jr wrote:
>
> >
> > 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?
>
> Maybe I am confused now, but I understood that the implementation of
> ap_cstr_casecmp() depends on what apr version the
> httpd binary was compiled against. If the httpd binary is compiled against
> 1.5, then we supply our own implementation in
> httpd, if compiled against 1.6 we call what we have in apr. How does it
> matter which apr version was used to compile the
> modules? Or do you want to check the used apr version during runtime and
> decide based on this?
>
> Regards
>
> Rüdiger
>
>
Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS
On 06/08/2016 05:31 PM, William A Rowe Jr wrote:
>
> 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?
Maybe I am confused now, but I understood that the implementation of
ap_cstr_casecmp() depends on what apr version the
httpd binary was compiled against. If the httpd binary is compiled against 1.5,
then we supply our own implementation in
httpd, if compiled against 1.6 we call what we have in apr. How does it matter
which apr version was used to compile the
modules? Or do you want to check the used apr version during runtime and decide
based on this?
Regards
Rüdiger
Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS
On Tue, Jun 7, 2016 at 12:15 AM, William A Rowe Jr
wrote:
> It seems the most critical point of my earlier post was missed...
>
> On Mon, Jun 6, 2016 at 10:08 AM, 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
Re: svn commit: r1747027 - /httpd/httpd/branches/2.4.x/STATUS
It seems the most critical point of my earlier post was missed...
On Mon, Jun 6, 2016 at 10:08 AM, 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
> 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.
It's a lesson from Compatibility 101.
