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 -deps all foo' erroneously loads test dependencies of packages imported by foo #40799

Closed
bcmills opened this issue Aug 14, 2020 · 3 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Aug 14, 2020

go list -deps all foo should list the dependencies of the packages in all and the dependencies of package foo, but should neither list nor attempt to resolve the test dependencies of other packages imported by foo.

Similarly, go list -deps -test all foo should list the test dependencies of the packages in all and the test dependencies of foo itself, but not the test dependencies of other packages imported by foo.

Today, it appears that go list erroneously resolves those dependencies, but successfully omits them from the listing. The erroneous resolution can be detected as a spurious change in the go.mod file when foo has an incomplete (“untidy”) go.mod file.

The test case below illustrates:

# This test demonstrates go commands that combine the 'all' pattern
# with packages outside of 'all'.

# With -deps, 'all' should include test dependencies of packages in the main
# module, but not should not include test dependencies of packages imported only
# by other root patterns.

cp go.mod go.mod.orig

go list -deps all x/otherroot

stdout '^x/inall$'
stdout '^x/inall/fromtest$'
stdout '^x/inall/fromtestinall$'
stdout '^x/otherroot$'
stdout '^x/otherdep$'

! stdout '^x/fromotherroottest$'
! stdout '^y/fromotherdeptest$'

cmp go.mod go.mod.orig  # FAIL: added erroneous requirement on y.

# With -deps -test, test dependencies of other roots should be included,
# but test dependencies of non-roots should not.

go list -deps -test all x/otherroot
stdout '^x/inall$'
stdout '^x/inall/fromtest$'
stdout '^x/inall/fromtestinall$'
stdout '^x/otherroot$'
stdout '^x/otherdep$'

stdout '^x/fromotherroottest$'
! stdout '^y/fromotherdeptest$'

cmp go.mod go.mod.orig  # FAIL: added erroneous requirement on y.

-- m.go --
package m

import _ "x/inall"
-- m_test.go --
package m_test

import _ "x/inall/fromtest"
-- go.mod --
module m

go 1.15

require x v0.1.0

replace (
	x v0.1.0 => ./x
	y v0.1.0 => ./y
)
-- x/go.mod --
module x

go 1.15
-- x/inall/inall.go --
package inall
-- x/inall/inall_test.go --
package inall_test

import _ "x/inall/fromtestinall"
-- x/inall/fromtest/fromtest.go --
package fromtest
-- x/inall/fromtestinall/fromtestinall.go --
package fromtestinall
-- x/otherroot/otherroot.go --
package otherroot

import _ "x/otherdep"
-- x/otherroot/otherroot_test.go --
package otherroot_test

import _ "x/fromotherroottest"
-- x/fromotherroottest/fromotherroottest.go --
package fromotherroottest
-- x/otherdep/otherdep.go --
package otherdep
-- x/otherdep/otherdep_test.go --
package otherdep_test

import _ "y/fromotherdeptest"
-- x/otherroot/testonly/testonly.go --
package testonly
-- y/go.mod --
module y

go 1.15
-- y/fromotherdeptest/fromotherdeptest.go --
// Package fromotherdeptest is a test dependency of x/otherdep that is
// not declared in x/go.mod. If the loader resolves this package,
// it will add this module to the main module's go.mod file,
// and we can detect the mistake.
package fromotherdeptest
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 14, 2020
@bcmills bcmills added this to the Go1.16 milestone Aug 14, 2020
@bcmills bcmills self-assigned this Aug 14, 2020
@bcmills
Copy link
Contributor Author

bcmills commented Aug 14, 2020

(Found by code inspection while working on #36460.)

@gopherbot
Copy link

Change https://golang.org/cl/222341 mentions this issue: cmd/go: add baseline test cases for non-lazy module loading

@gopherbot
Copy link

Change https://golang.org/cl/240505 mentions this issue: cmd/go/internal/modload: track which packages are in 'all' during loading

gopherbot pushed a commit that referenced this issue Aug 24, 2020
For #36460
For #40799

Change-Id: Id55934cc4d66743a4087b4c2644b6c3b95e7d2ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/222341
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Sep 9, 2021
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants