Skip to content
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

Custom build args to revel package #10

Closed
viblo opened this issue Aug 6, 2014 · 13 comments
Closed

Custom build args to revel package #10

viblo opened this issue Aug 6, 2014 · 13 comments

Comments

@viblo
Copy link

viblo commented Aug 6, 2014

Please add a way to pass build flags when running / building with revel.

I have a number of (rest)services all using the same structure and then one web site made with revel. For the services I have a makefile which pass in date and git revision as ldflags, and a couple of build tags.

In the end it becomes something like this

go install -tags="gm no_development" -ldflags "-X myorg/version.date $(DATE) -X myorg/version.version $(GITREV)"

I would like the same to be passed to our revel web site, so that I can reuse our existing code to track version and compilation date, and so that we can use 3rd party libs which use build tags.

@brendensoares
Copy link
Member

That sounds like a reasonable request, but can you not make use of the existing app version method (https://github.com/revel/revel/blob/master/harness/build.go#L120)?

Where are you using build date in your app? I could see that being useful enough for others to justify adding it to Revel.

If custom flags are still useful, we could add it to app.conf in the Revel framework's harness code since that is what builds the actual Revel app binary.

Thoughts? @revel/core chime in?

@viblo
Copy link
Author

viblo commented Aug 18, 2014

Let me expand a little about our use case.

We have a rather large number of go services with rest endpoints which is not using revel, and then I recently converted a very basic php admin web to use revel instead as an experiment.

To build our services for "release" we use a makefile, like this:

VERSIONCMD = "`git symbolic-ref HEAD | cut -b 12-`-`git rev-parse HEAD`"
VERSION = $(shell echo $(VERSIONCMD))
DATE = $(shell echo `date +%FT%T%z`)
LDFLAGS += -X myorg/version.version $(VERSION) -X myorg/version.date $(DATE)

build:
    export GOPATH="$(CURDIR)/vendor:$(CURDIR)/../.." && \
    go install  -tags="$(BUILD_TAGS)" -ldflags "$(LDFLAGS)" myorg/cmd/... && \

And inside myorg/version:

package version

import (
    "flag"
    "fmt"
    "html"
    "io"
    "net/http"
    "os"
)

var showVersion = flag.Bool("version", false, "Print version of this binary (only valid if compiled with make)")

var (
    version string
    date    string
)

func printVersion(w io.Writer, version string, date string) {
    fmt.Fprintf(w, "Version: %s\n", version)
    fmt.Fprintf(w, "Binary: %s\n", os.Args[0])
    fmt.Fprintf(w, "Compile date: %s\n", date)
    fmt.Fprintf(w, "(version and date only valid if compiled with make)\n")
}

// Init initializes the version flag and /debug/version/ http endpoint.
// Note that this method will call flag.Parse if the flags are not already
// parsed.
func Init() {
    if !flag.Parsed() {
        flag.Parse()
    }
    if showVersion != nil && *showVersion {
        printVersion(os.Stdout, version, date)
        os.Exit(0)
    }

    http.Handle("/debug/version/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        printVersion(w, html.EscapeString(version), html.EscapeString(date))
    }))
}

With this setup if you ever want to know what version a specific binary is, you just call it with the -version flag and you will get output like this:

➜  bin  ./myorg-service -version
Version: branchname-ea856cf5961b4f45394eddf15861d087bafb4c01
Binary: ./myorg-service
Compile date: 2014-08-05T11:26:25+0800
(version and date only valid if compiled with make)

We use the same setup for everything and it helps us keep track of exactly whats deployed, and we have found it a very useful know exactly when a binary was compiled and what version of the code it contains.

Then for build tags we also need specific tags. We use this imagemagick lib: https://github.com/rainycape/magick
To specify if you want to link with graphicsmagick you pass it the build tag gm and otherwise imagemagick will be used. Therefor we must be able to specify build tags when building. (Now it happens that no imagehandling is done from our small revel experiment, but we will need it in the future if we are to use revel for more)

