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: suggest sets of build constraints that cover all files in a package #26733

Open
jimmyfrasche opened this issue Aug 1, 2018 · 11 comments

Comments

@jimmyfrasche
Copy link
Member

commented Aug 1, 2018

https://golang.org/cl/12703044 added AllTags to *go/build.Package so that "[t]ools scanning packages can use this to decide how many variants there are and what they are."

Since it's often necessary to invoke go list rather than use go/build directly, I think the same reasoning applies for adding it to go list.

@jimmyfrasche jimmyfrasche added this to the Go1.12 milestone Aug 1, 2018

@jimmyfrasche

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2018

@mvdan

This comment has been minimized.

Copy link
Member

commented Aug 2, 2018

Also cc @alandonovan for go/packages.

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 2, 2018

How does AllTags actually help with deciding the number and set of variants?

In theory, the number of variants is exponential with the number of tags, but in practice the number of combinations found in build constraints source files is much smaller than the powerset of tags.

@jimmyfrasche

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2018

If you have a directory containing go files and you want to find some set of build tags such that loading the directory with each member of that set in turn leads to every go file in the directory being included at least once, you need to know what tags can affect selection.

If you go through every GOOS ⨯ GOARCH ⨯ release tag ⨯ cgo/!cgo ⨯ … ⨯ etc. you're looking at (way!) too many combinations and most of them will resolve to the same list of included files and you're ignoring user-defined tags and things like a GOOS added after you've compiled the tool doing the search.

I'm not entirely sure yet that AllTags is sufficient to perform such a search (I need to do some sketching/prototyping) but I do believe that it is necessary.

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 2, 2018

We shouldn't have the go command build an index for a problem that it doesn't need to solve, so we're probably scanning file contents either way.

Probably the right solution for tools is to obtain the complete list of source files from go list, and then have some library that can take that list of files, mash them through the parser (in PackageClauseOnly mode?), and extract the unique sets of build constraints. I doubt that the build-constraint syntax will change enough over time for version skew to be much of a problem.

@jimmyfrasche

This comment has been minimized.

Copy link
Member Author

commented Aug 2, 2018

That would work, too, I imagine.

I am not super familiar with that part of the codebase, but, based on my find/grep/go-to-definition spelunking, it looks like cmd/go/internal/load already computes AllTags, because go/build does. It just doesn't get copied into the struct that go list uses for output.

@bcmills

This comment has been minimized.

Copy link
Member

commented Aug 2, 2018

Even if it already computes that data, I would argue that we shouldn't expose it unless we think tools will use it appropriately. The cmd/go documentation is already pretty verbose, and I don't want to spend our users' attention on things that have better alternatives anyway.

@bcmills bcmills modified the milestones: Go1.12, Go1.13 Nov 14, 2018

@bcmills

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Probably the right solution for tools is […] some library that can take that list of files […] and extract the unique sets of build constraints.

@ianthehat, a use-case for your team to consider. 🙂

@bcmills bcmills changed the title cmd/go: include AllTags in go list x/tools: suggest sets of build constraints that cover all files in a package May 9, 2019

@bcmills bcmills removed the GoCommand label May 9, 2019

@bcmills bcmills modified the milestones: Go1.13, Unplanned May 9, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented May 9, 2019

(Also CC @jayconrod.)

@ianthehat

This comment has been minimized.

Copy link

commented May 9, 2019

It is surprisingly hard to fit into the model for go/packages
The library is package focused, not directory focused. This means that we have nowhere to put files that are not part of a package (which includes any file excluded by build tags) just because they were in the same directory as some files that were.

@bcmills

This comment has been minimized.

Copy link
Member

commented May 9, 2019

Yeah, I would expect this functionality to go in some package other than go/packages. I agree that it's not a good fit there.

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