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: executables are sometimes named after their module's last element, not directory #27283

Closed
spenczar opened this issue Aug 27, 2018 · 17 comments

Comments

Projects
None yet
10 participants
@spenczar
Copy link
Contributor

commented Aug 27, 2018

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

go1.11

Does this issue reproduce with the latest release?

yes

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

darwin amd64

What did you do?

In a package main's directory (specifically, github.com/twitchtv/twirp/protoc-gen-twirp), I had a go.mod like this:

module github.com/twitchtv/twirp/protoc-gen-twirp/v5

I then ran GO111MODULE=on go build . from within $GOPATH/src/github.com/twitchtv/twirp/protoc-gen-twirp.

I expected an executable named protoc-gen-twirp, because that is the last element of the directory. This is the documented behavior in https://tip.golang.org/cmd/go/#hdr-Compile_packages_and_dependencies:

When compiling a single main package, build writes the resulting executable to an output file named after the first source file ('go build ed.go rx.go' writes 'ed' or 'ed.exe') or the source code directory ('go build unix/sam' writes 'sam' or 'sam.exe').

Instead, an executable was produced named v5.

However, if I invoke go build with a full path, I get the expected name. GO111MODULE=on go build github.com/twitchtv/twirp/protoc-gen-twirp produces an executable named protoc-gen-twirp.

This is either a documentation error, or a bug.

@mvdan

This comment has been minimized.

Copy link
Member

commented Aug 27, 2018

@FiloSottile FiloSottile added this to the Go1.12 milestone Aug 30, 2018

@myitcv

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

Confirmed.

export GOPATH=$(mktemp -d)
cd $(mktemp -d)
mkdir hello
cd hello
go mod init example.com/hello/v6
cat <<EOD > main.go

package main

func main() {}
EOD
go build .

Now:

$ ls
go.mod  main.go  v6

But:

$ go install
$ ls $GOPATH/bin
hello

go install was fixed in #24667; sounds like a similar fix needed for go build.

@gopherbot

This comment has been minimized.

Copy link

commented Oct 10, 2018

Change https://golang.org/cl/140863 mentions this issue: cmd/go: fix the default build output name for versioned binaries

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Jan 2, 2019

If this does not land before RC1 (in the next couple of days), open a backport issue if it should be cherry-picked into 1.12.

/cc @hyangah

@gopherbot

This comment has been minimized.

Copy link

commented Jan 14, 2019

Change https://golang.org/cl/157798 mentions this issue: cmd/go: fix the default build output name for versioned binaries

@robfordww

This comment has been minimized.

Copy link

commented Jan 18, 2019

Modules with name 'example.com/mymodule.git' ,where the .git extension is used to trigger the local VCS command, get a binary named mymodule.git.exe. Shouldn't '.git' be stripped?

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Jan 24, 2019

@chappjc

This comment has been minimized.

Copy link

commented Feb 15, 2019

If this does not land before RC1 (in the next couple of days), open a backport issue if it should be cherry-picked into 1.12.

/cc @hyangah

Looks like it didn't make 1.12. I don't know how a backport issue is started, but if this could be cherry-picked into 1.12 I'd be very happy.

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@gopherbot please backport to 1.12

@gopherbot

This comment has been minimized.

Copy link

commented Feb 15, 2019

Backport issue(s) opened: #30266 (for 1.12).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

Are we trying to get https://golang.org/cl/140863 into 1.12? @hyangah @bcmills

@mvdan

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@ianlancetaylor from the "backport" wording earlier, I presume 1.12.1 was meant.

@bcmills

This comment has been minimized.

Copy link
Member

commented Feb 15, 2019

@mvdan is correct: we're targeting 1.12.1.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 13, 2019

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

@gopherbot gopherbot closed this in bf94fc3 Mar 13, 2019

gopherbot pushed a commit that referenced this issue Mar 13, 2019

[release-branch.go1.12] cmd/go: fix the default build output name for…
… 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>
@gopherbot

This comment has been minimized.

Copy link

commented Mar 14, 2019

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

gopherbot pushed a commit that referenced this issue Mar 14, 2019

Revert "[release-branch.go1.12] cmd/go: fix the default build output …
…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>

@dmitshur dmitshur self-assigned this Mar 19, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Mar 21, 2019

Change https://golang.org/cl/167503 mentions this issue: cmd/go: fix the default build output name for versioned binaries

@dmitshur

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Reopening because the original CL 140863 was reverted on master branch via CL 168402.

@dmitshur dmitshur reopened this Mar 22, 2019

@gopherbot gopherbot closed this in 94563de Mar 22, 2019

@gopherbot

This comment has been minimized.

Copy link

commented Mar 22, 2019

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 27, 2019

[release-branch.go1.12] cmd/go: fix the default build output name for…
… 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.