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

Export GOBIN env var #193

Closed
sslavic opened this issue Dec 15, 2017 · 6 comments
Closed

Export GOBIN env var #193

sslavic opened this issue Dec 15, 2017 · 6 comments
Labels
question Usability question, not directly related to an error with the image

Comments

@sslavic
Copy link

sslavic commented Dec 15, 2017

Please have GOBIN environment var defined in the Golang Docker images, so that go install works without -o switch.

Now it fails with:
go install: no install location for .go files listed on command line (GOBIN not set)

@tianon
Copy link
Member

tianon commented Dec 15, 2017 via email

@sslavic
Copy link
Author

sslavic commented Dec 15, 2017

Yes, it is.

Repro steps:

$ docker run -it --rm --name test golang:1 bash
$ go get github.com/nats-io/go-nats-streaming
$ go install /go/src/github.com/nats-io/go-nats-streaming/examples/stan-pub.go

Last command fails for me with
go install: no install location for .go files listed on command line (GOBIN not set)

If after that I do:

export GOBIN=$GOPATH/bin
go install /go/src/github.com/nats-io/go-nats-streaming/examples/stan-pub.go

Install now passes, binary gets stored in /go/bin.

To fix this, in all golang Docker files probably it should be enough to add after lines like https://github.com/docker-library/golang/blob/master/1.9/stretch/Dockerfile#L46 something like

ENV GOBIN $GOPATH/bin

@yosifkit
Copy link
Member

Later answers in the linked stackoverflow are the key: https://stackoverflow.com/a/36717762 and https://stackoverflow.com/a/39952720.

Actually there are 2 different kinds of behavior.

go install <package>

this is documented in Compile and install packages and dependencies You don't need GOBIN if you set GOPATH correctly.

go install <gofile>

this is not documented and you need GOBIN env variable in this mode.

So I would say that if you are using go install on a .go file, then you should set GOBIN, since it is not a documented use of go install.

$ go install --help
usage: install [build flags] [packages]

Install compiles and installs the packages named by the import paths,
along with their dependencies.

For more about the build flags, see 'go help build'.
For more about specifying packages, see 'go help packages'.

See also: go build, go get, go clean.

@sslavic
Copy link
Author

sslavic commented Jan 13, 2018

@yosifkit thanks for heads up. go install --help mentions

For more about specifying packages, see 'go help packages'.

go help packages among other things states:

As a special case, if the package list is a list of .go files from a
single directory, the command is applied to a single synthesized
package made up of exactly those files, ignoring any build constraints
in those files and ignoring any other files in the directory.

So I understand this that go install is also for .go files. Problem is inconsistency, in case of package name provided GOBIN is not required while in case .go file is being installed GOBIN is being required.
I've reported this issue on golang/go#23439

Another problem is that github.com/nats-io/go-nats-streaming/examples has multiple apps, each should be in it's own package/dir for current go install for each app to work with current go and without GOBIN being set.
For that I reported https://github.com/nats-io/go-nats-streaming/issues/164

Still I think it would not only not harm to have GOBIN explicitly set in golang Docker images, it would be helpful to better handle the inconsistencies.

@wglambert wglambert added the question Usability question, not directly related to an error with the image label Apr 25, 2018
@tianon
Copy link
Member

tianon commented May 11, 2018

If we set GOBIN in the image, then images which set GOPATH to something other than our default value (or add an additional path) will need to either unset GOBIN or modify GOBIN to point to their new directory, so I'm still not comfortable adding it to the image (since it will automatically default to GOPATH/bin, which will change depending on which component of the multi-directory GOPATH value we're building in, etc).

@sslavic
Copy link
Author

sslavic commented May 13, 2018

NATS Streaming examples have been fixed, each example was refactored to be in it's own package/dir so documented go install <package> variant works without GOBIN being defined.

It seems fix for inconsistency in go install variants is scheduled for Go 1.11 so GOBIN will likely not be needed also for the variant when .go file is passed as argument.

I also understand concerns @tianon mentioned.

AFAIC this issue can be closed.

@sslavic sslavic closed this as completed May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Usability question, not directly related to an error with the image
Projects
None yet
Development

No branches or pull requests

4 participants