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: 'go install mod/mainpkg' installs a binary with a version suffix #26869

Closed
bcmills opened this issue Aug 8, 2018 · 6 comments
Closed
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 8, 2018

As demonstrated in https://golang.org/cl/128137, the current behavior of go get and go install for main binaries appends a complete version suffix to the name of the binary.

This is somewhat related to #24667, but more severe. I'm not sure when the bug was introduced.

CC: @rsc @myitcv

env GO111MODULE=on

go mod init example.com/m

# get of a binary should install it to $GOPATH/bin
# BUG: vgo-tour should be installed as vgo-tour, not vgo-tour@v1.0.0.
go get research.swtch.com/vgo-tour
exec $GOPATH/bin/vgo-tour@v1.0.0
stdout 'Hello, world.'
rm $GOPATH/bin/vgo-tour@v1.0.0

# install of a binary should install it to $GOPATH/bin
# BUG: vgo-tour should be installed as vgo-tour, not vgo-tour@v1.0.0.
go install research.swtch.com/vgo-tour
exec $GOPATH/bin/vgo-tour@v1.0.0
stdout 'Hello, world.'
@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. modules labels Aug 8, 2018
@bcmills bcmills added this to the Go1.11 milestone Aug 8, 2018
@rsc
Copy link
Contributor

rsc commented Aug 9, 2018

This is because the directory where that command lives is called vgo-tour@v1.0.0 in the module cache. (Still a bug but that explains the bug.)

@myitcv
Copy link
Member

myitcv commented Aug 9, 2018

This is also further complicated if you have a replace directive to a local directory, because then the directory name of the replacement is used. It took me ages to work out why I had gopherjs and gopherjs@vx.y.z in my GOBIN.

@gopherbot
Copy link

Change https://golang.org/cl/128900 mentions this issue: cmd/go: fix install target name for versioned binaries

@gopherbot
Copy link

Change https://golang.org/cl/167503 mentions this issue: cmd/go: use path.Split on import path in DefaultExecName

@gopherbot
Copy link

Change https://golang.org/cl/168678 mentions this issue: cmd/go/internal/load: always use DefaultExecName to determine binary name

gopherbot pushed a commit that referenced this issue Mar 22, 2019
This change is a re-apply of the reverted CL 140863 with changes to
address issue #30821. Specifically, path.Split continues to be used
to split the '/'-separated import path, rather than filepath.Split.

Document the algorithm for how the default executable name is determined
in DefaultExecName.

Rename a variable returned from os.Stat from bs to fi for consistency.

CL 140863 factored out the logic to determine the default executable
name from the Package.load method into a DefaultExecName function,
and started using it in more places to avoid having to re-implement
the logic everywhere it's needed. Most previous callers already computed
the default executable name based on the import path. The load.Package
method, before CL 140863, was the exception, in that it used the p.Dir
value in GOPATH mode instead. There was a NOTE(rsc) comment that it
should be equivalent to use import path, but it was too late in Go 1.11
cycle to risk implementing that change.

This is part 1, a more conservative change for backporting to Go 1.12.2,
and it keeps the original behavior of splitting on p.Dir in GOPATH mode.
Part 2 will address the NOTE(rsc) comment and modify behavior in
Package.load to always use DefaultExecName which splits the import path
rather than directory. It is intended to be included in Go 1.13.

Fixes #27283 (again)
Updates #26869
Fixes #30821

Change-Id: Ib1ebb95acba7c85c24e3a55c40cdf48405af34f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/167503
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
@gopherbot
Copy link

Change https://golang.org/cl/168958 mentions this issue: [release-branch.go1.12] cmd/go: fix the default build output name for versioned binaries

gopherbot pushed a commit that referenced this issue Mar 26, 2019
…name

It should produce equivalent results to split on the import path of the
package rather than its directory, in GOPATH mode. That means the common
code in DefaultExecName can be used.

We're in the middle of Go 1.12 cycle, so now is a good time to make it
happen for Go 1.13.

Modify isVersionElement to accept path elements like "v2", "v3", "v10",
rather than "/v2", "/v3", "/v10". Then use it in DefaultExecName instead
of the ad-hoc isVersion function. There is no change in behavior.

Add tests for DefaultExecName and isVersionElement.

Updates #26869

Change-Id: Ic6da2c92587459aa2b327385e994b72a6e183092
Reviewed-on: https://go-review.googlesource.com/c/go/+/168678
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue Mar 27, 2019
… versioned binaries

This change is a re-apply of the reverted CL 140863 with changes to
address issue #30821. Specifically, path.Split continues to be used
to split the '/'-separated import path, rather than filepath.Split.

Document the algorithm for how the default executable name is determined
in DefaultExecName.

Rename a variable returned from os.Stat from bs to fi for consistency.

CL 140863 factored out the logic to determine the default executable
name from the Package.load method into a DefaultExecName function,
and started using it in more places to avoid having to re-implement
the logic everywhere it's needed. Most previous callers already computed
the default executable name based on the import path. The load.Package
method, before CL 140863, was the exception, in that it used the p.Dir
value in GOPATH mode instead. There was a NOTE(rsc) comment that it
should be equivalent to use import path, but it was too late in Go 1.11
cycle to risk implementing that change.

This is part 1, a more conservative change for backporting to Go 1.12.2,
and it keeps the original behavior of splitting on p.Dir in GOPATH mode.
Part 2 will address the NOTE(rsc) comment and modify behavior in
Package.load to always use DefaultExecName which splits the import path
rather than directory. It is intended to be included in Go 1.13.

Updates #27283
Updates #26869
Updates #30821
Fixes #30266

Change-Id: Ib1ebb95acba7c85c24e3a55c40cdf48405af34f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/167503
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
(cherry picked from commit 94563de)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168958
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants