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

x/tools/go/packages: support for loading files/syntax irrespective of build tags #28121

Open
myitcv opened this issue Oct 10, 2018 · 6 comments
Labels
modules Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Oct 10, 2018

What version of Go are you using (go version)?

go version go1.11.1 linux/amd64
go/packages commit 29f11e2b93f4d66f7c335bd7b2892836d4944f5c

Does this issue reproduce with the latest release?

Yes, per above

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build137760326=/tmp/go-build -gno-record-gcc-switches"

Picking up on the discussion from the 2018-10-09 golang-tools catch up

In the pre-modules world, go/build and go/parser could be used in combination to get file and syntax information for packages and their dependencies irrespective of GOOS, GOARCH and build tags via the UseAllFiles go/build.Context field.

This use case is important for tools like govers and gogrep.

go/packages is driven by Config which includes GOOS, GOARCH and build tags and hence its response is specific to the provided config. go/packages cannot, therefore, be used in these use cases.

This issue is a placeholder to continue discussion about how best to support loading of files, imports and syntax in a GOOS, GOARCH and build tags-agnostic way.

cc @ianthehat @matloob @alandonovan @bcmills @rogpeppe @mvdan

@myitcv myitcv added the modules label Oct 10, 2018
@gopherbot gopherbot added this to the Unreleased milestone Oct 10, 2018
@myitcv
Copy link
Member Author

myitcv commented Oct 10, 2018

One option discussed so far has been to make multiple calls to go/packages.Load for various GOOS, GOARCH and build tag combinations. This could conceivably be done concurrently with the results being added to the same *token.FileSet.

As was noted on yesterday's call, returning type information in this situation doesn't make sense.

However the go/packages.LoadMode currently specifies LoadSyntax as a "higher numbered mode" than LoadTypes. So it seems unclear how to get syntax for packages without types.

@matloob - perhaps you could comment on this particular question?

@mvdan
Copy link
Member

mvdan commented Oct 10, 2018

If I recall our meeting correctly, what @ianthehat was suggesting is a separate package that works on directories, not buildable packages. I presume that it would look more like a mix of the old go/build and go/parser.ParseDir.

That sounds fine to me, and I presume it would require no changes to go/packages.

@bcmills
Copy link
Contributor

bcmills commented Oct 10, 2018

They're not strictly separate, unfortunately.

For example, in a tool like goforward, we ideally want the source files for all architectures in the move phase, but build-system-agnostic source files in the replace and forward phases. It would be pretty irritating to have to use separate packages (and separate types!) for those phases, since they're doing a single multi-step transformation on the package.

@matloob
Copy link
Contributor

matloob commented Oct 10, 2018

Responding to @myitcv's question, it's not just the type information that doesn't make sense: adding the same file twice to a fileset will create two entries for that file, with different 'pos' values. So I don't think changing or adding a new Mode will help too much with this.

I think the best we can do is to call Load in LoadFiles mode and call parsefile ourselves. Calling parsefile is one of the simpler things go/packages does.

We suggested that this is something that could be done with go/packages, but I think go/packages isn't a good fit for this problem. Like @mvdan mentioned, a separate packages that works on directories might be the better solution. And because it wouldn't have to support other buildsystems, it wouldn't be hamstrung by the limitations go/packages faces.

@myitcv
Copy link
Member Author

myitcv commented Nov 14, 2018

Related to #27900 in as much as go mod why operates independent of build tags (or put another way, for all possible build combinations) and what's being discussed here is akin to go list -nobuild (or whatever name such a flag would have).

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Nov 11, 2021

This feature probably requires #42504.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants