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

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

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

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

jehiah opened this issue Feb 13, 2018 · 8 comments
Labels
Proposal Proposal-Hold
Milestone
Proposal

Comments

@jehiah
Copy link

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
Copy link
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
Copy link
Author

jehiah commented Feb 13, 2018

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

@mvdan
Copy link
Member

mvdan commented Feb 13, 2018

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

@mvdan mvdan added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 13, 2018
@mvdan mvdan added this to the Go1.11 milestone Feb 13, 2018
@mvdan
Copy link
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
Copy link
Contributor

stapelberg commented Feb 21, 2018 •
edited

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 cmd/go: add recursive Deps for TestImports and XTestImports Proposal: cmd/go: add recursive Deps for TestImports and XTestImports Feb 21, 2018
@mvdan mvdan modified the milestones: Go1.11, Proposal Feb 21, 2018
@mvdan
Copy link
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
Copy link
Author

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 Proposal: cmd/go: add recursive Deps for TestImports and XTestImports proposal: cmd/go: add recursive Deps for TestImports and XTestImports Feb 26, 2018
@rsc
Copy link
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 changed the title proposal: cmd/go: add recursive Deps for TestImports and XTestImports proposal: cmd/go: add transitive Deps for TestImports and XTestImports Feb 26, 2018
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal Proposal-Hold
Projects
None yet
Milestone
Proposal
Development

No branches or pull requests
5 participants
@jehiah @stapelberg @rsc @mvdan @gopherbot