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

proposal: x/exp: add a package to wrap 'go list -json' #38234

Closed
mvdan opened this issue Apr 3, 2020 · 6 comments
Closed

proposal: x/exp: add a package to wrap 'go list -json' #38234

mvdan opened this issue Apr 3, 2020 · 6 comments

Comments

@mvdan
Copy link
Member

mvdan commented Apr 3, 2020

Summary

We have go/packages as a high-level API for loading Go packages. It's abstracted away from go list -json, because it's also meant to support other build systems like Bazel.

I propose that we add a lower-level, simple Go API to call go list -json and go list -json -m. Given that it's only meant to support go list, it can show more information, and it can support listing modules too.

Description

There are plenty of use cases where one can rightfully only support Go modules and GOPATH, and not care about other build systems. For example, a code generation tool that's only used by a project's developers, when they are happy with using modules alone. Or a piece of tooling internal to a company, if they are confident that they won't switch build systems anytime soon.

In those scenarios, using go/packages can be enough if one's needs are simple. For example, looking at the Go files contained in a number of packages. But, in the context of modules, it can be pretty easy to run into limitations for little reason, such as:

  • What module does this package belong to?
  • Where is this package (in what directory)?
  • Is this package part of the standard library?
  • Which input patterns matched each resulting package?

All of these are contained by go list -json, but aren't exposed by go/packages, since they aren't common with other Go build systems. That makes sense, but it's a tradeoff that isn't right sometimes.

One can call go list -json directly - for example, to obtain a package's directory alone. Given the structs that one can see in go help list:

out, err := exec.Command("go", "list", "-f", "{{.Dir}}", pkgPath)
if err != nil { ... }
dir := string(bytes.TrimSpace(out))

However, it quickly gets complicated if one needs multiple values at once, or if one wants to look at multiple packages in a single call. For example, these open questions aren't easy to answer, for anyone who isn't very familiar with the Go tool:

  • Should I just copy the structs from go help list? How can I update them in the future without manual steps?
  • How should I deal with errors? Do I want to use the -e flag?
  • If I use -e, can the Go command still return a non-zero status code?

It's possible to make some assumptions or guesses here which might be OK for the user, but even then, the user will likely end up manually copying some code from go help list, and writing 20 to 40 lines of extra code with os/exec and encoding/json.

Proposed solution

As explained before, I propose that we add a package with two new APIs - loading packages, and loading modules. The latter would only work in module mode, just like go list -m. I have a proof of concept implementation here, which I'm using in a couple of projects, and which I'd be happy to donate via a CL.

The code isn't terribly complex or large, but I think it's easy to tell that exposing good errors isn't trivial.

I think x/exp would be a reasonable place for it to live for now, to give us some space to redesign the API later if we want. I haven't thought too hard about the package name itself, but one could imagine list following go list, or modules, as an analogous to packages given that long-term GOPATH will probably be deprecated/removed in favour of modules.

Considered alternatives

  1. I seem to recall some ideas about splitting cmd/go into smaller packages, such as what one can find in https://godoc.org/golang.org/x/mod. I think that's a great initiative, and I hope that module is expanded. However, I struggle to imagine how we could ever fit an API like this in x/mod:
  • Would we still call go list? The rest of the module is "pure Go" without exec, as far as I can tell
  • If our API there were pure Go, it would require pulling in a huge amount of code from cmd/go/internal, and it would mean that we no longer use the Go version that the machine has available.
  • If our API there used os/exec, it probably wouldn't use any other part of the x/mod module, and in my own mind would defeat the purpose of sharing the module
  1. Long term, I'd like for this package to live somewhere like x/tools/modules. However, I don't want to rush that decision, especially given how difficult it was to stabilize x/tools/packages.

CC @jayconrod @matloob @ianthehat @myitcv @leitzler

@mvdan mvdan added the GoCommand cmd/go label Apr 3, 2020
@mvdan mvdan added this to the Proposal milestone Apr 3, 2020
@matloob
Copy link
Contributor

matloob commented Apr 3, 2020

I think this is reasonable but we should have a discussion about it. Maybe we can schedule a video call to talk about it?

I think the issues you raised about copying the package struct and errors are valid, and at the very least, we should make the definition of the struct that go list -json outputs public. The confusion about go list -e errors is also a problem, but I think that's a deeper problem that will only be fixed by fixing the go command.

One more thing that we should be careful about: I'm worried that tools that don't need syntax and types might use the go list wrapper instead of go/packages, even when they don't need to. I'm not sure how we'd prevent that from happening.

@mvdan
Copy link
Member Author

mvdan commented Apr 3, 2020

Maybe we can schedule a video call to talk about it?

Sure! We have our monthly golang-tools catchup next week, so I'll propose a time then.

at the very least, we should make the definition of the struct that go list -json outputs public

If we decide to only do this, I guess they could go in x/mod.

The confusion about go list -e errors is also a problem, but I think that's a deeper problem that will only be fixed by fixing the go command.

It's one of the problems, but overall, running go list -json properly isn't trivial. So I'd prefer to not leave it to the user.

might use the go list wrapper instead of go/packages, even when they don't need to

A similar concern had come to mind, but I imagine it wouldn't be a big problem. Most people know go/packages by now, and its name is very clear. This package's godoc could have a big warning about encouraging go/packages for cases where one only wants to load Go packages in a high-level way.

@matloob
Copy link
Contributor

matloob commented Apr 8, 2020

We'll be discussing this on a video call at meet.google.com/btn-cryq-uwg, this Friday 10 April at 11:30 ET (15:30 UTC)

@rsc
Copy link
Contributor

rsc commented Apr 9, 2020

I can't make the meeting, but I think go/packages should be the one true API. If it's not good enough, it should be made good enough. Bazel should not hold it back. We shouldn't have two APIs that do the same thing.

@mvdan
Copy link
Member Author

mvdan commented Apr 9, 2020

@dominikh raised a similar point on Slack. I don't object to including the go list -json info into go/packages. However, that doesnt answer the part about go list -m -json, which I think is equally important.

@matloob
Copy link
Contributor

matloob commented Apr 14, 2020

As per the discussion at our meeting, issues #38445, #38446, and #38448 have been created, and now supersede this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants