x/tools/go/packages: add module information to the Package struct

[a8m]

Currently, packages.Load returns a packages.Package object that contains useful information from the go list API. However, sometimes the module information for the requested package is needed as well.

My proposal is basically to add the modinfo.ModulePublic info from go list to this package.

Similar to go list, if the package is not part of a module or it's provided by the Overlay API, Package.Module will be nil.

Let me know what do you think about this and I'll submit my CL for the proposal.

[zikaeroh]

This would probably make #35563 much simpler to implement.

[rsc]

/cc @matloob @ianthehat

[matloob]

For now the x/tools/go/packages package is meant to be build-system agnostic. Since modules don't appear in all Go build systems, that means that if we're keeping to being build-system agnostic.

That might not be the right decision, but I think we should revisit that decision before deciding on this.

Issue #38234 is somewhat relevant to this by the way.

[rsc]

Being build-system agnostic shouldn't mean ignoring core Go features, though.
If you're using Bazel those fields would be empty.

[matloob]

I'm merging this issue with #38446.

The main differences are that we still need to discuss exactly what struct we're putting in Package (module public is unexported, and whichever struct we pick should probably be defined in x/mod), and that we'd need a Need bit for the field.

[rsc]

Here's the Module struct from go help list:

When listing modules, the -f flag still specifies a format template
applied to a Go struct, but now a Module struct:

    type Module struct {
        Path      string       // module path
        Version   string       // module version
        Versions  []string     // available module versions (with -versions)
        Replace   *Module      // replaced by this module
        Time      *time.Time   // time version was created
        Update    *Module      // available update, if any (with -u)
        Main      bool         // is this the main module?
        Indirect  bool         // is this module only an indirect dependency of main module?
        Dir       string       // directory holding files for this module, if any
        GoMod     string       // path to go.mod file used when loading this module, if any
        GoVersion string       // go version used in module
        Error     *ModuleError // error loading module
    }

    type ModuleError struct {
        Err string // the error itself
    }

Is that what we want in go/packages? Is there anything we don't want in go/packages?

[matloob]

That sounds reasonable to me? I don't see anything that we'd obviously not want. Though I'd like to hear what @mvdan and @dominikh think.

[dominikh]

SGTM. Will go/packages acquire Versions and Update? If not, we may want to skip those.

[matloob]

Good point. Those could potentially be passed through as build flags, but that doesn't sound like something we'd want to encourage.

[rsc]

It does sound like we should leave out Versions and Update, making the struct we're talking about adding:

type Module struct {
    Path      string       // module path
    Version   string       // module version
    Replace   *Module      // replaced by this module
    Time      *time.Time   // time version was created
    Main      bool         // is this the main module?
    Indirect  bool         // is this module only an indirect dependency of main module?
    Dir       string       // directory holding files for this module, if any
    GoMod     string       // path to go.mod file used when loading this module, if any
    GoVersion string       // go version used in module
    Error     *ModuleError // error loading module
}

type ModuleError struct {
    Err string // the error itself
}

I'm not sure about ModuleError either, but we can see how it goes.

It sounds like people agree with the idea of adding the info. We can fine-tune the exact fields separately but the above is a start.

Based on the discussion above, this sounds like a likely accept.

[rsc]

No change in consensus, so accepted.

[gopherbot]

Change https://golang.org/cl/234219 mentions this issue: go/packages: add a Module field to the Package struct