relative imports and duplicate includes

[Kacper Wysocki]

On Tue, Nov 17, 2015 at 3:58 PM, Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
> So, this is one of those procedural things...
>
> It might have been better to open a discussion with the problem
> you're trying to solve, rather than throw out a patch that does
> something with only passing mention of the problem and no
> analysis of it :-)

Sorry, perhaps I got carried away researching the kinks. Before
opening a discussion, I felt I needed to map it to varnish internals,
but I'm not emotionally tied to the patch; it is just a product of
sleepless nights and an itch to scratch.

Making a functional solution to this problem, besides letting me
figure out the special cases, also made me realise what
limitations&facilities there are in the existing code.

It's first now that I've tried it out that I feel we can discuss the
issue without me treading my foot into some proverbial salad...

>> An instance of the problem I have out there is
>> a multi-site varnish has vcl snippets site1/main.vcl which includes
>> site1/foo.vcl and site1/bar.vcl and site2/main.vcl which includes
>> site2/foo.vcl site2/zool/baz.vcl. Today these includes have to either
>> be absolute, or relative to vcl_dir.

>  Now we're talking...

If you like that, consider VSF as another specific instance;
to me VSF feels more like a special case important to the few people
who use Varnish as a programming platform rather than the more general
case of ops people using Varnish to bring some happiness into their
ops, so the multi-site problem instance may be more common.

>>> What if one is:
>>>
>>>         import "foo";
>>>
>>> and the other is:
>>>
>>>         import "foo" from /somewhere/libvmod_foo.so";
[snip is this a problem?]
> Yes, but if some/other/vcl expects vmod_foo to be one thing and another
> already imported something different under that name, you are screwed
> and will get *really* weird error messages.

I get what you are saying.
With my simplistic patch there is no validation of the sameness of the
duplicate imports, and checking sameness seems a little heavy-handed.

You'll get messages like "no such symbol bar in vmod foo" which aren't
really that weird, and having two different vmods named the same is
nearly guaranteed to cause confusion anyway.

However, the user might at least expect a "Warning: vmod foo imported
twice, previous import here" message.

> The current "include" facility is 100% textual and you can do stuff
> like:
>
>         if (include condition.vcl;) { ... }
>
> Which is incredibly powerful and 100% unstructured.

Yes, imports and includes are not related except by my problem statement.

Includes are simple and powerful and that's the reason I like it. I'd
prefer it stay this way.
The include patch does not change this use.

> Both of the two issues you raise actually sound like something
> entirely different is needed, some kind of "local namespaces",
> so that the "include foo_lib.vcl" can have its own vmods and includes,
> which do not affect or spill out over the rest.

Yes, namespaces is another way to go. Kristian even mentions this, and
I am assuming he doesn't want to go there because it's a heavy-handed
approach to take for such a simple language. I would prefer a solution
that mirrors the simplest facility other languages have.

Each included source file already has a path, making it intuitive that
relative include-statements source from that path. For VCL that
doesn't have a path there is the vcl_dir.

I won't knock language features with merit though!

> Maybe what we really need is VCL libraries:
>         library geoip geoip.vcl [from "path"];
>         ...
>         sub vcl_recv {
>                 geoip.recv_stuff();
>         }

Maybe? From a programmer perspective this is certainly cleaner.
However,  the people who run Varnish out there are primarily concerned
with ops problems. Will the library/namespaces approach resonate with
the ops folks?