Address some TODOs #2
Conversation
| if os.Getenv("GOPATH") != "" { | ||
| die("to avoid confusion GOPATH must not be set") | ||
| warn("GOPATH envvar is ignored, use --go-path=\"$GOPATH\" option instead") |
There was a problem hiding this comment.
Could you explain your thinking behind keeping this as an arg instead of using the env var? (And similarly for GOROOT)
There was a problem hiding this comment.
Ah, that is going to be bikeshedding I'm afraid. Matter of opinions I think.
I prefer to have just one way of configuring the behavior of the application, in this case - just some parameters. I prefer to reserve env vars to configure behavior of some graphical toolkit or library the application might use. That is - behavior unrelated to what application does.
An example - you might want to configure toolkit with env vars to write some debug info to stdout or to use different backend (X11 or Wayland), but you configure behavior of graphical application by passing a parameter telling which file to open/process/whatever.
And about GOROOT - is there any point in using go 1.3 and specifying GOROOT pointing to root of go 1.4? I think that better idea is to point goaci to using specific go binary, which already knows that it should use its own GOROOT (via go env). Also, requiring specifying GOROOT is bothersome and shouldn't be needed. I don't have it set in my environment.
There was a problem hiding this comment.
Oh, and as of GOPATH - you usually have it set in your environment, but I think that usually you don't want to override it in goaci - clean and reproducible builds should be by default.
There was a problem hiding this comment.
And about GOROOT - is there any point in using go 1.3 and specifying GOROOT pointing to root of go 1.4?
Not sure exactly what you mean here, they should definitely not be different versions
I think that better idea is to point goaci to using specific go binary, which already knows that it should use its own GOROOT (via go env). Also, requiring specifying GOROOT is bothersome and shouldn't be needed. I don't have it set in my environment.
For anyone using a precompiled version of go that's not installed in the default directory (which is not necessarily uncommon), GOROOT is still required. I use it, for one ;-). So --go-binary alone is not sufficient:
$ bin/goaci --go-binary=/home/jon/go/bin/go github.com/coreos/etcd
GOROOT envvar is ignored, use --go-binary="$GOROOT/bin/go" option instead
go: cannot find GOROOT directory: /usr/local/go
error running go: exit status 2
There was a problem hiding this comment.
Oh, and as of GOPATH - you usually have it set in your environment, but I think that usually you don't want to override it in goaci - clean and reproducible builds should be by default.
I think that is reasonable, let's continue to be explicit about that.
|
@krnowak thanks for tackling this! I am a bit ambivalent about adding flags or having things configurable through environment variables. My original idea was to try keep it as flag-compatible as possible with |
|
I don't think it's practical - we are a separate tool, we don't have to mimick any Otherwise the parameters we pass to Oh, and so far |
krnowak
force-pushed
the
krnowak/work
branch
from
bc6d25a to
c7b5f09
Compare
This can be done with "exec" parameter. It can be used more than one time, for instance: goaci --exec=--param1=value1 --exec=--param2 \ --exec 'value2 with spaces' github.com/coreos/etcd
Pass the given package to go get command, otherwise it will get a package from current directory, that is - if running goaci github.com/coreos/etcd from github.com/jonboulle/goaci directory it will get goaci instead of etcd and create an etcd.aci with goaci binary in rootfs.
This was already fixed by using -installsuffix option to go get.
This change introduces --go-binary option, with which user can specify a path to go binary. This change makes goaci to ignore GOROOT environment variable. Pointing to specific go binary is enough - it knows which goroot it should use.
We ignore them anyway - our own set of env vars is passed to exec.Cmd.
This change adds an '--go-path' option for overriding the default GOPATH setting. Normally GOPATH is set to some directory in /tmp, so go get will download the project and its dependencies. This is good for isolation and reproducibility, but inconvenient. Overriding GOPATH with --go-path option allows user to reuse already downloaded project and its dependencies to create ACI. Another upside is creating ACI from a local version of a project without pushing changes to repository.
krnowak
force-pushed
the
krnowak/work
branch
from
c7b5f09 to
556c8e2
Compare
Yeah, I'm convinced. Please fix the GOROOT thing, then this is good to merge! Thanks! |
Most users of prebuilt go have to specify GOROOT env var themselves - prebuilt go insists on having GOROOT in, for example, /usr/local/go.
So far three of them are addressed - about specifying arguments to exec, about setting custom GOPATH and about doing static builds for go-1.4.
This also addresses #1 - GOROOT and GOPATH env vars are ignored.
The third one is problematic from distro point of view - if using go provided by distro (Fedora 21 in my case), then goaci can fail with following message:
go install runtime: mkdir /usr/lib/golang/pkg/linux_amd64_netgo/: permission denied
Not sure if there's a way to specify different pkg directory where libs could be stored. GOPKG env var would be cool (like GOBIN). Specifying installsuffix to something like /tmp/goaci-XXXXXX/netgo resulted in:
go install runtime: mkdir /usr/lib/golang/pkg/linux_amd64_: permission denied
I suppose running goaci for the first time as root might "fix" it, but it's messy - I haven't tried that.
In the meantime I fixed the "go get" command invocation - the package name (github.com/coreos/etcd) wasn't passed there, so go get was actually getting the package from current working directory.
Next step could be addressing the "// TODO(jonboulle): support multiple executables?", but the code now needs refactoring. Currently, almost everything is just in main function.