cactus / go-camo

A secure image proxy server
MIT License
255 stars 48 forks source link

examples: Rename Go files to avoid build failure on Heroku #7

Closed grosskur closed 10 years ago

grosskur commented 10 years ago

Currently the build fails with:

-----> Running: godep go install -tags heroku ./...
# github.com/cactus/go-camo/examples
examples/go-hex.go:11: CAMO_HOST redeclared in this block
        previous declaration at examples/go-base64.go:11
examples/go-hex.go:13: GenCamoUrl redeclared in this block
        previous declaration at examples/go-base64.go:17
examples/go-hex.go:26: main redeclared in this block
        previous declaration at examples/go-base64.go:30
godep: go exit status 2
dropwhile commented 10 years ago

hmm. Looking at how heroku-buildpack-go builds, it is indeed overeager and is matching the examples directory, which is not meant to be compiled -- they are indeed just examples. I am not sure it is warranted to rename them given that building via go get and/or the makefile (recommended) will both work as expected -- and renaming breaks any nice syntax highlighting.

Go has some special handling of file partials with _example in them. I will see if that is sufficient to get them ignored by the über globbing.

Aside: In general I recommend building as outlined in the README wherever if possible. Also, as per the readme it is also recommended to use the netgo build tag. It doesn't appear the build pack supports this. As I do not use heroku, I do not know if their native go lib install is also compiled with that option or not.

dropwhile commented 10 years ago

just pushed a change to add // +build ignore to the two files. Hopefully this works as I would prefer to retain syntax highlighting.

If it ends up not working, I think prefixing either the who directory name with an underscore, or the files themselves, should cause the compiler tooling to omit them. If the current solution ends up not working, I will try that next.

Let me know.

grosskur commented 10 years ago

Thank you for looking into this, and sorry for not replying sooner. Definitely agree adding // +build ignore is better than renaming the files. One nit: it needs a subsequent blank line to work. I've updated this pull request to fix that.

I also agree it's weird that heroku-buildpack-go is doing go get ./... rather than go get. And it might be nice if default build tags could be specified in the Godeps config file so that Heroku used netgo by default for go-camo. Right now I'm using a forked version of the buildpack that always adds the netgo build tag. I'll see if I can get patches into heroku-buildpack-go at some point for these things...

BTW, thanks for go-camo! I'm finding it very useful :)

dropwhile commented 10 years ago

Oh man. I always forget that blank line! Thanks for the fix.