-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: executables are sometimes named after their module's last element, not directory [1.12 backport] #30266
Comments
@mvdan - there isn't a reason provided in the gopherbot message. Would you mind providing one for this backport? |
See Filippo's comment in the original thread. Is there a convention to add the reason for the backport in the backport issue directly? |
Change https://golang.org/cl/167384 mentions this issue: |
Closed by merging 746edd4 to release-branch.go1.12. |
… versioned binaries `go build` has chosen the last element of the package import path as the default output name when -o option is given. That caused the output of a package build when the module root is the major version component such as 'v2'. A similar issue involving `go install` was fixed in https://golang.org/cl/128900. This CL refactors the logic added with the change and makes it available as internal/load.DefaultExecName. This CL makes 'go test' to choose the right default test binary name when the tested package is in the module root. (E.g., instead of v2.test, choose pkg.test for the test of 'path/pkg/v2') Fixes #27283 Fixes #30266 Change-Id: I6905754f0906db46e3ce069552715f45356913ae Reviewed-on: https://go-review.googlesource.com/c/go/+/140863 Reviewed-by: Bryan C. Mills <bcmills@google.com> (cherry picked from commit bf94fc3) Reviewed-on: https://go-review.googlesource.com/c/go/+/167384 Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
@mvdan Yes. I'll quote from https://golang.org/wiki/MinorReleases (emphasis mine):
When asking gopherbot to backport an issue, the reason should be included in that message. That |
Change https://golang.org/cl/167617 mentions this issue: |
…name for versioned binaries" This reverts commit 746edd4 (CL 167384). Reason for revert: Dmitri identified a potential problem in https://go-review.googlesource.com/c/go/+/140863/11#message-db0ff6bb2c7b06161ca47de771c4465afa8b1102, and we'd like more time to investigate without holding up the 1.12 release branch. Updates #27283 Updates #30266 Updates #30821 Change-Id: I49d7bbbe200e80b81899c3bcbf7844717af010aa Reviewed-on: https://go-review.googlesource.com/c/go/+/167617 Reviewed-by: Andrew Bonventre <andybons@golang.org>
Closed by merging 32355f5 to release-branch.go1.12. |
@bcmills why was this re-opened? It looks like the cherry pick to the 1.12 branch was already merged. I'm going to re-label and close it now, but re-open if there is something else still pending. |
Ah I see it was reverted at https://go-review.googlesource.com/c/go/+/167617 |
Change https://golang.org/cl/168958 mentions this issue: |
Approving this because it's a regression from GOPATH mode. There was an attempt to get it into 1.12.1 that didn't work out, so we're trying again for 1.12.2. |
… 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>
Closed by merging aeb9d03 to release-branch.go1.12. (I'm closing instead of @gopherbot because it doesn't handle this edge case right now, see issue #31094.) |
@mvdan requested issue #27283 to be considered for backport to the next 1.12 minor release.
The text was updated successfully, but these errors were encountered: