gocode and godef issues with go/packages

[Paul Jolly]

Hi Ian, Rebecca,

I've boiled down the issues I've been seeing with gocode and godef to
the following

* godef seems to have issues with build constraints and package docs
* gocode seems to have issues with build constraints

I suspect the issues are linked in some way.

The build constraints I suspect are a function of the fact that a
gocode completion request or a godef jump request don't include any
sort of build tags, hence files with build constraints get ignored by
go/packages?

I can raise separate issues in your forks if you enable Issues in
github if you prefer?

Here is a gist with two files: the first is the output on my setup,
the second is the full script:

https://gist.github.com/myitcv/10b13ce65e6655bdadc68e3c1ba719d7

(I've been running this within a Docker container to ensure the gocode
daemon process is isolated)

Let me know if you need any more details.

Thanks,


Paul

[Paul Jolly]

To add to this slightly:

 

* godef seems to have issues with build constraints and package docs
* gocode seems to have issues with build constraints

godef seems to have problems with any comment appearing before the package declaration, regardless of whether it's a package comment, build constraint or something like:

// Code generated by XXX. DO NOT EDIT.

 

[Rebecca Stambler]

Hi Paul,

Thanks for the detailed example - I just tried it out and was able to reproduce. go/packages does allow us to pass build tags, but I think more work still needs to be done on both the go/packages side and the gocode/godef/other tools side to make it work completely. Also - did gocode used to provide completions for files with build constraints? I tried it out with mdempsky/gocode, and I wasn't able to see any results either.

Thanks!

Rebecca

--
You received this message because you are subscribed to the Google Groups "golang-tools" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-tools...@googlegroups.com.
To post to this group, send email to golang...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/9b68154f-ab3e-4420-8ca9-5c41e5ca4f84%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Fatih Arslan]

Hi,

I struggled understanding on how to pass build tags but find a way (by reading the tests for go/package). I use the following in one of my projects:

tags := fmt.Sprintf("-tags=%s", "a list of build tags")

cfg := &packages.Config{

BuildFlags: []string{tags},

}

I think this could be an addition to the imaginary "go/packages/packagesutil" (or go/packages/pkgutils) package I suggested in our meeting (for things like creating fake tree, overlay, etc..)

We have already an easy way to pass buildtags via buildutils: https://godoc.org/golang.org/x/tools/go/buildutil#TagsFlag

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/CALKN5A%2BUi0_8YzEQh%2BF5JpU%2Bvgd0QhGn-JcArMtZO7KUVvsegw%40mail.gmail.com.

For more options, visit https://groups.google.com/d/optout.

--

Fatih Arslan

[Michael Matloob]

Yes, that's the correct way to pass tags in. One potential roadblock for packagesutil is that different build systems might have different ways to pass in flags.

The ideal is that we have libraries that provide passthroughs for the BuildFlags, and where possible, ask users to provide the build tags directly to the tool. Of course that won't work for every tool, especially those that need to set specific hardcoded tags.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/CAGHgn02SptD2MWKWSERLDG-omMipKtceDLgn7B466fW5p1pEzg%40mail.gmail.com.

[Nate Finch]

Does someone need to state the (hopefully) obvious?  An argument that is a slice of strings that correspond to the flags for a command line tool (that the consumer is not actually calling) makes for an unfriendly API.

I'm sure it makes the code for go/packagee simpler (if it's mostly calling out to the go command), but as is clear from this thread, it's not intuitive or discoverable. 

It would seem to be much preferable to offer strongly typed struct fields in the config object that have documentation in the godoc for the API itself and offer all the advantages of true go types rather than the restrictions present in string interpolation and relying on documentation not written with go/packages in mind. 

-Nate

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/CADVvUc0RyyE9MsKFX%3DgGm9GEMPLzjYFduehbxBWaT6OhNDoJMw%40mail.gmail.com.

[Paul Jolly]

> Does someone need to state the (hopefully) obvious? An argument that is a slice of strings that correspond to the flags for a command line tool (that the consumer is not actually calling) makes for an unfriendly API.

There's some confusion about the point I was trying to make; that
could well be down to my poor initial explanation.

The issue is not "how do I as a tool author pass build tags via
go/packages"; the docs are sufficiently clear to my mind
https://godoc.org/golang.org/x/tools/go/packages#Config. Indeed if
there some suggestion about refinement to the API for passing such
flags I think we should track that in an issue on Github/another
thread here.

Rather the issue is again one of context from a user's/user's editor's
perspective.

Because if we are going to "push back" on the user to supply build
tags, GOOS and GOARCH (which makes sense on some level) for every
gocode, godef etc request we need to think about how this manifests in
the editor. There have been a number of questions about how to get
started with WebAssembly in VSCode, Vim etc that highlight the problem
(where GOOS=js GOARCH=wasm is required). This same problem is going to
exist in an LSP world I suspect.

Roger Peppé has some good experience of this from the original days of
godef so please do chip in Roger!

