New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vfsgendev should be able to omit non-inverted build tags #64

Open
slimsag opened this Issue Dec 13, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@slimsag
Copy link
Contributor

slimsag commented Dec 13, 2018

Today, vfsgendev has a -tag flag documented as follows:

  -tag string
    	Specifies a single build tag to use for source. The output will include a negated version. (default "dev")

Unfortunately, this means that you cannot use a non-negated tag for the output file. For example, today you can use the default dev tag and it produces the following behavior:

  • go test ./some/pkg -> uses static assets
  • go test -tags=dev -> uses FS assets

This is annoying because often times it is desirable to not commit the assets_vfsdata.go file to your repository and instead only generate it as part of build/release time. This means your repository does not contain a assets_vfsdata.go file and you must add -tags=dev to every command (like go test, linters, etc) in order for them to operate as expected. I want to invert this such that:

  • go test ./some/pkg -> uses FS assets
  • go test -tags=dist -> uses static assets

But this is not possible because vfsgendev -tag=dist would emit a negated !dist tag.

Proposal:

  • Change the -tag flag such that it is not inverted. This would be a breaking change, but I suspect most users of vfsgendev today are using the default value anyway, so this change is unlikely to affect most users.
  • Change the default value of -tag to !dev, so the default behavior of vfsgendev remains the same.
  • After this change, vfsgendev -tag=dist would allow performing the above.

Other thoughts:

Would switching from !dev to dist by default make sense?

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Dec 15, 2018

Thanks for opening this issue and describing the problem you're facing. I understand that the current vfsgendev is indeed not providing a good experience for your use case. I have an idea for a different solution, let me describe it.

The package comment of vfsgendev describes it as:

vfsgendev is a convenience tool for using vfsgen in a common development configuration.

The common development configuration it refers to is described at https://github.com/shurcooL/vfsgen#go-generate-usage. vfsgendev is a simple tool that was created specifically for that development configuration: one where the filesystem is read only when a specific build tag is used (e.g., dev), and generated FS is expected to be the default when no tags are provided. Hence, outputting a negated version of the input build tag (e.g., !dev) is the right thing to do.

It consists of just 3 short .go files:

  • generate.go - a template for generating a .go file to invoke vfsgen
  • parse.go - helper code to parse command line flags and extract documentation from the target http.FileSystem variable
  • main.go - ties everything together: parses command line flags, creates a .go file in a temporary directory and executes it

I think it would be a mistake to try to change vfsgendev to support more than one development configuration. Instead, it seems much easier and better to simply create another vfsgendev-like tool that is equally opinionated and specialized towards a different development configuration, the one you want where the FS is accessed when no build tags are used, and the generated FS is used with a build tag like dist.

Perhaps vfsgendist would be a good name for such a tool, and it can be easily created by copying vfsgendev code (which is quite stable and doesn't change often) and modifying it to target the desired development configuration.

What do you think of the suggested approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment