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: 'go list' should not resolve or record modules that are not relevant to the requested output fields #29869

Open
rogpeppe opened this issue Jan 22, 2019 · 6 comments
Labels
GoCommand cmd/go modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@rogpeppe
Copy link
Contributor

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

$ go version
go version devel +4b3f04c63b Thu Jan 10 18:15:48 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 env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rog/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/rog/src/go"
GOPROXY="http://localhost:3000"
GORACE=""
GOROOT="/home/rog/go"
GOTMPDIR=""
GOTOOLDIR="/home/rog/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/m/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-build480679307=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Here is a script that illustrates the issue (reproduce by saving contents to a file and running the testscript command on it (go get github.com/rogpeppe/go-internal/cmd/testscript)

# Create the module and tidy it.
go mod init m
go mod tidy
cmp go.mod go.mod-after-tidy

# Run a `go list` command to list the parent of the used package.
go list github.com/btcsuite/btcutil
cmp go.mod go.mod-after-list

# Running `go mod tidy` removes the new dependencies.
go mod tidy
cmp go.mod go.mod-after-tidy

-- main.go --
package main

import _ "github.com/btcsuite/btcutil/base58"

func main() {
}
-- go.mod-after-tidy --
module m

go 1.12

require github.com/btcsuite/btcutil v0.0.0-20180706230648-ab6388e0c60a
-- go.mod-after-list --
module m

go 1.12

require (
	github.com/btcsuite/btcd v0.0.0-20190115013929-ed77733ec07d // indirect
	github.com/btcsuite/btcutil v0.0.0-20180706230648-ab6388e0c60a
	golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 // indirect
)

I would not expect go list to add dependencies that aren't actually used by the code to go.mod.

@jayconrod
Copy link
Contributor

Related #29452. I think we need to make a decision on what side effects go list should have before changing.

I believe this is currently working as designed. cmd/go saves information to go.mod so that commands are reproducible (including go list). This is not what many people expect of go list though.

@jayconrod jayconrod added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 22, 2019
@jayconrod jayconrod added this to the Go1.13 milestone Jan 22, 2019
@mvdan
Copy link
Member

mvdan commented Jan 22, 2019

@jayconrod is it the same issue, though? Note that @rogpeppe had the module in question already as a requirement, since go mod tidy was run. In fact, re-running tidy is removing the extra indirect lines once again. I don't think this is simply a "list should have no side effects" issue.

@jayconrod
Copy link
Contributor

@mvdan I think it's the same issue. The main module depends on the package github.com/btcsuite/btcutil/base58, which has no dependencies outside the standard library. go list github.com/btcsuite/btcutil adds the transitive dependencies of that package. go mod tidy removes those dependencies, since nothing in the main module transitively depends on them.

go mod tidy effectively acts like a garbage collector: it removes requirements on modules which are unreachable from packages in the main module.

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2019

#29452 is related but distinct.

#26977 is probably the same underlying problem. Arguably both go mod why and go list (without -versions) should avoid resolving the requested package unless it is already in some module in the build list.

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2019

Hmm, this isn't quite the same as #26977 either: in this case, the package in question is already in an active module, but that module's dependencies have not been resolved.

go list resolves those dependencies as a side-effect of loading the package, but since you didn't actually request any information about the package's imports, we arguably don't need to load them. (See also #29666.)

@bcmills
Copy link
Contributor

bcmills commented Jan 22, 2019

On further consideration, I think this is more-or-less a duplicate of #29666.

In contrast, if you had asked for any substantial information about the dependencies of github.com/btcsuite/btcutil (for example, by examining the ImportMap field using -f '{{.ImportMap}}' or -json), then the go tool should arguably add the corresponding version information to the go.mod file to ensure that the output of the command is reproducible.

@bcmills bcmills changed the title cmd/go: go list can add dependencies cmd/go: 'go list' should not resolve or record modules that are not relevant to the requested output fields Jan 22, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants