Re: Question about r1496711 which has been silently merged into 2.4.x branch

[Eric Covener]

On Wed, Nov 1, 2017 at 7:58 PM, Christophe JAILLET
 wrote:
> Hi,
>
> not sure on how to deal with the following issue. It has been found in a
> review of some trunk/CHANGES entries in order to remove some backported
> items.
>
>
> In commit r1772331, r1496711 has been silently merged into the 2.4.x branch.
>
> CHANGES for r149671 is:
>
>   *) mod_file_cache: mod_file_cache should be able to serve files that
>  haven't had a Content-Type set via e.g. mod_mime. [Eric Covener]
>
>
> Should this be:
>[1] reverted
>[2] added in the CHANGES of 2.4.x in the revision where it has been
> released
>[3] voted to keep track of it afterward
>[4] updated in the r1772331 commit message
> ?
>
> My guess is [2]+[4]+removal from trunk/CHANGES.
>

I think that's best as well. Thanks for catching it.

I vaguely recall that I later found things that made mod_file_cache
almost certainly unused in practice.

Question about r1496711 which has been silently merged into 2.4.x branch

[Christophe JAILLET]

Hi,

not sure on how to deal with the following issue. It has been found in a 
review of some trunk/CHANGES entries in order to remove some backported 
items.



In commit r1772331, r1496711 has been silently merged into the 2.4.x branch.

CHANGES for r149671 is:

  *) mod_file_cache: mod_file_cache should be able to serve files that
 haven't had a Content-Type set via e.g. mod_mime. [Eric Covener]


Should this be:
   [1] reverted
   [2] added in the CHANGES of 2.4.x in the revision where it has been 
released

   [3] voted to keep track of it afterward
   [4] updated in the r1772331 commit message
?

My guess is [2]+[4]+removal from trunk/CHANGES.

CJ

Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS

[Yann Ylavic]

On Wed, Nov 1, 2017 at 6:34 PM, William A Rowe Jr  wrote:
> You are right, thanks. With the new _ex entry points the backport looks ABI
> clean, nicely done Yann.

Thanks Bill, by taking a last look at the proposed patch, I've noticed
that v5 partially reverted r1747069, a change that you made and
backported to 2.4.21 (IIRC), but which happened later in time than
than my SSLProxy changes in trunk.
I missed that when resolving the conflict around this code in v5.

So I've just updated the proposal to v6 with this (only) change:

--- httpd-2.4.x-r1740928_and_co-v5.patch
+++ httpd-2.4.x-r1740928_and_co-v6.patch
@@ -2069,13 +2046,13 @@ Index: modules/ssl/ssl_engine_io.c
  }
  }
 -else if ((sc->proxy_ssl_check_peer_cn == SSL_ENABLED_TRUE) &&
-+else if ((dc->proxy->ssl_check_peer_cn != FALSE) &&
++else if ((dc->proxy->ssl_check_peer_cn == TRUE) &&
  hostname_note) {
  const char *hostname;
  int match = 0;
_

which restores the existing (and expected) behaviour here.


Regards,
Yann.

Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS

[William A Rowe Jr]

You are right, thanks. With the new _ex entry points the backport looks ABI
clean, nicely done Yann.

On Nov 1, 2017 06:19, "Marion & Christophe JAILLET" <
christophe.jail...@wanadoo.fr> wrote:

> Same analysis here. The 2 declarations are the same.
>
> CJ
>
>
> Le 01/11/2017 à 11:02, Rainer Jung a écrit :
>
>> Hi Bill,
>>
>> Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:
>>
>>> On Mon, Oct 23, 2017 at 10:17 AM,  wrote:
>>>
 Author: ylavic
 Date: Mon Oct 23 15:17:02 2017
 New Revision: 1813027

 URL: http://svn.apache.org/viewvc?rev=1813027=rev
 Log:
 Update comment according to patch version (v5).

 Modified:
  httpd/httpd/branches/2.4.x/STATUS

 @@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
+1: ylavic,
 wrowe: Suspect that this is an MMN Major bump, not minor
 without some
additional detection/workaround of legacy 2.4 compiled
 modules.
 - ylavic: Changed to v4 (+r1812193) which hopefully should address
 compat
 + ylavic: Changed to v5 (+r1812193) which hopefully should address
 compat
issues, Bill?

>>>
>>> That seems to help with the config parser.
>>>
>>> This looks problematic; it is undoubtedly a binary ABI change to a
>>> public interface.
>>>
>>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>>> (request_rec *r,
>>> -  proxy_worker *worker, proxy_server_conf
>>> *conf, char *url,
>>> -  const char *proxyhost, apr_port_t proxyport))
>>> -APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>>> (request_rec *r,
>>> -  char *url))
>>> [...]
>>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
>>> +  (request_rec *r, proxy_worker *worker,
>>> +   proxy_server_conf *conf, char *url,
>>> +   const char *proxyhost, apr_port_t proxyport))
>>> +APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
>>> +  (request_rec *r, char *url))
>>>
>>>
>> This part you cited to me looks like just a reformat (position of line
>> breaks changed), but I couldn't spot a functional change there.
>>
>> Regards,
>>
>> Rainer
>>
>>
>

Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS

[Marion & Christophe JAILLET]

Same analysis here. The 2 declarations are the same.

CJ


Le 01/11/2017 à 11:02, Rainer Jung a écrit :

Hi Bill,

Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:

On Mon, Oct 23, 2017 at 10:17 AM,  wrote:

Author: ylavic
Date: Mon Oct 23 15:17:02 2017
New Revision: 1813027

URL: http://svn.apache.org/viewvc?rev=1813027=rev
Log:
Update comment according to patch version (v5).

Modified:
 httpd/httpd/branches/2.4.x/STATUS

@@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   +1: ylavic,
    wrowe: Suspect that this is an MMN Major bump, not minor 
without some
   additional detection/workaround of legacy 2.4 
compiled modules.
- ylavic: Changed to v4 (+r1812193) which hopefully should 
address compat
+ ylavic: Changed to v5 (+r1812193) which hopefully should 
address compat

   issues, Bill?


That seems to help with the config parser.

This looks problematic; it is undoubtedly a binary ABI change to a
public interface.

-APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler, 
(request_rec *r,

-  proxy_worker *worker, proxy_server_conf
*conf, char *url,
-  const char *proxyhost, apr_port_t proxyport))
-APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler, 
(request_rec *r,

-  char *url))
[...]
+APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
+  (request_rec *r, proxy_worker *worker,
+   proxy_server_conf *conf, char *url,
+   const char *proxyhost, apr_port_t 
proxyport))

+APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
+  (request_rec *r, char *url))



This part you cited to me looks like just a reformat (position of line 
breaks changed), but I couldn't spot a functional change there.


Regards,

Rainer

Re: svn commit: r1813027 - /httpd/httpd/branches/2.4.x/STATUS

[Rainer Jung]

Hi Bill,

Am 31.10.2017 um 21:29 schrieb William A Rowe Jr:

On Mon, Oct 23, 2017 at 10:17 AM,   wrote:

Author: ylavic
Date: Mon Oct 23 15:17:02 2017
New Revision: 1813027

URL: http://svn.apache.org/viewvc?rev=1813027=rev
Log:
Update comment according to patch version (v5).

Modified:
 httpd/httpd/branches/2.4.x/STATUS

@@ -138,7 +137,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   +1: ylavic,
wrowe: Suspect that this is an MMN Major bump, not minor without some
   additional detection/workaround of legacy 2.4 compiled modules.
- ylavic: Changed to v4 (+r1812193) which hopefully should address compat
+ ylavic: Changed to v5 (+r1812193) which hopefully should address compat
   issues, Bill?


That seems to help with the config parser.

This looks problematic; it is undoubtedly a binary ABI change to a
public interface.

-APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler, (request_rec *r,
-  proxy_worker *worker, proxy_server_conf
*conf, char *url,
-  const char *proxyhost, apr_port_t proxyport))
-APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler, (request_rec *r,
-  char *url))
[...]
+APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, scheme_handler,
+  (request_rec *r, proxy_worker *worker,
+   proxy_server_conf *conf, char *url,
+   const char *proxyhost, apr_port_t proxyport))
+APR_DECLARE_EXTERNAL_HOOK(proxy, PROXY, int, canon_handler,
+  (request_rec *r, char *url))



This part you cited to me looks like just a reformat (position of line 
breaks changed), but I couldn't spot a functional change there.


Regards,

Rainer