> Also - did gocode used to provide completions for files with build constraints?

Rebecca - "old" gocode does not work with build constraints, no, but
godef works in all cases:

https://gist.github.com/myitcv/10b13ce65e6655bdadc68e3c1ba719d7#file-02-gopath-script-sh

To my mind all editor-based tools should behave consistently with
respect to build tags, GOOS and GOARCH. Which is what I'm trying to
get at here: we need to come up with a "solution" that works. That
solution might be:

* firming up on this idea of context from an editor/project
perspective. It's more complex than a "single setting per project";
sub directories (packages) might well have different constraints
* doing something in go/packages
* something else....

I suspect there is some overlap here with the question I had regarding
go generate, but we can park that for now.

Does that help to clarify?

Thanks,


Paul


Paul

[Michael Matloob]

I agree that our API is clunky. I don't like it either -- but I'm worried we'd add too much complexity and confusion to if these configuration parameters were specified in typed fields. I'd love to be proven wrong, and allow users to directly specify GOOS, GOARCH, and Tags fields on the Config.

The problems fall into these categories:

 redundancy with BuildFlags and Environment fields on the config

 number of and interaction between different options to the go tool

 alternative build systems

If we added those fields to the Config, I don't think it would be a good idea to keep the BuildFlags and Environment fields. They would be redundant with the GOOS, GOARCH, and Tags fields, adding complexity, and a precedence question. But without BuildFlags and Environment, we might be removing knobs tools need to specify the configuration. The go tool provides a number of other knobs to specify the configuration that users would might to pass through. These include the environment variables GOROOT, GOFLAGS, and GOMOD; and the flags -race, -msan, -gcflags -ldflags, and -mod. And others may be added in the future. Removing BuildFlags and Environment would make it difficult. On top of that, other build systems might have other knobs that determine which files are included or built. For instance, in Blaze the GOOS and GOARCH don't map neatly to the CPU and OS settings.

So to me it seems we have three undesirable options  (1) have both the direct GOOS, GOARCH, and Tags fields, and BuildFlags and Environment, which would be even more confusing when both are provided (rare though that may be), (2) drop some knobs that users may want in the future and remove BuildFlags and Environment, or (3) add all the possible configuration fields for all build systems to Config (or a struct pointed to from it).

Does this sound reasonable? Again, I'd love to go with something simpler, so I'd love to know if I'm wrong here.

Thanks!

Michael

[Bryan C. Mills]

There is a fourth option; I don't know whether it's equally undesirable.

(4) Add helper functions or methods to construct or set BuildFlags and/or Env for common Go command parameters, with the actual behavior ideally determined by the specific driver.

For example:

func (*Config) SetGOOS(string)

func (*Config) SetGOARCH(string)

func (*Config) SetTags([]string)

func (*Config) SetRace(bool)

func (*Config) SetMSan(bool)

--

You received this message because you are subscribed to the Google Groups "golang-tools" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-tools...@googlegroups.com.
To post to this group, send email to golang...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/CADVvUc11OvyauZnKA9sNZGr%2B2VoeXpT2yFZmHW_g_a%3DY4%3DOnnA%40mail.gmail.com.

[roger peppe]

Given all those considerations, the current straightforward if obscure
approach seems OKish to me.

But it does raise the question in my mind as to whether the
go/packages API should be seen as a transparent layer on top of the go
tool, or as a general way to specify the inspection of Go code without
regard to implementation tool.

> The go tool provides a number of other knobs to specify the configuration that users would might to pass through. These include the environment variables GOROOT, GOFLAGS, and GOMOD; and the flags -race, -msan, -gcflags -ldflags, and -mod.

I don't really understand how most of those flags would be useful to
pass to go/packages. Most of them relate to runtime behaviour, not to
compile-time information, and AFAICS the go/packages API does not
provide any runtime behaviour.

For example, why would it be useful to pass the -race flag other than
to set the race-detector build tag? Why -ldflags when we're not doing
any linking?

ISTM that the old go/build behaviour (build.Config.BuildFlags) was OK
here. I suspect that hardly anyone used build.Config.BuildFlags, and
even though there might technically be some redundancy there, if you
mess with BuildFlags, you need to know what you're doing.

