Go: proposal: x/tools/go/packages: add Dir and Target (and perhaps other) fields to Package struct

[matloob]

This is proposal 1 of 3 coming out of the discussion of issue #38234.

We propose adding the following fields to the Package struct, which might not necessarily be present for all build systems:

• Dir, which would be the directory associated with the package, if any. For the go build system, this would be the directory the package is contained in. This might not be as clearly defined for Bazel, but might be the directory the BUILD file is in.
• Target, which is the install path of the .a file, for libraries, and executable file for binaries,

and perhaps other such fields, pending the discussion. If these fields are added, we'd need to also add Need fields. We'd also need to make it clear to users that these fields might not be set, even if they are requested via needs fields.

[dominikh]

I would really like to see ForTest exposed, but that one is only defined for Go's own build system.

[myitcv]

Agreed on ForTest. It's exposed internally and used in gopls, which confirms to my mind it's useful. If necessary, this could be a build system-specific field.

[matloob]

I think if the proposal to add the field with build system specific information as accepted, ForTest would best belong there. Otherwise, I could see an argument to put it directly on Package. I just want to be careful about the number of fields we add to Package-- I'm a bit worried it would grow too large.

[dominikh]

I'm a bit worried it would grow too large

If proposal 3 doesn't get accepted, then I would argue that go/packages should support the important features of the most common build system, which is go list.

[matloob]

If proposal 3 doesn't get accepted, then I would argue that go/packages should support the important features of the most common build system, which is go list.

That's reasonable. I think we should probably do one or the other.

[dominikh]

Another field I would deem useful is Match:

Match []string // command-line patterns matching this package

This would allow tools to emulate Go's behavior of warning about patterns that don't match any packages, a la go: warning: "./foobar/..." matched no packages.

Whether me needing yet another field pushes us more towards proposal 3 I don't know.

[mvdan]

Another one would be a package's build ID, once/if https://github.com/golang/go/issues/37281 is implemented. We could call the field BuildID, but if we want something more generic, we could also go for CacheKey.

[matloob]

Hm, I'm thinking that BuildID is so low-level that it's reasonable to ask users to run go list -json themselves.

Match is reasonable: it was a pretty early request when we were developing go/packages.

[dominikh]

Hm, I'm thinking that BuildID is so low-level that it's reasonable to ask users to run go list -json themselves.

The issue, as so often, is performance: we want to use go/packages to be build-system agnostic, but we also want low-level details when available, without incurring the cost of a second go list invocation. In particular, our desire to get the BuildID is as an optimization to avoid computing our own hash of build inputs; but computing the hash is still much faster than calling go list twice (once via go/packages, once directly).

[heschik]

I thought Match wasn't implementable in Bazel?

[matloob]

It isn't. If this proposal is accepted, we'll relax our requirement that all of Package's fields correspond to concepts that exist in every build system. It'll be important to set the expectation for users that the fields won't be filled for every build system.

We'll have to be careful about this to make sure that we don't end up with too many tools that don't work on Blaze/Bazel.

[matloob]

I'm hoping that fixing this (adding some build-system-specific fields to Package) will provide users enough information so that #38448 isn't needed. So I'd like to hear if there are any other build-system-specific fields users need.

I think we should have a plan for what we're going to do with both proposals before proceeding.

[mvdan]

@matloob ping - has this advanced in the past few months? Or are you waiting for more input from users?

[jpap]

I'd love to see a Dir field here also -- "go/build".Packages has it, and it would be very useful here too.

I am currently using a kludgey workaround:

• Request NeedFiles
• Perform the Load
• Pick off the first GoFiles[0], and extract the path, relying on the fact that GoFiles represent the absolute path to each file within the package.

At first I did try to retrieve the Module with NeedModule, but that of course doesn't work with GOPATH packages.