Re: [PATCH] remove squid-old.h

On Fri, Aug 17, 2012 at 2:53 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> On 17/08/2012 8:49 a.m., Kinkie wrote:
>>
>> On Thu, Aug 16, 2012 at 6:00 PM, Alex Rousskov
>> <rousskov_at_measurement-factory.com> wrote:
>>>
>>> On 08/16/2012 12:56 AM, Kinkie wrote:
>>>>>
>>>>> I am guessing this is the patch that broke build in numerous places. I
>>>>> am surprised it was not reverted upon the first signs of trouble. Here
>>>>> are some of the errors that I see:
>>>>
>>>> Prior to committing I have run a test-builds.sh on my kubuntu machine
>>>> and it turned out fine; apparently the more recent kernel and/or glibc
>>>> and/or gcc triggers some more permissive include paths.
>>>
>>> AFAICT, the problems are related to missing #includes (a header that was
>>> included via squid-old.h is no longer included). For example,
>>> src/adaptation/ecap/MessageRep.cc is now missing protos.h, which
>>> provides a urlParse() declaration.
>>>
>>> A test build that enables ssl_crtd or eCAP could not succeed so I am
>>> guessing your test-builds.sh does not enable them. I did not check
>>> whether any other features are affected.
>>
>> I agree, that is the expected source of problems right now.
>> "My" test-builds.sh is actually the whole build farm, whose status is:
>> CentOs-5, CentOS-5-icc, ubuntu-precise, debian-squeeze-arm,
>> freebsd-6.4 (west), mandriva, debian-unsatable: OK.
>> FreeBSD-9, OpenSUSE, MacOS-Leopard: fail due to problems in the Rock unit
>> test
>> FreeBSD-9-clang: fail due to strtoll being misdetected
>> OpenBSD(all): fail due to library circular dependencies (Msg/CacheMgr)
>> Freebsd-7.2 (east), debian-sid-clang: fail due to system issues
>> Mswin: compat/ failing to completely fulfill its role.
>>
>> In other words, none in 15 tested platforms shows the issue that
>> you're highlighting. If anything, this means that must the build tests
>> must be furhter improved: what can't be seen can't be fixed, despite
>> the best efforts one can put in place.
>
>
> This sounds like the CRTD was not added to test-builds maximus listing of
> switches to turn on.
>
> I guess its time for another audit of ./configure flags to see what the
> build levels should contain.

I'm testing a farm-build containing the switch and a fix for the error
Alex spotted in my staging branch.
Here's the patch:
---------------------------------------------------------------------
=== modified file 'src/ssl/helper.cc'
--- src/ssl/helper.cc 2012-08-06 17:21:57 +0000
+++ src/ssl/helper.cc 2012-08-17 08:35:33 +0000
@@ -4,6 +4,7 @@

 #include "squid.h"
 #include "anyp/PortCfg.h"
+#include "protos.h"
 #include "ssl/Config.h"
 #include "ssl/helper.h"
 #include "SquidTime.h"

=== modified file 'test-suite/buildtests/layer-02-maximus.opts'
--- test-suite/buildtests/layer-02-maximus.opts 2010-12-10 09:59:02 +0000
+++ test-suite/buildtests/layer-02-maximus.opts 2012-08-17 08:01:54 +0000
@@ -108,6 +108,7 @@
        --with-pic \
        --with-pthreads \
        --enable-build-info=squid\ test\ build \
+ --enable-ssl-crtd \
        "
------------------------------------------------------------------

May I merge it if the test-suite succeeds in staging?

-- 
    /kinkie