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

go1.13b1 tool compile -p breaks main packages #33055

Closed
steeve opened this issue Jul 11, 2019 · 7 comments
Closed

go1.13b1 tool compile -p breaks main packages #33055

steeve opened this issue Jul 11, 2019 · 7 comments

Comments

@steeve
Copy link
Contributor

steeve commented Jul 11, 2019

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

$ go version
go version go1.13beta1 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/steeve/Library/Caches/go-build"
GOENV="/Users/steeve/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/steeve/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/steeve/sdk/go1.13beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/steeve/sdk/go1.13beta1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bs/51dlb_nn5k35xq9qfsxv9wc00000gr/T/go-build568993362=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

$ cat main.go
package main

func main() {
	println("1")
}
$ go1.13beta1 tool compile -p mytest.com/pkg -pack main.go
$ go1.13beta1 tool link main.a
runtime.main_main·f: function main is undeclared in the main package
$ go tool nm main.a
         U
     409 R gclocals·33cdeccccebe80329f1fdbee7f5874cb
     411 R gclocals·9fb7f0986f647f17cb53dda1484e0f7a
     3be ? go.cuinfo.packagename.mytest.com/pkg
     3c5 ? go.info.mytest.com/pkg.main
     3e6 ? go.isstmt.mytest.com/pkg.main
     3c5 ? go.loc.mytest.com/pkg.main
     3e6 ? go.range.mytest.com/pkg.main
     3c2 R go.string."1"
     3c3 R go.string."1\n"
         U gofile../Users/steeve/go/src/github.com/steeve/test/main.go
     3f1 ? mytest.com/pkg..inittask
     345 T mytest.com/pkg.main <-------------------------------------------------------
         U runtime.morestack_noctxt
         U runtime.printlock
         U runtime.printstring
         U runtime.printunlock

What did you expect to see?

Successful link.

What did you see instead?

Failure to link.

It seems the -p package is now honored on files which are package main, which wasn't the case on Go 1.12.

This breaks bazel's rules_go on 1.13b1: bazel-contrib/rules_go#2133

@steeve steeve changed the title go tool compile -p break main packages go1.13b1 tool compile -p break main packages Jul 11, 2019
@steeve steeve changed the title go1.13b1 tool compile -p break main packages go1.13b1 tool compile -p breaks main packages Jul 11, 2019
@cherrymui
Copy link
Member

I think -p flag should be honored. (Even in Go 1.12, it is partially honored, e.g. in debug info.) I'm not sure why one would pass -p something-not-main to a main package. cc @jayconrod about bazel.

@steeve
Copy link
Contributor Author

steeve commented Jul 11, 2019

Thinking about it, isn't -p redundant to the package directive ?
I'm not sure the purpose it serves, actually

@cherrymui
Copy link
Member

No. package keyword only specifies the name of the package, not the import path. For example, the compiler's cmd/compile/internal/ssa package has package declaration package ssa.

@jayconrod
Copy link
Contributor

I'm not sure what changed on the compiler / linker side, but from cmd/go, it's important that -p is honored for main packages. So I think the 1.13 behavior is correct. This needs to be fixed in rules_go.

There are a couple ways that multiple main packages can be linked into a binary. Broad go test -coverpkg arguments are especially bad about this. In those cases, we need to pass -p actual/import/path instead of -p main.

@cherrymui
Copy link
Member

In Go 1.13 the compiler honors the -p flag more that before. It uses the -p package path fo symbol names. Before, it is only used in some non-critical places.

Ok, -coverpkg is a use case. But it is different in that the "main" packages there are not actually the main package in the executable. And the compiler and the linker are consistent there. So that is fine. Here the compiler sees a non-main package (from -p) whereas it is expected to be linked as the main package. This doesn't work (and should not).

I guess we can close this as it is not a bug on the Go side?

@jayconrod
Copy link
Contributor

Yeah, I think we can close this. The compiler and linker shouldn't need to do anything differently. The build system (Bazel in this case) should be responsible for setting -p correctly, so I'll fix bazel-contrib/rules_go#2133 soon.

@steeve
Copy link
Contributor Author

steeve commented Jul 11, 2019

Thanks folks, that was quick !

@golang golang locked and limited conversation to collaborators Jul 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants