golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.22k stars 17.37k forks source link

cmd/go: runtime/debug build information not populated for test binaries when package name is not 'main' #33976

Open bcmills opened 4 years ago

bcmills commented 4 years ago

What did you do?

https://play.golang.org/p/0ITNcl4kXGN

$ gotip version
go version devel +8c5de667 Fri Aug 30 08:28:40 2019 +0000 linux/amd64

foo$ gotip mod init foo
go: creating new go.mod: module foo

foo$ cat > foo_test.go
package foo_test

import (
        "runtime/debug"
        "testing"
)

func TestBuildInfo(t *testing.T) {
        info, ok := debug.ReadBuildInfo()
        if !ok {
                t.Fatal("no debug info")
        }
        t.Log(info.Main.Version)
}

foo$ go test

What did you expect to see?

=== RUN   TestBuildInfo
--- PASS: TestBuildInfo (0.00s)
    foo_test.go:13: (devel)
PASS

(https://play.golang.org/p/0ITNcl4kXGN)

What did you see instead?

--- FAIL: TestBuildInfo (0.00s)
    foo_test.go:11: no debug info
FAIL

CC @jayconrod @rsc; see also #33975.

breml commented 4 years ago

I ended up on this issue while investigating how to access (non Go) files located in a sub-directory of my Go module. The access should happen from tests located in sub-sub-package. We have it working with a relative path (e.g. ../../../folder/file) but it would be beneficial to have a less brittle way of accessing these files (especially for the case of refactoring).

So I tried the same as @bcmills is describing in this issue and I wanted to access info.Main.Path with the end goal of being able to access my files with info.Main.Path + "/folder/file".

Questions:

  1. Is my assumption correct, that info.Main.Path would contain the basepath for the module, if the buildinfo would be available in test builds?
  2. Why is the the buildinfo currently not available in test builds?
breml commented 4 years ago

Just as a note, using go list does work to get details about Go modules. E.g.:

func moduleBasePath(t *testing.T) string {
    t.Helper()

    err := os.Setenv("GO111MODULE", "on")
    if err != nil {
        t.Fatalf("unable to set GO111MODULE env var: %v", err)
    }

    cmd := exec.Command("go", "list", "-f", "{{.Module.Dir}}")
    out, err := cmd.Output()
    if err != nil {
        t.Fatalf("failed to evaluate Go module base path: %v", err)
    }

    return strings.TrimSpace(string(out))
}
bcmills commented 4 years ago
  1. Is my assumption correct, that info.Main.Path would contain the basepath for the module, if the buildinfo would be available in test builds?

info.Main.Path in general contains a module path, not a filesystem path.

  1. Why is the the buildinfo currently not available in test builds?

That is the bug described in this issue.

bcmills commented 4 years ago

Just as a note, using go list does work to get details about Go modules. E.g.:

Note that running go list in the working directory of a test will tell you about the module that contains the test itself, not the main module in which the test was built.

breml commented 4 years ago
  1. Is my assumption correct, that info.Main.Path would contain the basepath for the module, if the buildinfo would be available in test builds?

info.Main.Path in general contains a module path, not a filesystem path.

OK, understood. So this would not have helped me anyway.

  1. Why is the the buildinfo currently not available in test builds?

That is the bug described in this issue.

I was under the impression, that the buildinfo has been left out intentionally and not that it is a bug. Good to hear, that it is considered a bug.

breml commented 4 years ago

Just as a note, using go list does work to get details about Go modules. E.g.:

Note that running go list in the working directory of a test will tell you about the module that contains the test itself, not the main module in which the test was built.

That is not clear to me. Is this not the same in the case of tests? My understanding is, that go test builds for every package a test binary and executes it. The guarantee is, that the current working directory is actually the directory that contains the test. So therefore, the module that contains the test and the module the test was built for is the same (or is there something special going on in regards to modules when building tests?). This is why go test -o test ./... does not work (error message: cannot use -o flag with multiple packages).

bcmills commented 4 years ago

The guarantee is, that the current working directory is actually the directory that contains the test.

Yes.

So therefore, the module that contains the test and the module the test was built for is the same (or is there something special going on in regards to modules when building tests?).

The module that contains the test is the module containing the working directory, yes.

However, that directory does not indicate the version of that module, and the versions reported by go list -m all will not be accurate: those versions are determined by the “main module” (in the sense of https://tip.golang.org/cmd/go/#hdr-The_main_module_and_the_build_list), not “the module containing package main”. (See #33975.)

breml commented 4 years ago

However, that directory does not indicate the version of that module, and the versions reported by go list -m all will not be accurate: those versions are determined by the “main module” (in the sense of https://tip.golang.org/cmd/go/#hdr-The_main_module_and_the_build_list), not “the module containing package main”. (See #33975.)

Thanks for your detailed explanation. This comment made it clear to me with the provided example. For my use case this does not matter, because the "main module" and "the module containing package main" are the same.

ChrisHines commented 3 years ago

I am exploring adding some logic and maybe a feature or two to github.com/go-stack/stack that take advantage of build information, but was surprised to discover that it didn't work with go test. I am glad to see it has already been reported, but sad that it hasn't been fixed yet.

I can probably get my ideas to work with a different testing strategy, but the new features would have to come with a big caveat about not working in tests since github.com/go-stack/stack is often used by logging code and that in turn is often relied upon to help diagnose test failures.

Has anyone investigated the level of effort to fix this? I am willing to help given some suggestions about where to start digging into the issue.

ChrisHines commented 3 years ago

I also have found that when the package is named main the BuildInfo.Path and .Main fields are populated but BuildInfo.Deps field is nil even when the package under test depends on code from other modules.

mark-rushakoff commented 2 years ago

I am working on a project that delivers a binary built with go test -c, and I was expecting to be able to embed and report the project's current commit with vcs.revision from debug.ReadBuildInfo().Settings.

I guess we can fall back to the old ldflags approach for my use case.

dolanor commented 2 years ago

This bug hit me as well as we use module info to start some server based on the availability on the system. My alternative is to have a binary built externally, and run it with os.Exec, but it is not quite as clean as running the main func directly from the code. Is there any idea on how this could be fixed?

bcmills commented 2 years ago

Is there any idea on how this could be fixed?

Probably the condition here needs to be updated: https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/load/pkg.go;l=1965;drc=a6e5be0d30770e8bd1ba1e4bac2089218df121d9

I'm not sure exactly what to update it to, though.

prochac commented 1 year ago

Any updates? It doesn't seem like a technical issue, it's just being blocked by the if condition, right?

deregtd commented 1 year ago

+1 on this. Having to fall back to some nasty parsing off the function location off the CallersFrame return in this case in cases with broken buildinfo like this, so our unit tests are ugly and we can't actually unit test the behavior we're looking for. :(

muir commented 1 year ago

+1 on this. I don't have a workaround.

mvdan commented 1 year ago

Please remember https://github.com/golang/go/wiki/NoPlusOne. I'm hiding the last responses to prevent the issue from growing in size, which doesn't help.

If you want to help, feel free to try a fix following Bryan's advice in https://github.com/golang/go/issues/33976#issuecomment-1179279071, though note that it requires a bit of thought and none of the existing tests should break.

matloob commented 1 week ago

We set the buildinfo for the testmain package using the buildinfo of the package under test: https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/load/test.go;l=284;drc=a85b8810c49515c469d265c399febfa48442a983. It seems to me like we can just call setBuildInfo on pmain (the testmain package) if the package under test does not have buildinfo set. That would be similar to what's being done in https://go.dev/cl/595961

matloob commented 1 week ago

cc @samthanawalla