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 · 5 comments

Comments

Projects
None yet
5 participants
@myitcv
Copy link
Member

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

This comment has been minimized.

Copy link
Member Author

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.