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

cmd/go: go list filtering is too coarse #30592

Open
nim-nim opened this issue Mar 5, 2019 · 9 comments
Open

cmd/go: go list filtering is too coarse #30592

nim-nim opened this issue Mar 5, 2019 · 9 comments
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@nim-nim
Copy link

nim-nim commented Mar 5, 2019

$ go version
go version go1.11 linux/amd64

I need go list ./... to print all the files involved in testing or building on the linux GOOS, regardless of the target architecture or project-specific constrains such as selinux or apparmor.

Unfortunately IgnoredGoFiles does not cut it because it also includes files for other systems, and windows for example is so different from linux that it pulls it all lots of things I have no use for and can not satisfy in my environment.

So go list ./... needs a way to set some form of context, different from everything or current system

go/packages does not cut it either because as far as I can see its scope is package-specific when I need a module scope (./...)

@mvdan
Copy link
Member

mvdan commented Mar 5, 2019

This seems like a very specific need; why not build a simple Go program to perform this with https://golang.org/pkg/go/build/?

Also, as explained in #30504, it's not clear that it would be possible for go list to ignore certain build constraints when loading packages. It seems like you want to load a directory of files, and not a Go package.

@ianlancetaylor
Copy link
Contributor

I agree with @mvdan that this doesn't seem to make sense for the go tool.

If we did add something to the go tool, what specifically do you suggest?

@agnivade agnivade added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 7, 2019
@bcmills bcmills changed the title go list filtering is too coarse cmd/go: go list filtering is too coarse Mar 7, 2019
@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Mar 7, 2019
@nim-nim
Copy link
Author

nim-nim commented Mar 12, 2019

This seems like a very specific need; why not build a simple Go program to perform this with https://golang.org/pkg/go/build/?

The problem with such an approach is that the sanity checks you code end up diverging from the sanity checks go upstream builds in its own tools, and this divergence is the source of tricky bugs.

Also, as explained in #30504, it's not clear that it would be possible for go list to ignore certain build constraints when loading packages. It seems like you want to load a directory of files, and not a Go package.

I want to load a Go module because Go is moving to modules. The sources of a module are a directory tree not a single package.

@mvdan
Copy link
Member

mvdan commented Mar 12, 2019

So you want to list all source files for a Go module? What does that have to do with GOOS?

@nim-nim
Copy link
Author

nim-nim commented Mar 12, 2019

@bcmills

If we did add something to the go tool, what specifically do you suggest?

go list ./... --os=linux --allarchs --alltags

or

go list ./... --os=linux --arch=x86_64 --tag=selinux --tag=cake

(and I know the difference between tags and arches is quite weak in Go right now but for an integrator they are not the same thing at all, one is a technical target, the other a functional one)

@nim-nim
Copy link
Author

nim-nim commented Mar 12, 2019

So you want to list all source files for a Go module? What does that have to do with GOOS?

Because we integrate for a specific OS, and the support files for other OSes cost us all the other OS-specific imports in those files, when we have no use for them.

@mvdan
Copy link
Member

mvdan commented Mar 12, 2019

I personally think this is unlikely to happen in go list. Filtering by build tags (like GOOS) happens at a package level, not at a module level. Similarly, a module is a collection of packages, not really a collection of Go files. See https://github.com/golang/go/wiki/Modules#modules.

Also note that the -os flag is unnecessary; you should be able to do GOOS=linux go list -json . to see the Go files that would form a Linux build of a package. I realise that's not what you want, but at least the -os flag is unnecessary.

Finally, it seems to me like GOOS=linux and -alltags would be contradictory; the operating system selection is done via build tags too, so you'd effectively be ignoring some tags, but not others. I don't think any part of the Go tool does this at the moment.

I still think this should be a separate tool; for one, I've never heard of anyone else with these specific needs. If you think this is a very common need, it would be helpful to give some proof or examples.

The problem with such an approach is that the sanity checks you code end up diverging from the sanity checks go upstream builds in its own tools, and this divergence is the source of tricky bugs.

Your tool could be built on top of go list, so I disagree. For example, you could go list -m -json to get the current module and its list of packages, and then in your code, iterate over the package directories and look at the files on disk. You should be able to do that part easily with go/build and go/parser.

@nim-nim
Copy link
Author

nim-nim commented Mar 12, 2019

I personally think this is unlikely to happen in go list. Filtering by build tags (like GOOS) happens at a package level, not at a module level. Similarly, a module is a collection of packages, not really a collection of Go files. See https://github.com/golang/go/wiki/Modules#modules.

That's just adding an indirection to get to the files part.

Also note that the -os flag is unnecessary; you should be able to do GOOS=linux go list -json . to see the Go files that would form a Linux build of a package. I realise that's not what you want, but at least the -os flag is unnecessary.

That is nice to know thanks

Finally, it seems to me like GOOS=linux and -alltags would be contradictory; the operating system selection is done via build tags too, so you'd effectively be ignoring some tags, but not others. I don't think any part of the Go tool does this at the moment.

That's why I wrote “I know the difference between tags and arches is quite weak in Go right now but for an integrator they are not the same thing at all, one is a technical target, the other a functional one“

You can change your functional build target. You can not change the arches you build for (not if you want the result to run on the target hardware).

I still think this should be a separate tool; for one, I've never heard of anyone else with these specific needs. If you think this is a very common need, it would be helpful to give some proof or examples.

It's a Linux distribution need. We distribute for Linux, thus, we care only about a single system. We build lots of apps, and arches, thus we care about all build tags and arches. We reprocess upstream source files, because if they had no need of reprocessing, our users would just get them directly, without using distribution packages.

Reprocessing implies lots of filtering and selecting steps.

And, it's quite painful to report all that to Go upstream, because the generic answer is "why are you doing all those distributor things, we find no need for them". We do those things because we are a distributor and they constitute the distributor raison d'être. The go project does not do that because it's not a distributor (and violently does not want to be, hence the emphasis in shipping everything unchanged).

@agnivade
Copy link
Contributor

agnivade commented Apr 8, 2019

@bcmills for further comments.

@ALTree ALTree added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jun 18, 2021
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

7 participants