vfsgendev cannot handle main package

[fawick]

I have a main package for which I want to compile assets with vfsgendev, but it fails:

$ vfsgendev -source="<path-to-my-package>".Assets
-source flag has invalid value: invalid format, expression &{0xc42006aa80 18 / 0xc42005e2e0} is not a selector expression but *ast.BinaryExpr
Usage: vfsgendev [flags] -source="import/path".VariableName
  -n    Print the generated source but do not run it.
  -source string
        Specifies the http.FileSystem variable to use as source.
  -tag string
        Specifies a single build tag to use for source. The output will include a negated version. (default "dev")

The minimum viable demo case is:

$ find
.
./assets_dev.go
./assets
./assets/hello.txt
./main.go
$ cat main.go
package main

func main() {}
$ cat assets_dev.go
/ +build dev

package main

import "net/http"

// Assets contains project assets.
var Assets http.FileSystem = http.Dir("assets")

[fawick]

For others who run into the same issue: A workaround is to use the dedicated .go file for generation:

// +build ignore

package main

import (
        "log"

        "github.com/shurcooL/vfsgen"

        target "github.com/fawick/vfsmain"
)

func main() {
        err := vfsgen.Generate(target.Assets, vfsgen.Options{
                BuildTags:    "!dev",
                VariableName: "Assets",
        })
        if err != nil {
                log.Fatalln(err)
        }
}

[dmitshur]

Yes, this is a limitation of vfsgendev. It generates Go code that imports the variable you specify, and since main packages cannot be imported, it can't work.

I'm not sure how to work around it other than require users to put assets in a non-main package that can be imported, unfortunately.

It might be possible to work around it by making a copy of the main package and modifying it to be non-main, but that seems quite involved.

[dmitshur]

That said, the error you got seems unrelated to the issue:

$ vfsgendev -source="<path-to-my-package>".Assets
-source flag has invalid value: invalid format, expression &{0xc42006aa80 18 / 0xc42005e2e0} is not a selector expression but *ast.BinaryExpr

What was the exact command you ran?

The error message can be improved to be more clear.

[dmitshur]

Ok, I can reproduce that error by running this in bash:

$ vfsgendev -source="foo/bar".baz
-source flag has invalid value: invalid format, expression &{foo 4 / 0xc4200e2080} is not a selector expression but *ast.BinaryExpr

The issue is how bash interprets that parameter, it drops the quotes. So you'll need to wrap the entire parameter in single quotes for bash:

$ vfsgendev -source='"foo/bar".foo'
2017/11/28 15:16:05 can't import package "foo/bar"

[fawick]

[...] and since main packages cannot be imported, it can't work.

The main package can be imported as long as it is with a identifier. That's what I did in #36 (comment) above where I renamed it to target. I'd assume that should be possible in vfsgendev as well.

[dmitshur]

Interesting. Perhaps I'm misremembering about inability to import main packages (i.e., commands). Maybe what's not possible to use is go get on packages that import commands...

If importing a main package does work, then vfsgendev should also work. Can you try again, but escape the -source parameter in a bash-compatible way?

$ vfsgendev -source='"<path-to-my-package>".Assets'

[dmitshur]

Interesting. Perhaps I'm misremembering about inability to import main packages (i.e., commands). Maybe what's not possible to use is go get on packages that import commands...

Nope, I wasn't. The Go compiler (go1.9.2) refuses to build Go code that imports commands, even if renamed:

package main

import (
	"fmt"

	target "github.com/shurcooL/play/31"
)

func main() { fmt.Println(target.Assets) }

main.go:3:8: import "github.com/shurcooL/play/31" is a program, not an importable package

[fawick]

$ vfsgendev -source='"github.com/fawick/vfsmain".Assets'
/tmp/vfsgendev_406538001/generate.go:8:2: import "github.com/fawick/vfsmain" is a program, not an importable package

It doesn't work here as the import statement is not using a dedicated package name and hence defaults to the name specified in the package (main). If you imported with import target "github.com/fawick/vfsmain"and then explicitely usedtarget` later it would work, I guess.

EDIT: I was wrong. The real difference is where generate.go is stored. My workaround from the comment above only ran because it was in the same directory as the package for which the generation should occur.

[dmitshur]

See my comment above. I tried renaming, it still doesn't work. Not with Go 1.9. It worked with older versions of Go (1.6 or so, I can't remember when exactly it stopped working).

[fawick]

I guess in

vfsgen/cmd/vfsgendev/generate.go

Line 30 in bb654ea

PackageName: {{.PackageName | quote}},

the PackageName struct field of vfsgen.Optionsshould not be set if it target is a main package.

EDIT: No, that was nonsense.

[fawick]

Hmm, the workaround from #36 (comment) only runs if generate.go is in the same directory as the main package. As soon as I move it out of there I get the same error message. At the moment I am at a complete loss why that might be.

[dmitshur]

the workaround from #36 (comment) only runs if generate.go is in the same directory as the main package. As soon as I move it out of there I get the same error message.

Thank you for providing that insight, that is very helpful and explains why I wasn't able to reproduce this previously.

At the moment I am at a complete loss why that might be.

I have a really good guess. Looking at commit golang/go@0f06d0a helped me get to it.

I think the exception, letting main packages be importable from same directory, is there to support external tests in commands. I.e., imagine a command that has a _test.go file that starts with package main_test and proceeds to import the main package. (I'm pretty sure that's allowed, but I haven't confirmed this.)

I will need to think about the implications of that, and whether it's something I can accept to rely on.

[fawick]

Thank you very much for taking the time to investigate! I really appreciate your efforts to get to the bottom of this and help me understand!

From what I can grasp from that commit is that my PR #37 in its current form might be a really bad idea, as future Go versions might add further checks that prevent this from working. With some minor changes, however, it might become a bit more reliable and versatile:

1. The generator template would not write a file that is package main itself, but a vfsgendev_generate_test.go (or any arbitraryly and collisionfreely named _test.go) with a single test function of a random name, e.g. func Test{{.UUID}}(t *testing.T). That function does what main in the template did before.
2. vfsgendev_generate_test.go has a build constraint // +build vfsgendev{{.UUID}}.
3. The test is part of the package that it wants to embed files for, e.g. it is not written as package foo_test, but really of package foo itself.
4. Instead of go run -tags dev, vfsgendev would run go test -run Test{{.UUID}} -tags vfsgendev{{.UUID}}.
5. vfsgendev_generate_test.go is removed.

The advantages I would come to expect are:

• The approach does not conflict with the ways that the Go authors had in mind.
• It works for package main
• Unexported variables in the target package may be used for the asset generation. I believe that was not possible before, right?

What do you think?

[dmitshur]

Using go test to piggyback some code to package main is a very interesting idea, one that I wasn't aware of in the past. Thanks for coming up with it.

I will need at least a few days to think this over.

For me, the biggest concern and part of the decision here is whether I'm willing to move into the territory of creating temporary files in the user's real GOPATH workspace.

So far, I've been very careful to never do that, and only ever create temporary .go files in a temporary directory. This gives me full certainty that I will never inadvertently cause user to lose data, or leave unwanted files behind. That's why I am quite hesitant to write (and delete) anything to the user's real GOPATH.

Another option to consider here may be copying the target package to a temporary directory and modifying it there.

This issue is not very high priority for me at this time, but I will think about it and post updates. If you're in a rush to have something working sooner, I suggest using your fork, and waiting for me to slowly upstream these enhancements. Thanks!

[fawick]

Okay, understood. Take all the time you need. I have a working solution in place that fully covers my use case.

If you are interested, on the weekend I may find time to rerfactor #37 into a draft for the _test.go approach mentioned above. That way you have something to play around with when you are weighing the pros and cons someday.

Just to give my two cents on the rationale:
You are already writing to the user's $GOPATH: the resulting file. IMHO writing a single temporary file in the user's real $GOPATH and then deleting it after it ran is hardly worse than writing the final resulting file. Using a UUID for the filename virtually eliminates the danger of overwriting a user file. And even then the tool could simply log an error "file already exists" and exit early before doing so. Leaving the file behind will not cause problems during compilation or test thanks to the UUID-protected build constraint. A user can run git clean or similar if the file is found to be bothering.