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

Jump to bottom

cmd/go: don't include the dependencies of external tests in the dependency graph #33857

Closed
leighmcculloch opened this issue Aug 27, 2019 · 7 comments
Closed

cmd/go: don't include the dependencies of external tests in the dependency graph #33857

leighmcculloch opened this issue Aug 27, 2019 · 7 comments
Labels
FrozenDueToAge

Comments

@leighmcculloch
Copy link
Contributor

leighmcculloch commented Aug 27, 2019 •
edited

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

$ go version
go version go1.13rc1 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
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/leighmcculloch/local/bin"
GOCACHE="/home/leighmcculloch/.cache/go-build"
GOENV="/home/leighmcculloch/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/leighmcculloch/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/leighmcculloch/local/bin/go/1.13rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/leighmcculloch/local/bin/go/1.13rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build523664316=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Migrate a repository from Dep to Modules (stellar/go#1634) that is currently dependent on:

  • github.com/go-sql-driver/mysql@2e00b5cd7039
  • github.com/jmoiron/sqlx@v1.2.0

What did you expect to see?

These dependencies to be selected by Modules at the same versions, as listed above.

What did you see instead?

Modules selected the following versions:

  • github.com/go-sql-driver/mysql@v1.4.0
  • github.com/jmoiron/sqlx@v1.2.0

According to go mod why and go mod graph the reason Modules is requiring I use at least mysql@v1.4.0 is because sqlx.test depends on mysql@v1.4.0. The sqlx package itself does not import mysql in any file, and it is only _test.go files that import the mysql dependency.

From what I can see the dependency graph looks like this:

                 +-----------------------------+
                 | +------+      +-----------+ |
      +----------->+ sqlx +<-----+ sqlx.test | |
      |   v1.2.0 | +------+      +-----------+ |
      |          +-----------------------------+
+-----+-----+                          |
| mypackage |                          |
+-----+-----+                          |
      |                                |
      |            +-------+           |
      +----------->+ mysql +<----------+
                   +-------+ v1.4.0

Wrangling with dependencies that are being restricted by test-only dependencies is something I've been struggling with over the last week and this is just one example.

Given that mypackage is not dependent on sqlx.test it seems inconvenient, unintuitive, and unnecessary that mypackage would inherit the requirements that are unique only to the tests of sqlx. In the reading I did about my understanding of why this exists I could only find comments from folks mentioning a developer might want to run all the tests for all the dependencies of a package. But in this situation, won't the tests for each dependency be built separately, and executed as their own main, and could assume their own dependency graphs without infecting the dependency graph of mypackage? It seems like we could make this easier to work with by limiting the impact on the main modules dependency graph by not including external test dependencies.
@av86743
Copy link

av86743 commented Aug 27, 2019

Discussed before by @bcmills et al.

With a present "fluent" state of module support in go and policies of the project, I think that most you may be able to get out of your complaint is a split presentation of dependencies in go.mod where test-only dependencies are grouped separately, to address cluttering part of the problem:

require (
   ...
   /// test-only dependencies
   ...
)

@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

won't the tests for each dependency be built separately, and executed as their own main, and could assume their own dependency graphs without infecting the dependency graph of mypackage?

Then you wouldn't be able to diagnose problems in your dependencies by running their tests, because running the test would potentially result in a different configuration from the one in which you observed the problem.

@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

Duplicate of #26626

@bcmills bcmills marked this as a duplicate of #26626 Aug 27, 2019
@bcmills bcmills closed this as completed Aug 27, 2019
@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

most you may be able to get out of your complaint is a split presentation of dependencies

That's #26955.

@av86743
Copy link

av86743 commented Aug 27, 2019

most you may be able to get out of your complaint is a split presentation of dependencies

That's #26955.

@bcmills This is trivial to implement - only generated canonical external representation will be affected, with fully backward compatible content. I am surprised that you do not even have a CL for this yet. So that those willing could forward apply it - like for #32389.

@bcmills
Copy link
Contributor

bcmills commented Aug 27, 2019

I am surprised that you do not even have a CL for this yet.

@av86743, remember that among our Gopher values is to “be charitable”.

The reason we don't have a CL for this yet is because we've been dealing with the (literally) hundreds of other cmd/go and module-related issues and feature requests. It's not from lack of effort or attention.

@leighmcculloch
Copy link
Contributor Author

Thanks for responding so quickly @bcmills and pointing me to an issue that has comments discussing the why behind this behavior. Linking here for anyone else who needs it: #26626 (comment).

@golang golang locked and limited conversation to collaborators Aug 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@leighmcculloch @av86743 @bcmills @gopherbot