Replace go-bindata by vfsgen #4430

simonpasquier · 2018-07-27T15:19:37Z

Closes #2411.

Looking at https://tech.townsourced.com/post/embedding-static-files-in-go/ (which was mentioned in the issue), vfsgen has all the needed features.

In particular:

Reproducible builds (no issue with timestamping).
Well maintained and relatively popular.
Integration with go generate.
Self-contained (no external dependency).

grobie · 2018-07-27T16:37:10Z

With go generate support, there isn't any need anymore to check in the generated files, right? That would remove the need to remind contributors to run make assets in pull requests.

simonpasquier · 2018-07-30T13:53:08Z

@grobie I'm not sure. AFAICT it is more a personal preference: some projects like kubernetes still commit all the generated code. I don't have any personal preference though.

simonpasquier · 2018-08-20T13:51:32Z

@brian-brazil can you have a look?

brian-brazil · 2018-08-20T14:20:36Z

web/ui/README.md

-This will put `go-bindata` in DEBUG mode where it serves from your local filesystem.
+To make this work, add `-tags dev` to the `flags` entry in `.promu.yml`, and then `make build`.
+
+This will serves all files from your local filesystem.


brian-brazil · 2018-08-20T14:51:35Z

Makefile

+	cd $(PREFIX)/web/ui && go generate
+
+$(VFSGENDEV):
+	cd $(PREFIX)/vendor/github.com/shurcooL/vfsgen/ && go install ./cmd/vfsgendev/...


I'm not sure we should be modifying the user's environment like this

That's a good point. I've replaced vfsgenby a custom helper program that does the same (just a couple of lines).

brian-brazil · 2018-08-21T15:02:56Z

Makefile

-	@$(GO) get -u github.com/jteeuwen/go-bindata/...
-	@go-bindata $(bindata_flags) -pkg ui -o web/ui/bindata.go -ignore '(.*\.map|bootstrap\.js|bootstrap-theme\.css|bootstrap\.css)'  web/ui/templates/... web/ui/static/...
-	@$(GO) fmt ./web/ui
+	$(GO) get -u github.com/shurcooL/vfsgen


Is this still needed given its vendored?

Yes. I have removed it from vendor/ in the last version because otherwise govendor ilst +unused complains that it isn't used.

That doesn't sound right, assets_generate.go is referencing it.

It has a // +build ignore constraint https://github.com/prometheus/prometheus/pull/4430/files#diff-eb481951f2846eb52fb4b11bfa5b8afdR14

Using a blank import has made govendor happy.

brian-brazil · 2018-08-22T09:45:50Z

👍

grobie · 2018-08-22T10:12:56Z

AFAICT it is more a personal preference: some projects like kubernetes still commit all the generated code. I don't have any personal preference though.

I'm not aware that anyone is actively reviewing the code generated by go-bindata / vfsgen, so I don't see a use case for actually checking in that file. As it's now possible to easily generate the files during compilation time on-the-fly, I'd vote to remove the need to ask every contributor to remember executing make assets.

simonpasquier · 2018-08-22T11:54:16Z

I'd vote to remove the need to ask every contributor to remember executing make assets

@brian-brazil @juliusv what are your views on this?

brian-brazil · 2018-08-22T11:56:24Z

If we've gotten the system to a place where it's no longer required, I see no issue with removing it.

grobie · 2018-08-22T12:07:55Z

Ah, now I remember the issue with go generate, it's not part of go build, so one couldn't longer do go get github.com/prometheus/prometheus/cmd/prometheus to install Prometheus. In that case, maybe leave it as it is right now @simonpasquier.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

This avoids installing vfsgendev in the target environment. Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

simonpasquier · 2018-08-22T14:06:34Z

@grobie yep it seems to be the less disruptive way for now. Also when the generated code isn't committed, we need to add the assets prerequisite to a few targets (eg build, test, ...) as well as modify the CircleCI and Travis CI configurations.

simonpasquier · 2018-08-23T12:23:28Z

@grobie @brian-brazil are you ok merging this? I'll take of migrating AlertManager and Pushgateway too.

brian-brazil · 2018-08-23T12:31:08Z

I already gave a +1

grobie · 2018-08-24T07:03:28Z

Thanks, great work @simonpasquier!

simonpasquier force-pushed the vfsgen-poc branch from 51ada91 to 32127b1 Compare July 30, 2018 13:10

simonpasquier changed the title ~~[WIP] Replace go-bindata by vfsgen~~ Replace go-bindata by vfsgen Jul 30, 2018

This was referenced Jul 30, 2018

Replace go-bindata prometheus/pushgateway#193

Closed

Replace go-bindata prometheus/alertmanager#1487

Closed

web: check that bindata.go is up-to-date #4223

Closed

simonpasquier force-pushed the vfsgen-poc branch 2 times, most recently from 983aa5a to fe42e80 Compare August 7, 2018 14:21

brian-brazil reviewed Aug 20, 2018

View reviewed changes

simonpasquier force-pushed the vfsgen-poc branch from 8393ae6 to 2cc3596 Compare August 21, 2018 14:08

brian-brazil reviewed Aug 21, 2018

View reviewed changes

simonpasquier added 12 commits August 22, 2018 14:27

[WIP] Replace go-bindata by vfsgen

ee1fe53

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Add license + remove doc.go

293c8ba

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Generate templates assets

086e364

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Use new templates assets

0b7d1b6

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

split static assets

2ee3c80

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Idempotent make assets

2ca63ac

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Update vendor/

5e20761

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

vendor vfsgendev

c2d8931

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Update README.md

fba7249

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Simplify assets generation

b8cba15

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Fix README.md

9e80c56

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Use generate helper program instead of vfsgen

4224f25

This avoids installing vfsgendev in the target environment. Signed-off-by: Simon Pasquier <spasquie@redhat.com>

simonpasquier added 5 commits August 22, 2018 14:27

Remove unused vfsgen package

a9ce040

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Fix Makefile

a9c743c

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

vendoring shurcooL/vfsgen

a95da1b

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Fix go generate command

9f51f78

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Sync web/ui/assets_vfsdata.go

e2a487d

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

simonpasquier force-pushed the vfsgen-poc branch from 4efdda4 to e2a487d Compare August 22, 2018 12:28

grobie merged commit 3581377 into prometheus:master Aug 24, 2018

simonpasquier mentioned this pull request Aug 27, 2018

Migrate from go-bindata to vfsgen prometheus/pushgateway#198

Merged

This was referenced Aug 27, 2018

Migrate from go-bindata to vfsgen prometheus/alertmanager#1532

Merged

api: Introduce OpenAPI generated Alertmanager API V2 prometheus/alertmanager#1352

Merged

simonpasquier mentioned this pull request Sep 12, 2018

windows: UI error reading base template #4596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace go-bindata by vfsgen #4430

Replace go-bindata by vfsgen #4430

simonpasquier commented Jul 27, 2018

grobie commented Jul 27, 2018 •

edited

Loading

simonpasquier commented Jul 30, 2018

simonpasquier commented Aug 20, 2018

brian-brazil Aug 20, 2018

brian-brazil Aug 20, 2018

simonpasquier Aug 21, 2018

brian-brazil Aug 21, 2018

simonpasquier Aug 21, 2018

brian-brazil Aug 21, 2018

simonpasquier Aug 21, 2018

simonpasquier Aug 22, 2018

brian-brazil commented Aug 22, 2018

grobie commented Aug 22, 2018

simonpasquier commented Aug 22, 2018

brian-brazil commented Aug 22, 2018

grobie commented Aug 22, 2018 •

edited

Loading

simonpasquier commented Aug 22, 2018

simonpasquier commented Aug 23, 2018

brian-brazil commented Aug 23, 2018

grobie commented Aug 24, 2018

Replace go-bindata by vfsgen #4430

Replace go-bindata by vfsgen #4430

Conversation

simonpasquier commented Jul 27, 2018

grobie commented Jul 27, 2018 • edited Loading

simonpasquier commented Jul 30, 2018

simonpasquier commented Aug 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brian-brazil commented Aug 22, 2018

grobie commented Aug 22, 2018

simonpasquier commented Aug 22, 2018

brian-brazil commented Aug 22, 2018

grobie commented Aug 22, 2018 • edited Loading

simonpasquier commented Aug 22, 2018

simonpasquier commented Aug 23, 2018

brian-brazil commented Aug 23, 2018

grobie commented Aug 24, 2018

grobie commented Jul 27, 2018 •

edited

Loading

grobie commented Aug 22, 2018 •

edited

Loading