[squid-dev] [RFC] The situation with helpers/

[Kinkie]

On Tue, Jan 5, 2016 at 6:01 PM, Alex Rousskov
<rousskov at measurement-factory.com> wrote:
> On 01/04/2016 08:58 PM, Amos Jeffries wrote:
>> The SBuf and CharacterSet patch auditing has brought up the questions of
>> why helpers/ is separate from src/, built first, what cross-dependency
>> there can be, and even why we are bundling C++ helpers to begin with.
>>
>> So lets clarify it formally once and for all.
>>
>> Q1: why is helpers/ built before src/ ?
>
> A1: Primarily for historical and paranoia reasons.
>
>
>> Q2: why is helpers/ separate from src/ ?
>
> A2: Primarily for historical and paranoia reasons.
>
>
>> should we shuffle them back to src/*/helpers/ directories on a
>> per-component basis ?
>
> Yes w.r.t. src/. There is no known good reason to enact artificial hard
> barriers between parts of Squid code. All Squid code should be in src/.

Could or could not. For sure it would help with dependency tracking
where needed, and there are cases where it is needed.

> Not sure whether the helpers/ subdirectory is [always] needed but that
> is a separate question.
>
>
>> * it does somewhat hinder anyone wanting to manually build a single
>> helper. Though I have not heard of anyone doing so in many years.
>
> The move does not preclude anyone from building a helper. The fact that
> a few more things may be built along the way is a minor detail IMO.
> Patches to improve dependency tracking (by migrating away from recursive
> Makefiles??) are welcomed.

Ugh. see src/Makefile.am for the monster file this would yield.

>> * it does mean we should probably discard the goal of having the helpers
>> be demo code examples for people to base their custom helpers on.
>
> The location of the helper code is pretty much irrelevant for the above
> goal. However, if we want to provide demo helpers that are easy to
> understand and customize, then we probably should not use C++ to write
> those demo helpers.

I don't agree. We should use whatever tool makes sense, depending on
the availability of libraries, complexity, volunteer time etc.

> * If we want to provide the smallest set of default helpers for the
> widest possible set of deployment environments, then internal
> dependencies are preferred to the external ones and, hence, C++ is the
> best language choice.
>
> * If we want to provide default production-quality helpers for the most
> demanding deployment environments, then internal dependencies are
> irrelevant, and C++ is the best language choice. Please note that I do
> not know whether it is wise to declare performance the primary goal,
> given the performance deficiencies of the helper interface itself and
> the current Squid Project resources.
> Any open source helper can be examined, customized, and cloned, of
> course. Our decision with regard to programming language and
> dependencies may make it easier or more difficult, but not impossible.
> It is a question of the primary optimization target: ease of
> understanding/customizability versus supported platforms versus performance.

I believe our goals should be, in descending order of importance:
1- cover a reasonable set of needs
2- provide a gallery of examples for administrators who have special
needs not covered by the provided set of helpers. Corollaries of this
objective are:
2a- bundled helpers should aim at being as readable as possible.
Whether this qualifies as "high quality" is up for debate :)
2b- bundled helpers should try to avoid relying on Squid internal code
within reason. We can expect administrators embarking in the task to
be competent programmers, asking them to be also competent Squid
coders is maybe too much to ask.
3- performance is nice to have


About build dependencies from src/ : while refactoring some helpers I
came across a recipe which seems reasonably clean and maintainable,
and I submit it for consideration until a more structured approach
(such as lifting helpers/ into src/) can be chosen:

In each helper's Makefile.am, the following clauses must be added:

IMPORTED_FILES = <list of files including paths relative to src/ to be
imported into the helper's directory from src>
nodist_<target>_SOURCES = $(IMPORTED_FILES)
<target>_DEPENDENCIES = $(IMPORTED_FILES)
CLEANFILES += $(IMPORTED_FILES)

$(IMPORTED_FILES): $(top_srcdir)/src/$@
        test -d $(dir $@)|| mkdir -p $(dir $@)
        cp $(top_srcdir)/src/$@ $@

<target> must be replaced with the executable's name. The need for
this replacement unfortunately means that this recipe must be copied
to each helper, and can't be factored out.
This has been tested on the full build farm and satisfies our
portability requirements.

-- 
    Francesco