Support generating multiple variables (within same package). #23

fabstu · 2016-09-12T14:05:31Z

Multiple calls to vfsgendev create all _vfsgen_dirInfo and _vfsgen_fs so its not possible to put multiple http.FileSystems into one package.

How to do this?

mine_js_vfsdata.go|41| _vfsgen_fs redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:43
mine_js_vfsdata.go|43| _vfsgen_fs.Open redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:45
mine_js_vfsdata.go|65| _vfsgen_dirInfo redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:67
mine_js_vfsdata.go|71| (*_vfsgen_dirInfo).Read redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:73
mine_js_vfsdata.go|74| (*_vfsgen_dirInfo).Close redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:76
mine_js_vfsdata.go|75| (*_vfsgen_dirInfo).Stat redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:77
mine_js_vfsdata.go|77| (*_vfsgen_dirInfo).Name redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:79
mine_js_vfsdata.go|78| (*_vfsgen_dirInfo).Size redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:80
mine_js_vfsdata.go|79| (*_vfsgen_dirInfo).Mode redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:81
mine_js_vfsdata.go|80| (*_vfsgen_dirInfo).ModTime redeclared in this block
||  previous declaration at ./mine_css_vfsdata.go:82
mine_js_vfsdata.go|80| too many errors

The text was updated successfully, but these errors were encountered:

dmitshur · 2016-09-13T08:53:42Z

Thanks for reporting this. This is a valid issue.

It's also a known issue, but somehow I forgot to file it. So thanks for doing it.

Yes, with the current setup, a single package can only have one instance of a vfsgen-generated file. That's because each file bundles support types (with methods) with fixed names, so as soon as you put two _vfsdata.go files together in same package, it's a name collision.

I've thought a little about a way to resolve this, but couldn't find anything great so far.

The thing is, this limitation (1 _vfsdata.go file per Go package) is usually not a problem. I usually end up composing my desired filesystem to contain multiple pieces using union http.FileSystem.

See some examples:

After starting to do that, I haven't run into this issue and completely forgot about it.

That said, it would be good to resolve it and allow vfsgen to generate multiple files per single package, but only if it can be done in a way that's good, without negative compromises.

Some ideas for how to resolve this issue:

add an Option field to control if types should be emitted
- this is hard to use API, error-prone
move the types into another package and import it
- this makes the generated file less standalone, requires vendoring or risking breakages
add a suffix to the type names based on filename, to avoid collisions
- each _vfsdata.go file will have its own duplicated copy of the type, slightly messy

fabstu · 2016-09-14T21:24:49Z

God I love your union filesystem!
Had to rewrite my asset program after that:
https://github.com/fabstu/assetgen

I think with it this is not necessary either.

fabstu · 2016-09-18T14:02:07Z

Closing this since its not necessary. 👍

dmitshur · 2016-09-18T21:47:34Z

I feel like resolving this would still be nice, if we can find a great way of doing it. But I agree the need for it is low right now. I'm okay to keep this closed until there's a higher pressing need, but I'll still keep trying to come up with a great solution.

ababo · 2016-12-22T11:37:46Z

