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

runtime/debug: BuildInfo lacks information about “the main module” #33975

Closed
bcmills opened this issue Aug 30, 2019 · 19 comments
Closed

runtime/debug: BuildInfo lacks information about “the main module” #33975

bcmills opened this issue Aug 30, 2019 · 19 comments
Labels
Documentation FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 30, 2019

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

~/go/src$ go version
go version devel +d56a86e01f Thu Aug 29 12:51:31 2019 +0000 linux/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/src$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/bcmills/.cache/go-build"
GOENV="/usr/local/google/home/bcmills/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/usr/local/google/home/bcmills"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/bcmills/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/bcmills/go/pkg/tool/linux_amd64"
GCCGO="/usr/local/google/home/bcmills/bin/gccgo"
AR="ar"
CC="gcc"
CXX="c++"
CGO_ENABLED="1"
GOMOD="/usr/local/google/home/bcmills/go/src/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build430797677=/tmp/go-build -gno-record-gcc-switches"

What did you do?

From within module y, build a binary from module x than examines the Main field of runtime/debug.BuildInfo:
https://go-review.googlesource.com/c/go/+/192558/1/src/cmd/go/testdata/script/mod_modinfo.txt

What did you expect to see?

Since the field's documentation says it is (emphasis mine) “The main module information”, I expect it to contain information about “the main module” in the sense of https://tip.golang.org/cmd/go/#hdr-The_main_module_and_the_build_list.

For this example, I expected the information reported by the binary to look like

mod y (devel)
dep x v0.0.0-00010101000000-000000000000
dep golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
dep rsc.io/quote v1.5.2 h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=
dep rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=

What did you see instead?

Instead, the Main field contains information about the module containing package main:

mod x v0.0.0-00010101000000-000000000000
dep golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
dep rsc.io/quote v1.5.2 h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=
dep rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=

with no information about the main module (y) at all, not even as a dep.

As far as I can tell, it has been that way since module support was first implemented.


CC @jayconrod @rsc @thepudds @hyangah

@bcmills bcmills added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 30, 2019
@bcmills bcmills added this to the Go1.14 milestone Aug 30, 2019
@bcmills bcmills self-assigned this Aug 30, 2019
@bcmills
Copy link
Contributor Author

bcmills commented Aug 30, 2019

Given the current behavior of the BuildInfo.Main field, I think we should probably leave its content as-is, and update its documentation to clarify the meaning.

However, we should also add the actual “main module”, probably as a new field in the BuildInfo struct.

@hyangah
Copy link
Contributor

hyangah commented Sep 3, 2019

Leaning towards documentation change. "mod" line should be the version info of the module that contained the main package.

I don't know what value mod y (devel) provides users. Especially, if a user builds and installs the binary from an empty or temporary directory (for clean build), there will be no version or remote vcs associated with it and the main module name will be an arbitrary temporary directory name. Maybe whether the workspace was a clean build or not may be useful.

@bcmills
Copy link
Contributor Author

bcmills commented Sep 3, 2019

I don't know what value mod y (devel) provides users.

Mostly it would be a clear indication that the user was not working within any version of the x module when they built the binary. That information would be useful to a developer of x, because it would give them a clue as to how the user got into the final configuration in the first place.

(For example, it might give them someplace to start sending PRs to get their users out of a known-buggy configuration.)

@mholt
Copy link

mholt commented Sep 3, 2019

Given the current behavior of the BuildInfo.Main field, I think we should probably leave its content as-is

If possible at this stage of modules, I would like to see the behavior improved/corrected, since there's already enough "gotchas" related to dealing with modules, it would be great to reduce those as much as possible.

@hyangah
Copy link
Contributor

hyangah commented Sep 3, 2019

@bcmills The 'y' is arbitrary (temporary module name), and '(devel)' is arbitrary in the example. If we add any other field or repurpose the Main field and they should be meaningful, I think this should include more info than those short string.

@bcmills
Copy link
Contributor Author

bcmills commented Sep 3, 2019

The y in the example is arbitrary, but in general the name of the main module is not: for example, if the binary is a dependency defined through a +build tools source file, the main module may be a long-lived, maintained module and its version may even be significant.

@thepudds
Copy link
Contributor

thepudds commented Sep 3, 2019

Leaning towards documentation change.

I think I am not fully following the alternatives, but is it still under consideration to use VCS information to properly populate the version for the "main module” (not the package containing main) in at least some circumstances?

For example, from the currently open #29814:

When a binary is build from within a module's source tree, the output from runtime/debug.ReadBuildInfo currently reports that module as having version (devel).

If the source tree is a pristine checkout from a version-control system — or is within the (read-only) module cache — we could instead interrogate the version-control system to find the corresponding version or pseudo-version to embed.

@bcmills
Copy link
Contributor Author

bcmills commented Sep 3, 2019

is it still under consideration to use VCS information to properly populate the version

Yes, but that is (mostly) orthogonal to this issue.

This issue is: “should we include the main module at all if it doesn't contribute any packages to the build, and if so, where?”

#29814 is, “if the main module appears in BuildInfo, which version should we report for it?”

@rsc
Copy link
Contributor

rsc commented Sep 17, 2019

BuildInfo.Main is the module with the main package, not the main module (where those differ).
The docs should reflect that.

@gopherbot
Copy link

Change https://golang.org/cl/196522 mentions this issue: runtime/debug: correct BuildInfo.Main documentation

@icholy
Copy link

icholy commented Sep 19, 2019

I thought the module with the main package was the main module.

@dmitshur
Copy link
Contributor

@icholy The term "main module" is described in the first sentence at https://golang.org/cmd/go/#hdr-The_main_module_and_the_build_list.

@icholy
Copy link

icholy commented Sep 20, 2019

@dmitris sorry for being dense, but how would it possible for the main module to be different from the module with the main package?

@jayconrod
Copy link
Contributor

@icholy Consider the commands below:

cd $(mktemp -d)
go mod init example.com/mod
go get golang.org/x/tools/cmd/goimports

The main module is example.com/mod. It's determined by the go.mod file in the current directory.

The main package is golang.org/x/tools/cmd/goimports, the subject of the go get command. It's in the module golang.org/x/tools. There may be many main packages in different modules.

gopherbot pushed a commit that referenced this issue Sep 20, 2019
The term "main module" has a special meaning [1]
and is not what we intended to refer to with BuildInfo.Main.

[1] https://golang.org/cmd/go/#hdr-The_main_module_and_the_build_list

Updates #33975

Change-Id: Ieaba5fcacee2e87c5c15fa7425527bbd64ada5d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/196522
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@FiloSottile
Copy link
Contributor

This issue is: “should we include the main module at all if it doesn't contribute any packages to the build, and if so, where?”

I think we shouldn't and that the current behavior is good as is. From a reproducibility point of view, the module of the main package and its dependencies should be everything that define the build, and given the same versions (and build flags, and GOOS/GOARCH, etc), the binary should build the same regardless of what module it's built from.

@mback2k
Copy link

mback2k commented Mar 22, 2020

Wouldn't this issue then just boil down to be a duplicate of #29814 and therefore #37475?

@FiloSottile
Copy link
Contributor

Wouldn't this issue then just boil down to be a duplicate of #29814 and therefore #37475?

This issue is about including the main module if it doesn't contribute any packages to the build. If we decide we shouldn't, then this issue would be closed, yes.

@bcmills
Copy link
Contributor Author

bcmills commented Mar 25, 2020

@FiloSottile, I think the main use-case for embedding info about “the main module” is specifically for controlling test behavior.

If a given test is in the main module (or in another module with the same maintainers) then it seems more reasonable for that test to assume or require properties of the development environment above and beyond the basic go test environment. For example, if a system-local tool such as git (or the go command itself) is missing, the test might Fail when it is in the main module, but to instead Skip when it is run as a dependency of some other module.

@bcmills
Copy link
Contributor Author

bcmills commented Oct 26, 2021

Given #37475 this might not actually be necessary — the VCS stamp will be present if the binary is in the main module, and will not be present otherwise.

@bcmills bcmills closed this as completed Oct 26, 2021
bors bot added a commit to meilisearch/meilisearch-go that referenced this issue Apr 5, 2022
279: Feature/Analytics r=alallema a=brunoocasali

- Add `User-Agent` inside the pre-defined headers

Add Go support as requested here meilisearch/integration-guides#150

I tried other options, but they do not fit so well:

- https://pkg.go.dev/debug/buildinfo (does not work very well :/ has a lot of issues golang/go#29814, golang/go#33975
- https://github.com/ahmetb/govvv (I think it is not an option since it seems to be not maintained anymore).


Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: Amélie <alallema@users.noreply.github.com>
bors bot added a commit to meilisearch/meilisearch-go that referenced this issue Apr 5, 2022
279: Feature/Analytics r=alallema a=brunoocasali

- Add `User-Agent` inside the pre-defined headers

Add Go support as requested here meilisearch/integration-guides#150

I tried other options, but they do not fit so well:

- https://pkg.go.dev/debug/buildinfo (does not work very well :/ has a lot of issues golang/go#29814, golang/go#33975
- https://github.com/ahmetb/govvv (I think it is not an option since it seems to be not maintained anymore).


Co-authored-by: Bruno Casali <brunoocasali@gmail.com>
Co-authored-by: Amélie <alallema@users.noreply.github.com>
@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests