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

proposal: cmd/go: add transitive Deps for TestImports and XTestImports #23806

Open
jehiah opened this Issue Feb 13, 2018 · 8 comments

Comments

Projects
None yet
5 participants
@jehiah

jehiah commented Feb 13, 2018

go list provides a .Deps which has the recursive list of dependencies for .Imports. This is important because it provides the complete list of dependencies that must be fulfilled to run go build

Unfortunately there is no recursive dependency provided for TestImports or XTestImports. Those have a dependency tree that needs to be fulfilled in order to go test but it's non-trivial to retrieve that dependency list. (especially when trying to filter out stdlib entries)

Currently

// Dependency information
Imports      []string // import paths used by this package
Deps         []string // all (recursively) imported dependencies
TestImports  []string // imports from TestGoFiles
XTestImports []string // imports from XTestGoFiles

What's expected?

// Dependency information
Imports      []string // import paths used by this package
Deps         []string // all (recursively) imported dependencies
TestImports  []string // imports from TestGoFiles
TestDeps     []string // all (recursively) imported dependencies from TestGoFiles
XTestImports []string // imports from XTestGoFiles
XTestDeps    []string // all (recursively) imported dependencies from XTestGoFiles

This would allow a command like the following to output the complete set of dependencies needed to run go build or go test.

go list -f '{{join .Deps "\n"}}{{if len .TestDeps}}{{"\n"}}{{end}}{{join .TestDeps "\n"}}{{if len .XTestDeps}}{{"\n"}}{{end}}{{join .XTestDeps "\n"}}' | sort | uniq

Currently you have to run go list '{{join .Deps "\n"}}' $import_path on each import path from .TestImports and .XTestImports.

@mvdan

This comment has been minimized.

Member

mvdan commented Feb 13, 2018

How about one single .TestDeps, holding .Deps plus all the transitive test dependencies?

That would mirror what the get command has. For example, .Deps is the same that it uses in go get, while .TestDeps would be the same that it uses in go get -t.

@jehiah

This comment has been minimized.

jehiah commented Feb 13, 2018

@mvdan that seems reasonable and targets my use case equally well.

@mvdan

This comment has been minimized.

Member

mvdan commented Feb 13, 2018

Happy to give it a go if others agree on the design.

@mvdan mvdan added the NeedsDecision label Feb 13, 2018

@mvdan mvdan added this to the Go1.11 milestone Feb 13, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Feb 13, 2018

Also, I'm thinking now that it probably requires a better name, if we are to mirror go get -t. TestDeps seems to mean "only the test dependencies", not "all dependencies including the test ones".

Perhaps DepsWithTest or DepsInclTest. Or perhaps we should make TestDeps be the transitive test dependencies only, and it's up to the user to use both Deps and TestDeps if required (or merge the lists, somehow).

@stapelberg

This comment has been minimized.

Contributor

stapelberg commented Feb 21, 2018

The discussed change would be valuable for Debian as well, where we currently query {{ .Deps }}, and entirely miss dependencies which are only used for testing.

Edit: it was pointed out to me that in the particular use-case (generating the Built-Using field), excluding testing deps is actually correct, so nevermind :).

@mvdan mvdan changed the title from cmd/go: add recursive Deps for TestImports and XTestImports to Proposal: cmd/go: add recursive Deps for TestImports and XTestImports Feb 21, 2018

@gopherbot gopherbot added the Proposal label Feb 21, 2018

@mvdan mvdan modified the milestones: Go1.11, Proposal Feb 21, 2018

@mvdan

This comment has been minimized.

Member

mvdan commented Feb 21, 2018

I'm repurposing this as a proposal, in the hopes that a decision will be made on it while the Go 1.11 tree is open. The proposed change isn't trivial either, so it makes sense to have a proposal.

@jehiah

This comment has been minimized.

jehiah commented Feb 21, 2018

Thanks @mvdan. Let me know if i can be of any assistance with context related to my use.

@rsc rsc changed the title from Proposal: cmd/go: add recursive Deps for TestImports and XTestImports to proposal: cmd/go: add recursive Deps for TestImports and XTestImports Feb 26, 2018

@rsc

This comment has been minimized.

Contributor

rsc commented Feb 26, 2018

We will need to address this more generally for the new go/loader package. I think this can wait until after that's done - it will probably just fall out naturally.

@rsc rsc added the Proposal-Hold label Feb 26, 2018

@rsc rsc changed the title from proposal: cmd/go: add recursive Deps for TestImports and XTestImports to proposal: cmd/go: add transitive Deps for TestImports and XTestImports Feb 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment