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 mod why' should have an answer for every module in 'go list -m all' #27900

Open
bcmills opened this issue Sep 27, 2018 · 16 comments
Open
Labels
FeatureRequest modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 27, 2018

Currently, if you run go mod why -m on the output of every module in go list -m all, some modules may come back with the answer (main module does not need module […]), even after a go mod tidy.

The reason is that the module graph is conservative: go list -m all answers the question “which module versions are implied by the requirements of the main module?”, not “which modules does the main module need to download to complete go list all?”.¹

In contrast, go mod why explicitly ties its answer to go list all.

We should have some flag to go mod why to answer the related go list -m all question, “what path of requirements imposes the version requirement on module x?”


¹The command to answer the latter question, as it turns out, is:

go list -f '{{with .Module}}{{.Path}} {{.Version}}{{end}}' all | sort -u
@myitcv
Copy link
Member

myitcv commented Nov 12, 2018

In contrast, go mod why explicitly ties its answer to go list all.

I don't think this is true, because go list all is subject to build constraints. The statement would be true if we were to say the union of go list all for all possible build constraints, or the hypothetical go list -nobuild all which would enable go list to list package irrespective of build constraints.

Consider github.com/myitcvscratch/depanalysis/cmd/analyzer. The main package whose import path coincides with the module path effectively sits behind a js,wasm build constraint. As does one of its dependencies.

This is therefore unsurprising:

$ go list
can't load package: package github.com/myitcvscratch/depanalysis/cmd/analyzer: build constraints exclude all Go files in /tmp/depanalysis/cmd/analyzer

If we provide the constraints we can list the main package:

$ GOOS=js GOARCH=wasm go list
github.com/myitcvscratch/depanalysis/cmd/analyzer

Similarly, unless we satisfy the constraints, go list all won't work either:

$ go list -f "{{if not .Standard}}{{.ImportPath}}{{end}}" all | sort -u
go: warning: "all" matched no packages

Again, fixed:

$ GOOS=js GOARCH=wasm go list -f "{{if not .Standard}}{{.ImportPath}}{{end}}" all | sort -u
github.com/myitcvscratch/depanalysis/cmd/analyzer
github.com/myitcvscratch/depanalysis/jsdep
github.com/myitcvscratch/depanalysis/normdep

go list -m is not subject to build constraints; it's operating at the module level, and therefore outside the build:

$ go list -m all
github.com/myitcvscratch/depanalysis/cmd/analyzer
github.com/myitcvscratch/depanalysis v0.0.1
github.com/myitcvscratch/depanalysis/distant v0.0.1

Hence it is not equivalent to a go list -f where we output the module path:

$ go list -f "{{if not .Standard}}{{.Module.Path}}{{end}}" all | sort -u
go: warning: "all" matched no packages

Because when listing packages we need to consider build constraints:

$ GOOS=js GOARCH=wasm go list -f "{{if not .Standard}}{{.Module.Path}}{{end}}" all | sort -u
github.com/myitcvscratch/depanalysis
github.com/myitcvscratch/depanalysis/cmd/analyzer

go list -m gives the nodes of the requirement graph output by go mod graph:

$ go mod graph
github.com/myitcvscratch/depanalysis/cmd/analyzer github.com/myitcvscratch/depanalysis@v0.0.1
github.com/myitcvscratch/depanalysis@v0.0.1 github.com/myitcvscratch/depanalysis/distant@v0.0.1

The docs for go mod why say:

By default, why queries the graph of packages matched by "go list all",
which includes tests for reachable packages.

But that does not appear to be the case. go mod why instead seems to correspond to the hypothetical go list -nobuild all mentioned above:

$ go mod why github.com/myitcvscratch/depanalysis/jsdep
# github.com/myitcvscratch/depanalysis/jsdep
github.com/myitcvscratch/depanalysis/cmd/analyzer
github.com/myitcvscratch/depanalysis/jsdep

$ go mod why github.com/myitcvscratch/depanalysis/normdep
# github.com/myitcvscratch/depanalysis/normdep
github.com/myitcvscratch/depanalysis/cmd/analyzer
github.com/myitcvscratch/depanalysis/normdep

$ go mod why github.com/myitcvscratch/depanalysis/unused
# github.com/myitcvscratch/depanalysis/unused
(main module does not need package github.com/myitcvscratch/depanalysis/unused)

$ go mod why image/jpeg
# image/jpeg
github.com/myitcvscratch/depanalysis/cmd/analyzer
fmt
reflect
reflect.test
encoding/json
encoding/json.test
image
image.test
image/jpeg

$ go mod why -m github.com/myitcvscratch/depanalysis
# github.com/myitcvscratch/depanalysis
github.com/myitcvscratch/depanalysis/cmd/analyzer
github.com/myitcvscratch/depanalysis/jsdep