This is exactly what I need :(

dmitshur · 2016-12-22T17:53:14Z

@ababo, does the above solution of using the union http.FileSystem not work for you?

ababo · 2016-12-22T17:56:12Z

No since I use one FS for templates which should not be exposed outside, another one for statics. So I had to create two different packages.

dmitshur · 2016-12-22T17:58:12Z

I see. Separate packages certainly works.

If you happen to get an idea for how to make this possible in a good way (without compromising on the nice properties of the current solution), I'd be glad to hear it, but I don't have any ideas myself right now and I've thought about this quite a bit.

ababo · 2016-12-22T21:06:42Z

Probably I don't understand the nature of the problem. Why not to generate code which will depend on your package (and there you can define all the shared types, e.g. file, dir, etc.)?

dmitshur · 2016-12-22T21:50:05Z

Do you mean to factor out all the common types into a package (which currently generate an error as soon as you have two or more instances of them in same package), and have the generated code import that package?

That would work, but it would lose the property of the current generated code being completely standalone. It'd kinda require you to vendor the said package, otherwise risk your generated code going out of sync with the package it imports.

ababo · 2016-12-22T21:59:10Z

If you want to generate completely standalone package your tool can create two files: one for shared definitions, another - for compressed data. The first one will simply be rewritten each time user calls vfsgendev to add a new FS.

dmitshur · 2016-12-22T23:34:49Z

@ababo That sounds like a fantastic idea (and pretty obvious in hindsight, but I couldn't think of it before)... Thank you.

I'll give it a shot.

ababo · 2016-12-22T23:36:05Z

👍

The goal is to resolve #23, to make it possible to use vfsgen multiple times in same package. Previously, that would cause an error because the same types would be defined in each generated file. The approach to solve that is to generate two files: - VFS data file - common type definitions file This avoids the multiple declaration issues in an efficient way. A downside is having 2 files as generated output, instead of one.

dmitshur · 2016-12-23T02:06:37Z

@ababo I have a working prototype of this ready, see #25. Are you interested in trying it out and letting me know if it helps your use case?

ababo · 2016-12-23T20:41:01Z

Seems to be OK. The only (minor) thing I would suggest is to shorten names of generated files. Why not to provide idiomatically laconic names, e.g. <var_name>.go + vfs.go?

dmitshur · 2016-12-23T21:34:03Z

The only (minor) thing I would suggest is to shorten names of generated files. Why not to provide idiomatically laconic names, e.g. <var_name>.go + vfs.go?

I was worried about naming collisions. But I suppose they should be rare enough, so I like your suggestion of short names. It's possible to provide custom filenames if one wants (unless they use vfsgendev tool, then they can't).

Although I think it's nice to be able to spot generated files (that one can safely ignore when looking to edit code) from non-generated ones by looking at the filenames, which becomes harder when the filenames are that short and nice.

dmitshur added the thinking label Sep 13, 2016

fabstu closed this as completed Sep 18, 2016

dmitshur added the enhancement label Sep 18, 2016

dmitshur reopened this Dec 22, 2016

dmitshur removed the thinking label Dec 22, 2016

dmitshur linked a pull request Dec 23, 2016 that will close this issue

WIP: Generate separate data and common files. #25

Open

dmitshur mentioned this issue Apr 25, 2017

Support generating multiple variables #29

Closed

dmitshur changed the title ~~Multiple calls to vfsgendev create all _vfsgen_dirInfo and _vfsgen_fs so its not possible to put multiple http.FileSystems into one package~~ Support generating multiple variables (within same package). Apr 25, 2017

dmitshur mentioned this issue Apr 20, 2018

multiple static paths #44

Open

dmitshur mentioned this issue Dec 2, 2018

Add ForceAllTypes to Options #63

Closed

dmitshur mentioned this issue Apr 22, 2020

Unable to generate multiple VFS into same package #80

Closed

dmitshur mentioned this issue Jun 12, 2020

Change name of output file assets_vfsdata.go? #81

Open

Pothulapati mentioned this issue Nov 23, 2020

tracing: new jaeger independent helm chart linkerd/linkerd2#5240

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support generating multiple variables (within same package). #23

Support generating multiple variables (within same package). #23

fabstu commented Sep 12, 2016 •

edited by dmitshur

dmitshur commented Sep 13, 2016

fabstu commented Sep 14, 2016

fabstu commented Sep 18, 2016

dmitshur commented Sep 18, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 23, 2016 •

edited

ababo commented Dec 23, 2016

dmitshur commented Dec 23, 2016 •

edited

Support generating multiple variables (within same package). #23

Support generating multiple variables (within same package). #23

Comments

fabstu commented Sep 12, 2016 • edited by dmitshur

dmitshur commented Sep 13, 2016

fabstu commented Sep 14, 2016

fabstu commented Sep 18, 2016

dmitshur commented Sep 18, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 22, 2016

ababo commented Dec 22, 2016

dmitshur commented Dec 23, 2016 • edited

ababo commented Dec 23, 2016

dmitshur commented Dec 23, 2016 • edited

fabstu commented Sep 12, 2016 •

edited by dmitshur

dmitshur commented Dec 23, 2016 •

edited

dmitshur commented Dec 23, 2016 •

edited