[squid-dev] [PATCH] Compilation fix for v5 r14973

[Alex Rousskov]

On 12/12/2016 01:16 AM, Amos Jeffries wrote:
> On 11/12/2016 10:35 p.m., Eduard Bagdasaryan wrote:
>> Attached a patch witch removes SquidConfig  dependency
>> on vector<SchemesConfigs> and uses vector<SchemesConfigs> *.
>> instead. This fixes pinger linking error.

> Attached is an alternative that fits better (but not completely) with
> the HotConf project goals.

This is a [small] step backwards actually: Converting SquidConfig
members into global variables [in the Auth namespace] hurts hot
reconfiguration support because proper hot reconfiguration support
requires storing multiple configuration versions which should be done
using a single Config class (comprised of pointers to module-specific
configuration classes), not many global variables.

As you know, my understanding of how hot reconfiguration should be
supported was developed at [1] and probably differs from some of your
ideas at the time.

[1] http://wiki.squid-cache.org/Features/HotConf/Discussion


> Alex: Care to make the call between one of these two patches or wait a
> day or so more for the full refactor ?

I do not have a strong preference here, but if you want me to rate the
available options, here they are sorted from best to worst, assuming all
of them "work":

1. Auth::Config class containing all authentication options. Please note
that this is not what the current Auth::Config is. AFAICT, the current
Auth::Config should become Auth::SchemeConfig and Auth::Config should
store a container of those, among other things. SquidConfig should
maintain a pointer to an Auth::Config object.

2. Eduard's short-term fix to the current build problems.

3. Your short-term fix to the current build problems. Please do use a
typedef for the std::vector<Auth::SchemesConfig> type instead of
duplicating that code.

If you are going to deliver #1 in a few days and nobody else needs a
working ICMP/whatever until then, then I am OK with waiting.


HTH,

Alex.


>> On 10.12.2016 23:55, Amos Jeffries wrote:
>>> On 11/12/2016 6:12 a.m., Christos Tsantilas wrote:
>>>> I applied the patch, however still exist problem. The icmp pinger does
>>>> not build correctly.
>>>> We should add libsbuf library to pinger libraries, but still there are
>>>> references to HistStat.cc file (maybe add HistStat stub files for
>>>> pinger?).
>>> pinger does not use the Auth:: things, so it really should not pull them
>>> + dependencies in.
>>>
>>> The correct fix I think is to refactor the Auth::Config so that the
>>> various global auth* directives can all be stored there. I'm working on
>>> that right now.
>>>
>>> Amos
>>>