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

cmd/go: default GOBIN to GOPATH[0]/bin when outside of GOPATH? #23439

Open
sslavic opened this issue Jan 13, 2018 · 17 comments
Open

cmd/go: default GOBIN to GOPATH[0]/bin when outside of GOPATH? #23439

sslavic opened this issue Jan 13, 2018 · 17 comments
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@sslavic
Copy link

sslavic commented Jan 13, 2018

What version of Go are you using (go version)?

1.9.2

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Linux, amd64

What did you do?

$ 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

What did you expect to see?

/go/bin/stan-pub binary installed, consistent with the requirements when go install <package> is used, just GOPATH is set and that should be enough, GOBIN should not be required.

What did you see instead?

Error message:
go install: no install location for .go files listed on command line (GOBIN not set)

@cznic
Copy link
Contributor

cznic commented Jan 13, 2018

Arguments of go install are documented to be packages, not files. Try

$ go get github.com/nats-io/go-nats-streaming
$ go install github.com/nats-io/go-nats-streaming/examples
$ 

or

$ go build /go/src/github.com/nats-io/go-nats-streaming/examples/stan-pub.go

@sslavic
Copy link
Author

sslavic commented Jan 13, 2018

Thanks @cznic for your comment. What I understand from documentation and implementation is that the name of the argument is packages, but .go files being passed is documented as special case (it's not documented that it works only if GOBIN is set, and I would like that to be fixed not to require GOBIN). I.e. see https://golang.org/cmd/go/#hdr-Compile_and_install_packages_and_dependencies

$ 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.

Notice there go help packages reference.
See https://golang.org/cmd/go/#hdr-Description_of_package_lists

$ go help packages
Many commands apply to a set of packages:

	go action [packages]

...

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.

...

If one go install github.com/nats-io/go-nats-streaming/examples, it results in unwanted behaviour, just first main/app in directory with 3 apps gets installed to /go/bin, stan-bench, with examples as binary name. Yes, example apps are preferred by convention to be in dedicated packages/dirs, but they aren't - I've logged improvement ticket for that (see https://github.com/nats-io/go-nats-streaming/issues/164). Even without them being in dedicated packages/dirs, by the docs go file name should be enough, it should be used as package name and resulting binary name. Since the special case docs does not mention that it additionally requires GOBIN to be set, implementation should be fixed not to require it, or docs could be updated to mention GOBIN is required in this case - I prefer former.

Second suggestion, to go build /go/src/github.com/nats-io/go-nats-streaming/examples/stan-pub.go builds stan-pub binary into current working directory, which makes it a partial workaround to go install inconsistency issue, but that does not fix the inconsistency.

This ticket is intended to fix the inconsistency in go install, inconsistency affecting many e.g. see https://stackoverflow.com/q/25216765/381140.

@titanous titanous added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 16, 2018
@titanous titanous added this to the Go1.11 milestone Jan 16, 2018
@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Go1.12 Jul 2, 2018
@bcmills bcmills added GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 2, 2018
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Oct 2, 2018
@rsc
Copy link
Contributor

rsc commented Oct 24, 2018

The use of GOPATH/bin only applies to packages stored in GOPATH.
In general GOPATH is a list of tree roots, and a package in one root gets installed to the corresponding bin directory.

go install /other/file.go is not considered to be in any GOPATH, so it can only use GOBIN.

In the long term, with modules, we expect that people will stop setting GOPATH and then GOBIN may be more important (or maybe it will increase pressure to default to GOPATH[0]/bin).

@rsc
Copy link
Contributor

rsc commented Oct 24, 2018

FWIW I would put the examples in subdirectories and then you can go install blah/blah/examples/...

@rsc
Copy link
Contributor

rsc commented Oct 25, 2018

Note that if we do make GOBIN effectively default to GOPATH[0]/bin, then we could make 'go env GOBIN' show that default.

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Jan 18, 2019
@bcmills bcmills changed the title cmd/go: GOBIN inconsistently not required for installing package but required for .go file cmd/go: default GOBIN to GOPATH[0]/bin when outside of GOPATH Jan 18, 2019
@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 18, 2019
@bcmills
Copy link
Contributor

bcmills commented Jan 18, 2019

See also #29005.

@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 18, 2019
@bcmills bcmills changed the title cmd/go: default GOBIN to GOPATH[0]/bin when outside of GOPATH cmd/go: default GOBIN to GOPATH[0]/bin when outside of GOPATH? Jan 18, 2019
@newtorn

This comment has been minimized.

@andybons

This comment has been minimized.

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 25, 2019
@bcmills bcmills removed this from the Go1.14 milestone Sep 25, 2019
@bcmills bcmills added this to the Unplanned milestone Sep 25, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 25, 2019
@bcmills
Copy link
Contributor

bcmills commented Feb 5, 2020

@jayconrod, @matloob: I think we should address this in Go 1.15. The inability to rely on $(go env GOBIN) when setting PATH makes new-user setup more complex than it needs to be.

@bcmills bcmills modified the milestones: Unplanned, Go1.15 Feb 5, 2020
@matloob
Copy link
Contributor

matloob commented Feb 5, 2020

sounds good to me

@jayconrod jayconrod self-assigned this May 4, 2020
@gopherbot
Copy link

Change https://golang.org/cl/232017 mentions this issue: cmd/go: default GOBIN to GOPATH[0]/bin for packages outside GOPATH

@ianlancetaylor
Copy link
Contributor

The CL has been abandoned, so I'm guessing that this won't get done for 1.17. Should we change the milestone to 1.18 or Backlog?

@jayconrod jayconrod modified the milestones: Go1.17, Backlog May 4, 2021
@jayconrod jayconrod removed their assignment May 4, 2021
@jayconrod
Copy link
Contributor

Ah, should have changed that earlier. Moved to Backlog.

@mvdan
Copy link
Member

mvdan commented Aug 2, 2022

I still feel the pain of #23439 (comment) regularly when helping people relatively new to Go :)

What was the work that https://go.dev/cl/232017 was missing to be finished? Perhaps with that information, someone else with a bit of experience in cmd/go could pick up the patch.

@bcmills
Copy link
Contributor

bcmills commented Sep 6, 2022

From what I can tell, the remaining issues were described in this comment:
https://go-review.googlesource.com/c/go/+/232017/4#message-75b3b75fc59c83ddca9fba768d9a7ced203f4c29

@mvdan
Copy link
Member

mvdan commented Sep 7, 2022

This may be an infeasible approach, but... what about just teaching go env that it can show the most reasonable default for GOBIN when it's in module mode? Internally, none of the logic needs to change for now. It won't quite be right for case 3 that Jay mentioned (When cross-compiling, by default, we install to GOPATH/bin/goos_goarch), but we could always teach go env about it too.

I imagine swapping the default internally too might be easier when we drop GOPATH mode entirely, which hopefully isn't too far away at this point. Two out of the four remaining issues that Jay mentioned are in GOPATH mode.

@bcmills
Copy link
Contributor

bcmills commented Sep 9, 2022

I don't think a halfway approach in go env is worth the churn. (It's too easy to end up with a random UX regression, and if we're going to deal with the random regressions I'd rather we do a complete fix.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests