x/tools/go/packages: tags proposal / design inquery

[kardianos]

Regarding package golang.org/x/tools/go/packages.

I'm looking forward to Go modules. When go1.11 lands I hope to convert some work projects over from govendor to go modules. At work we have a policy to review all dependencies. We currently facilitate this by reviewing the vendor directory. One main difference between the govendor vendor tree and the Go modules vendor tree is that govendor doesn't copy un-imported packages. Furthermore, it also has a custom package loader that allows flexibly choosing tags, GOOS, and GOARCH. In order to make this govendor -> Go modules go smoothly, I plan to make a tool that trims out the unused packages from the vendor folder.

To trim out the unused packages from the vendor folder, I plan to:

• Use the golang.org/x/tools/go/packages to load packages.
• Determine what packages are un-used according to govendor style rules.
• Remove the un-used package from the vendor folder.

There are two problems with using the new packages package for this purpose:

1. It requires a specific GOOS and GOARCH. I would Ideally like to analyze the full set of packages regardless of GOOS and GOARCH (though I'd like to potentially black-list some GOOS similar to tags below).
2. I don't see any method to specify build tags. I'm unsure if this can be passed into Env or Flags fields, but I would like to see this more flexible. govendor by default includes all tags, but allows excluding tags, GOOS, GOARCH, test.

I will avoid speculating on a design that would allow these features until I hear back to see if these would even be in scope. If not in scope I'll write a version that supports what I need directly.

/cc @suzmue

[suzmue]

/cc @matloob @ianthehat

[matloob]

(a) is difficult to do: module resolution depends on the imports in your build, which in turn depends on GOOS and GOARCH. I'm not sure how we could design around that. It seems like the only option is to run through the GOOS x GOARCH product and re-Load the packages in each configuration...
(b) We were planning to add tags as an option, but decided that it would be cleaner to just let them be passed in through the Flags field. Perhaps we should add a comment to Flags to indicate where we'd pass in the flags?

[kardianos]

It seems like the only option is to run through the GOOS x GOARCH product and re-Load the packages in each configuration...

It really depends on what you are trying to do. If you are attempting to calculate the maximum dependency graph (as I am here with exceptions), then you can just read all relevant files to do so. However, most tools (go, others) that this probably shells out to (I'm assuming) won't accept that.

Between this assumption of GOOS and GOARCH and the tags being loaded into the Flags, upon further investigation the design of packages is really a papering over of other build tools with some useful hooks. If this is accurate then what I'm trying to do will be out of scope (which is fine).

---

On a site note: If this is intended to be a general purpose package loader tools such as guru or other code refactoring tools should use, I would air a few concerns:

• It is reasonably common for a variable that needs to be renamed to span across multiple files guarded by different GOOS x GOARCH.
• Expressing tags under "Flags" seems like really bad UX to me. I would recommend to decide what this package should support and require implementations interfaces (go, bazel) to know how to specify tags and such on their own.

I could be mis-understanding the intended user of this package too (I thought it would be used by general purpose tools like goimports and guru).

If our interests align, I could certainly work on a part of this, but I don't want to step on toes either.

[ianthehat]

To really answer your question I would need to know slightly more about your goals, and their granularity.

If you are using go in module mode, then it ignores the vendor folder, so any kind of checks on the vendor folder are not going to be much use to you. Assuming you know that and you are just using the vendor folder as a way of materializing information about your build, there are a few options available to you:

• If you want to produce a vendor folder with no unused dependencies, I think you can just run go mod -sync to prune your module file, and then go mod -vendor to rebuild the vendor folder. It's not quite the same, because pruning is at the module level not the package level.

• If you want to control allowed reviewed dependencies, the idea is that you would have your own module proxy that only has allowed modules in it, which you can do for your entire organization rather than a single project.

• If you just want to check the module dependencies, you can do that with go list -m all or go mod -graph

• If really want to see what you depend on at the package level, you can use the go/packages API to walk that information for a specific build configuration. You are going to have to iterate through all the valid build configurations for your environment at a higher level though, but you can do that by setting Flags and Env in the Config.

It is not really feasible for us to process all build configurations, because the number of possible combinations is too large to do them all, but trying to do it for all configurations at once can result in a cyclic graph (it is valid to have a dependency edge flip direction depending on tagged out files, and I have seen an example of someone doing it).
It would also not be possible to run any of the higher modes because the source list would not type check, so it would have to be a mode dependent option, something we have been trying to avoid.

[kardianos]

@ianthehat (real quick background: I'm the author of govendor, I've made some minor contributions to vgo and generally understand the code there, I understand what flags are available, and I understand that -sync does a maximum graph resolution while go build inspects GOARCH and GOOS).

To really answer your question I would need to know slightly more about your goals, and their granularity.

I spelled out my exact goals. I'm not sure what more to tell you.

because pruning is at the module level not the package level.

Yes, I specifically called this out. I'm looking for package level pruning with custom tag/GOOS/GOARCH exclusions.

If you just want to check the module dependencies, you can do that with go list -m all or go mod -graph

This doesn't work because I'm not looking for the pure maximal graph, as I'd like to prune based on tags/GOOS/GOARCH.

walk that information for a specific build configuration.

That's a non-starter. It really is. If this is going to be used by guru (goimports might be fine), this design concerns me. But it also isn't my call :).

It is not really feasible for us to process all build configurations,

Right. That's not a good idea. (So don't do that?) You know how go mod -sync walks the source tree differently then go build? I'm looking for a variant of how go mod -sync walks the tree. You are implementing packages like how go build walks the source tree.

It would also not be possible to run any of the higher modes because the source list would not type check, so it would have to be a mode dependent option, something we have been trying to avoid.

So again, I'll don't think you can avoid the difference; it is inherit complexity. guru will need all related symbols so it can truly do a implements or rename function properly. But a build tool or a type checker must resolve to a concrete GOOS and GOARCH. The difference isn't large in resolution details, but it is fundamental and needed.

I'll close the issue as I'm not interested in a package resolver that requires a GOOS and GOARCH.

Thanks for your time.

[kardianos]

I stand corrected. Existing tools today (guru, gorename) only use a single GOOS x GOARCH. yikes. I learn something new each day. :/

[ianthehat]

I used govendor on multiple projects in the past (thanks btw!) and recognized your name from the import path :)
I was just trying to be very explicit mostly for anyone else reading along. There seems to be some confusion about the granularity of repository > module > package right now, so I am being careful in my responses to be very clear, mostly because these comments live forever and turn up in searches.

As you say, "a type checker must resolve to a concrete GOOS and GOARCH"; all existing code inspection tools (godef, guru etc) and refactoring tools (gorename, eg etc) that I know of require type information to work, so they all run in a single configuration.
In theory one could write a version of the types package that could cope, but in practice the extra complexity in the API (allowing multiple edges where the language only allows one) would be a hard sell to people that mostly just don't care about multiple configurations.
We also report and use the export data, which is inherently single configuration, and because go list reports the processed cgo files, that also needs to be done in a specific configuration.

For inspection tools, it's never really a problem, editing/debugging in the default configuration is what most people expect if they even think about it at all. It is normally possible to modify the configuration if really needed.

For refactoring tools its a much more serious problem, because refactoring in a single configuration causes breaking changes to the other configurations. I don't know of any attempts to fix this at the moment, but we do have some ideas about the right way to write these kinds of tools, it involves a higher level API that probably sits on top of go/packages and probably runs it multiple times making safe incremental changes each time.

Expanded package graphs are a rare enough use case that they probably need custom code, you are probably right to not use the go/packages API. We are trying to keep this API as simple as possible for the common go source inspection tools that people tend to write, we explicitly chose to leave out some power use cases from this API to keep the simple common case easy. If you can see a good way to add it, we would welcome suggestions, the design is far from set in stone, it is explicitly in the x/tools repository so that we are able to make changes based on people really trying to use it.

We may add further more powerful APIs to a different package in the future, and would be happy to collect any thoughts you have about what those APIs need to expose, and good ways to expose it.