I would prefer if we could reuse our current solution also for our new revel web.

@brendensoares
Copy link
Member

This looks like a nice solution. Well done!

First a disclaimer: I am not the author of Revel, I simply have volunteered to maintain it. I'm more of a steward. That said, I don't have an expert knowledge of ALL of Revel's code.

My concern is that you may not be able to build Revel manually due to how routes, modules, etc are compiled. See https://github.com/revel/revel/blob/master/harness/build.go#L74

The best we can do, off the top of my head, is inject ldflags via the app.conf.

Alternatively, we could "pop the why stack" and understand what you're trying to accomplish. It seems to me that you're trying to is inject a version into the binary which we already support via git describe or a environment variable (See https://github.com/revel/revel/blob/master/harness/build.go#L120). The only thing we're missing is a build date.

If we were able to accommodate these needs, would that negate your need for ldflags?

@viblo
Copy link
Author

viblo commented Aug 27, 2014

Re build tags: Thanks to your link I figured out its already possible to define build tags with build.tags in the app.conf (and now I see they are also mentioned in the documentation). Totally workable.

Re ldflags: Right, with the APP_VERSION its only compile date that is missing. Since version is already there I guess it makes sense to also add date in a similar way? Like for example APP_COMPILE_DATE (but without environment variable override).

On the other hand, maybe ldflags in general would be good to have in app.conf like build tags, other people could have some use for them.

I should add that Im not totally sold on the idea of having these compile time configuration values in the app.conf. If I change for example http.port or app.secret I would expect the app to use the new value after a restart. But that wont work for these values which can make it confusing.

On the other hand I see that there is a text in the docs under "Areas for development" that say "Allow inserting command line arguments as config values or otherwise specifying config values from the command line." If that was available then all would be good I think.

@brendensoares
Copy link
Member

Glad you found a workable solution. This is something we can consider down the road to help developers gain more control over the output of the Revel build process.

@viblo
Copy link
Author

viblo commented Aug 28, 2014

I just realized that I forgot one part. With my current solution I can pass -version to the binary and it will print out the version and exit, before it has done much work. This is useful when you have a binary that might miss some things (for example in case of revel you might have the compiled binary by itself without any support files such as templates) but still want to check what version it is.

Is something like that possible currently?

@brendensoares
Copy link
Member

No, there is no flag support for version, but it wouldn't be hard to add it. We'll need to make changes on the revel/revel repo to support it. From this repo I would say there are 2 goals:

  • Add version flag to revel app binary that outputs version and build date details, like what you have in your custom implementation
  • Add build date tracking similar to app version

Anything else?

@viblo
Copy link
Author

viblo commented Sep 1, 2014

Those two things should be all.

However, about the -version flag. Another option is to make it possible to hook into the flag thing myself so that I can add flags at will, and have somewhere to read them and process them early. I tried to add them to the init function in app/init.go, but there they are not parsed, and I cant parse the flags because not all other flags are defined at that point. So I guess that this would be more work to implement..

@brendensoares
Copy link
Member

@viblo that's a possible enhancement in the future :)

@ptman
Copy link
Contributor

ptman commented May 26, 2016

I'd also like to change -ldflags, primarily to add -s -w in order to produce smaller binaries. Also CGO_ENABLED, would be interesting to set, but that is kept from the environment currently.

@ptman
Copy link
Contributor

ptman commented Jun 6, 2016

Actually, with the release of 0.13 I ran into an issue with https://github.com/revel/cmd/blob/master/harness/build.go#L83 as well. I use a distro provided go compiler and compile using CGO_ENABLED=0. This fails with

go install net: open /usr/lib/go/pkg/linux_amd64/net.a: permission denied

unless I remove the -i argument. This should probably also be configurable.

@brendensoares
Copy link
Member

Reviewing open issues, this seems like it would still be valuable. Now it's just a matter of coordinating a solution.

@notzippy
Copy link
Contributor

Grouped with #81

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

No branches or pull requests

5 participants