FWIW in a fork of go/build that I did for my godeps tool
(https://godoc.org/github.com/rogpeppe/godeps/build), I found the
go/build approach insufficient, as I specifically needed to include
dependencies for all architectures.

I added a MatchTag field to Context:

type Context {
// ... other fields

// MatchTag, if specified, is called to determine whether a given
// tag should be considered to match for the purposes of importing.
// If neg is false, it should report whether the tag matches,
// otherwise it should report whether the tag does not match.
// A MatchTag that always returns true is equivalent to
// specifying UseAllFiles=true.
//
// If MatchTag is nil, Context.DefaultMatchTag will be called to
// to decide if a tag matches.
MatchTag func(tag string, neg bool) bool

// The build and release tags specify build constraints
// that should be considered satisfied when processing +build lines.
// Clients creating a new context may customize BuildTags, which
// defaults to empty, but it is usually an error to customize
ReleaseTags,
// which defaults to the list of Go releases the current
release is compatible with.
// In addition to the BuildTags and ReleaseTags, build constraints
// consider the values of GOARCH and GOOS as satisfied tags.
//
// Both these fields are considered only by DefaultMatchTag and
// will be ignored if MatchTag is non-nil.
BuildTags []string
ReleaseTags []string
}

Then the godeps tool does:

ctx.MatchTag = func(tag string, neg bool) bool {
if build.KnownOS(tag) || build.KnownArch(tag) {
return true
}
// Fall back to default settings for all other tags.
return ctx.DefaultMatchTag(tag) != neg
}

to select all OS's and architectures, but exclude other tags such as
"js" and "ignore", which was the best general heuristic I came up with
for generating plausible dependency graphs.

I suspect that many possible clients of go/packages might want
something similar. It often doesn't matter whether a program compiles,
just that all the source files that the tool cares about are held in
the tree.

The requirements are quite different for a use such as godef, which
doesn't work if unless consider one set of valid tags at a time (it
would actually be really nice to provide the set of possible results
when different OSs, architecture or other tags apply, but that might
be computationally quite expensive).




On 25 September 2018 at 20:23, 'Bryan C. Mills' via golang-tools

> https://groups.google.com/d/msgid/golang-tools/CAKWVi_S7R7-TK%2BoPsRwqY-_sCZXaq1nEHe6NQVyRm2pwibi8Wg%40mail.gmail.com.

[Paul Jolly]

Thanks for chipping in, Roger.

> The requirements are quite different for a use such as godef, which
> doesn't work if unless consider one set of valid tags at a time (it
> would actually be really nice to provide the set of possible results
> when different OSs, architecture or other tags apply, but that might
> be computationally quite expensive).

The case of godef, gocode etc, i.e. where one set of valid tags is
required, is the focus of my question.

As we followed up offline, I think the conclusion I've drawn thus far
is that editors will need to support, at a minimum, a user setting
GOOS, GOARCH and build tags.

In future we might look to establish module, package or file level
"preferences" for those values, but discussion on that is premature.

I'll add an item to the agenda for our next catchup.


Paul

[Rebecca Stambler]

An aside - I tried to support build constraints in gocode, but haven't merged it into master since I haven't tested it too much. https://github.com/stamblerre/gocode/tree/build-constraints if you want to try it out.

[Michael Matloob]

To reply to a couple of Roger's points:

I think it's impossible for us to support arbitrary build tags combinations and specifically loading files that match all build tags, and maintain compatibility with other build systems. Not having it means there are some tools we can't build with go/packages, and I'm sad that go/packages doesn't support it, but it's incompatible with building tools for other build systems.

Many of those flags won't be useful to pass to go/packages, and I don't think there's much outside of GOROOT, GOPATH, and tags that most tools need, and if we're all convinced that those are the only fields that matter and we don't need to supply arbitrary flags and environment variables, I'd be for us to replacy BuildFlags and Environment with GOROOT,GOPATH,Tags. But I think there are enough tools that might want more, and there's the possibility that new flags and environment variables will be added, such as those for modules. And that means that if we go the route listing each parameter, we could end up either not supporting functionality added in the future or end up with a long list of parameters in the Config.

--

You received this message because you are subscribed to the Google Groups "golang-tools" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-tools...@googlegroups.com.
To post to this group, send email to golang...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-tools/CALKN5ALfH0Yx_m_wtVF%2BHx474j2VgO-Er-BUbbNCevSfJ9H%3DJw%40mail.gmail.com.

[roger peppe]

On Thu, 27 Sep 2018, 6:04 pm Michael Matloob, <mat...@golang.org> wrote:

To reply to a couple of Roger's points:

I think it's impossible for us to support arbitrary build tags combinations and specifically loading files that match all build tags, and maintain compatibility with other build systems. Not having it means there are some tools we can't build with go/packages, and I'm sad that go/packages doesn't support it, but it's incompatible with building tools for other build systems.

Does that mean that it is not possible to build, for example, a "gogrep" tool that searches all dependencies of a package for a given string?

(that's a tool that I already have, and I'd miss it).

Many of those flags won't be useful to pass to go/packages, and I don't think there's much outside of GOROOT, GOPATH, and tags that most tools need, and if we're all convinced that those are the only fields that matter and we don't need to supply arbitrary flags and environment variables, I'd be for us to replacy BuildFlags and Environment with GOROOT,GOPATH,Tags.

But I think there are enough tools that might want more, and there's the possibility that new flags and environment variables will be added, such as those for modules. And that means that if we go the route listing each parameter, we could end up either not supporting functionality added in the future or end up with a long list of parameters in the Config.

I'd love some concrete examples of what you might be thinking of here. If we need to add BuildFlags in the future, that's always possible, but we can't take it away if we add it now.