$ go mod why -m github.com/myitcvscratch/depanalysis/distant
# github.com/myitcvscratch/depanalysis/distant
(main module does not need module github.com/myitcvscratch/depanalysis/distant)

As you say, we need the ability to answer the question "why does github.com/myitcvscratch/depanalysis/distant appear in our module requirement graph?"

I'm less clear we need something like a -nobuild flag for go list. Instead, on the (rather dangerous) assumption my analysis correct, I think the docs for go mod why need a slight tweak.

@mwf
Copy link

mwf commented Feb 13, 2019

Please, keep in mind the case with test dependencies - would be cool if go mod why could tell if the dependency is used transitively in tests - #30206

@aofei
Copy link
Contributor

aofei commented Jan 7, 2020

And I think the note (main module does not need module ...) is flawed. If a module isn't needed by our main module, then it shouldn't cause our main module to be broken (see #36423 for example).

@OJFord
Copy link

OJFord commented May 27, 2022

I assume this is the same issue - I was trying to update to fix a dependabot issue; but I was still left with the unpatched major version I was hoping to remove. go mod why on it said it's not needed, so I thought ok it's just for some reason left in go.sum; but if I manually remove it then go get -u puts it back.

Is there any (not intensively manual) workaround to find which main package dependency leads to it if why won't tell me?

@mvdan
Copy link
Member

mvdan commented May 27, 2022

@OJFord you want go mod why -m, as go mod why works on packages. If go mod why -m still tells you that a module is not needed even when it's in your go list -m all, then it is this bug.

@OJFord
Copy link

OJFord commented May 27, 2022

If go mod why -m still tells you that a module is not needed even when it's in your go list -m all, then it is this bug.

Yes, that is the case. Thanks.

@LeGEC
Copy link

LeGEC commented Jun 29, 2022

@OJFord you want go mod why -m, as go mod why works on packages. If go mod why -m still tells you that a module is not needed even when it's in your go list -m all, then it is this bug.

@mvdan : I think I have reached that situation, which you describe as a bug :

I was looking into a way to cut the dependency on go.opencensus.io from googleapis/google-api-go-client.

My current attempt is here : https://github.com/LeGEC/google-api-go-client/ (at commit 3005284e02)

On my laptop, I am currently in the following state :

$ go version
go version go1.18.1 linux/amd64
$ go mod tidy
$ grep -l -e go.opencensus.io -r .   # the only files where 'go.opencensus.io' is mentioned are go.sum files
./internal/kokoro/discogen/go.sum
./go.sum

# go.opencensus.io is listed in 'go list -m all'
$ go list -m all | grep opencensus
github.com/census-instrumentation/opencensus-proto v0.2.1
go.opencensus.io v0.23.0

# both go mod why and go mod why -m say that package is not needed :
$ go mod why go.opencensus.io
# go.opencensus.io
(main module does not need package go.opencensus.io)
$ go mod why -m go.opencensus.io
# go.opencensus.io
(main module does not need module go.opencensus.io)

I'm trying to clear the various caches I am aware of (GOCACHE and GOMODCACHE), I still have this package mentioned in go.sum and go list -m all, but not used by the code.

@bcmills
Copy link
Contributor Author

bcmills commented Jun 29, 2022

@LeGEC, I'm happy to help with that problem but it's a bit off-topic for this issue.

Consider starting a thread on golang-nuts, or find me in the #modules channel on the Gophers Slack.

@jan--f
Copy link

jan--f commented Jul 20, 2023

@LeGEC, I'm happy to help with that problem but it's a bit off-topic for this issue.

Consider starting a thread on golang-nuts, or find me in the #modules channel on the Gophers Slack.

@bcmills any chance you can link a follow-up here is there was one? I'm in a similar situation as described in #27900 (comment) where a CVE is involved and its unclear if our package is impacted. The vulnerable module is mentioned in go.sum and go list -m all, but go mod why -m x claims (main module does not need module x)

@bcmills
Copy link
Contributor Author

bcmills commented Jul 20, 2023

@jan--f, if the CVE in question is included in https://pkg.go.dev/vuln/, I would suggest using govulncheck to investigate its impact on your module.

@jan--f
Copy link

jan--f commented Jul 20, 2023

It's not yet merged unfortunately, though proposed. In any case I experience the same situation @LeGEC describes, so I'd be thankful for pointer to follow up on any threads of slack conversations.

@tonglil
Copy link

tonglil commented Aug 4, 2023

@OJFord you want go mod why -m, as go mod why works on packages. If go mod why -m still tells you that a module is not needed even when it's in your go list -m all, then it is this bug.

This is the case.

@oshoval
Copy link

oshoval commented Dec 12, 2023

If go mod why -m still tells you that a module is not needed even when it's in your go list -m all, then it is this bug.

This is the case.

Same here, can give a reproducible env for it if needed
even after go clean -modcache

@OJFord
Copy link

OJFord commented Dec 12, 2023

Can we quote the rest of that paragraph without pinging me going forwards please 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests