wishlist: Permit import multiple times

varnishcache / varnish-cache

Varnish Cache source code repository

https://www.varnish-cache.org

Other

3.68k stars 377 forks source link

wishlist: Permit import multiple times #2687

Closed lkarsten closed 6 years ago

lkarsten commented 6 years ago

[11:37:44] < scn> are "import std;" multiple times allowed now?
[11:38:12] < scn> because i'm in a catch22 situation where I can't test my (autogenerated) configuration because it uses std, and I'm not allowed to import it
                  twice
[11:38:15] < scn> (on 5.2.1)
[11:38:48] < scn> and I'm not going to replace the live configuration files on disk because the obvious power-cut-known-good situation
[11:39:45] < scn> (default.vcl does import std, and later "include conf/all-built.vcl"; I need to run checkconf on all-built.vcl standalone)
[11:58:38] < slink> scn actually i ran into a similar thing lately, I agree that we should just ignore multiple imports
[12:01:46] < daghf> or limit failing them to imports with '.. from "/foo/bar"' that don't agree

(also for other vmods)

bsdphk commented 6 years ago

We need to decide what "identical" and "colliding with" means, but otherwise no objection.

gquintard commented 6 years ago

In VCP we ignore the second import, and it can't have from specifier.

fgsch commented 6 years ago

IOW:

import <something> multiple times should be permitted (subsequent imports ignored).
import <something> from <somewhere> should fail if <something> is already imported, with or without a path.

gquintard commented 6 years ago

IIRC, that's what we do, yes. Want the patch?

fgsch commented 6 years ago

@gquintard please!

bsdphk commented 6 years ago

Why the restriction on "from ..." ?

If the two .so files are the same, why does it matter that one was found with "from ..." ?

rezan commented 6 years ago

I think the challenge is how do you know the 2 .so files are identical? The easy way to do this is to only allow from on the first use of import.

fgsch commented 6 years ago

I think for simplicity we should not compare if the two .so files are identical. Instead we should allow the first import and simple ignore subsequent imports for the same module (much as python does). For import <something> from ... I think it makes sense to have a stricter approach; mixing both or having import <something> from ... multiple times does not help with readability and makes debugging harder for users. This is one of the cases where we should draw the line IMO.

bsdphk commented 6 years ago

Implemented in cb3cc7b39a45119d62cebc0d9bcc41c32582737b.

I use the autogenerated vmod "file_id" as oracle for identity.