You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ni...@apache.org on 2006/07/26 10:40:25 UTC

svn commit: r425677 - /httpd/httpd/branches/2.2.x/STATUS

Author: niq
Date: Wed Jul 26 01:40:24 2006
New Revision: 425677

URL: http://svn.apache.org/viewvc?rev=425677&view=rev
Log:
Cast a baleful eye over proposal to ship hacked third-party package

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

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=425677&r1=425676&r2=425677&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Wed Jul 26 01:40:24 2006
@@ -103,6 +103,10 @@
       Fix brokenness on certain platforms when building with -DDEBUG.
       http://svn.apache.org/viewvc?view=rev&revision=381783
       +1 sctemme, fielding
+      -1 niq: Why are we hacking a third-party package as bundled,
+              rather than upstream?  This has potential for chaos
+              for modules (and we have a history of PCRE trouble)
+              as well as a maintenance nightmare!
 
     * mod_isapi: Simply backport the host of fixes for compilation on unix,
       PR#'s 15993 29098 30022 16637 30033 28089



Re: svn commit: r425677 - /httpd/httpd/branches/2.2.x/STATUS

Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 27, 2006 at 10:36:00AM -0700, Sander Temme wrote:
...
> The real solution of course, is to upgrade to the current release of  
> PCRE, version 6.7. However, we should probably not do that on the  
> stable branches.

Agreed.  Given that the pcre already shipped and in the tree is patched 
it's a bit late to start vetoing shipping a patched pcre.  Selectively 
applying patches here seems absolutely fine.

joe

Re: svn commit: r425677 - /httpd/httpd/branches/2.2.x/STATUS

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jul 28, 2006, at 2:31 AM, Nick Kew wrote:

> I'm in two minds about that.  There is the workaround of
> configure --with-pcre
> but where does that leave packages?  PR#27550 names two
> modules that needed to work around the bundled PCRE:
> mod_php and mod_caml.  That implies two workarounds for the
> same problem.  What if the PHP and CAML folks (or AN Other with
> the same problem) were to adopt mutually incompatible solutions?
>
> AFAICT the only mitigating circumstance is that this patch is not
> the underlying problem, so I'm complaining at a symptom rather
> than the cause.  But it feels like "when in a hole, dig deeper".

Your reasoning may be valid for trunk, but not for 2.2.  2.2 is
saddled with a builtin PCRE and it would be foolish not to fix
its bugs to the extent that we can do so without breaking binaries.

I suggest you put the energy into finding a clean way to remove
PCRE from trunk.  Until we do that, "no patches to PCRE" is an
irresponsible position -- we have to maintain the code that we ship.

....Roy


Re: svn commit: r425677 - /httpd/httpd/branches/2.2.x/STATUS

Posted by Nick Kew <ni...@webthing.com>.
On Thursday 27 July 2006 18:36, Sander Temme wrote:

> Have you reviewed the patch? This is a small modification that takes
> unsupported code out of the compile path when building with -DDEBUG.

I'm not happy with applying *any* local patch to a third-party package.
With PCRE we have a quite a track record of trouble, as witness
for example the length of the Cc; list (and the comments!) at
http://issues.apache.org/bugzilla/show_bug.cgi?id=27550

> I have this in all my working copies. It does not break the PCRE
> interface. In the following message to dev@httpd, PCRE author Philip
> Hazel all but admits forgetting to ifdef these calls out in this file
> (which gets #included from pcre.c only if DEBUG is #defined):
>
> http://mail-archives.apache.org/mod_mbox/httpd-dev/200504.mbox/%
> 3cPine.SOC.4.61.0504260951520.22116@draco.cus.cam.ac.uk%3e

It seems to me he (tentatively?) agrees there's a problem.
That's not the same as accepting your patch: it just means
ensuring the underlying problem gets fixed.

> This just makes it easier for folks to build debug builds and help us  
> out:

People building for debug are presumably starting from source.
Where's the problem with
configure --with-pcre?
for them?

> The real solution of course, is to upgrade to the current release of
> PCRE, version 6.7. However, we should probably not do that on the
> stable branches.

At the risk of being a bore, the real solution is to unbundle pcre!
Less confusing for people who compile from source, and perfectly
within the capabilities of any package management system worth
its salt.

> Please reconsider your bale.

I'm in two minds about that.  There is the workaround of
configure --with-pcre
but where does that leave packages?  PR#27550 names two
modules that needed to work around the bundled PCRE:
mod_php and mod_caml.  That implies two workarounds for the
same problem.  What if the PHP and CAML folks (or AN Other with
the same problem) were to adopt mutually incompatible solutions?

AFAICT the only mitigating circumstance is that this patch is not
the underlying problem, so I'm complaining at a symptom rather
than the cause.  But it feels like "when in a hole, dig deeper".

-- 
Nick Kew

Re: svn commit: r425677 - /httpd/httpd/branches/2.2.x/STATUS

Posted by Sander Temme <sa...@temme.net>.
Nick,

On Jul 26, 2006, at 1:40 AM, niq@apache.org wrote:

> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS? 
> rev=425677&r1=425676&r2=425677&view=diff
> ====================================================================== 
> ========
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Wed Jul 26 01:40:24 2006
> @@ -103,6 +103,10 @@
>        Fix brokenness on certain platforms when building with -DDEBUG.
>        http://svn.apache.org/viewvc?view=rev&revision=381783
>        +1 sctemme, fielding
> +      -1 niq: Why are we hacking a third-party package as bundled,
> +              rather than upstream?  This has potential for chaos
> +              for modules (and we have a history of PCRE trouble)
> +              as well as a maintenance nightmare!

Have you reviewed the patch? This is a small modification that takes  
unsupported code out of the compile path when building with -DDEBUG.  
I have this in all my working copies. It does not break the PCRE  
interface. In the following message to dev@httpd, PCRE author Philip  
Hazel all but admits forgetting to ifdef these calls out in this file  
(which gets #included from pcre.c only if DEBUG is #defined):

http://mail-archives.apache.org/mod_mbox/httpd-dev/200504.mbox/% 
3cPine.SOC.4.61.0504260951520.22116@draco.cus.cam.ac.uk%3e

This just makes it easier for folks to build debug builds and help us  
out:

http://mail-archives.apache.org/mod_mbox/httpd-dev/200604.mbox/% 
3c20060422230001.GA16875@doctor.nl2k.ab.ca%3e

The real solution of course, is to upgrade to the current release of  
PCRE, version 6.7. However, we should probably not do that on the  
stable branches.

Please reconsider your bale.

S.

-- 
sander@temme.net              http://www.temme.net/sander/
PGP FP: 51B4 8727 466A 0BC3 69F4  B7B8 B2BE BC40 1529 24AF