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

proposal: x/tools/go/packages: add Goroot to Packages struct #43499

Closed
jpap opened this issue Jan 4, 2021 · 6 comments
Closed

proposal: x/tools/go/packages: add Goroot to Packages struct #43499

jpap opened this issue Jan 4, 2021 · 6 comments

Comments

@jpap
Copy link
Contributor

jpap commented Jan 4, 2021

Please export Goroot in Package. It is available in go list -f '{{ .Goroot }}'.

The only way to programmatically get it right now is to use a workaround to get the package dir, then compare to runtime.GOROOT().

@gopherbot gopherbot added this to the Proposal milestone Jan 4, 2021
@mvdan
Copy link
Member

mvdan commented Jan 4, 2021

Why not just rely on the rule described in #32819? It's technically not documented yet, but the rule already exists in practice and can be relied upon. Two of my tools use it already, and it's cheaper than having to query the packages.

@jpap
Copy link
Contributor Author

jpap commented Jan 4, 2021

Thanks for highlighting the undocumented convention. It unfortunately doesn't catch the case where "a module name without a dot happens to build today" (quoted from your issue).

Either way, it's cleaner to rely on a bool on a *Package when you already have the instance, or need it for other purposes as well.

@mvdan
Copy link
Member

mvdan commented Jan 4, 2021

If you do absolutely need such low level details, I'd suggest to try just using go list -e -json args... directly. go/packages is an abstraction layer that supports other build systems by design, so it can't expose cmd/go details which are too specific. See #38445, for example, which are fields that exist in one way or another in most build systems.

@jpap
Copy link
Contributor Author

jpap commented Jan 4, 2021

If you do absolutely need such low level details, I'd suggest to try just using go list -e -json args... directly.

I indeed reach in to go list for some things -- it's rather unfortunate that everything exported in that JSON payload isn't made available in x/tools/go/packages... that is exactly what motivates issues like #38445 (which was linked to in the top-post), and now this one.

So either the developer needs to find a workaround to x/tools/go/packages, or needs to bypass it while reaching into go list with JSON processing. It would instead be great if they were each equivalent. Nobody wants to have to rewrite x/tools/go/packages (or maintain a fork) to add functionality from go list into a high level Go-typed abstraction. It sounds like you made a similar argument in #38234, but unfortunately 2/3 of your requests still remain open after quite some time.

go/packages is an abstraction layer that supports other build systems by design, so it can't expose cmd/go details which are too specific.

I'm not sure what you mean by this: is it designed to support some specific build systems (which ones? Bazel? Any others?) or that it is designed to support third-party build systems in general? What details are "too specific" vs. not? Why would you want to shut down this proposal vs. the three resulting from your issue #38234?

I am using x/tools/go/packages for a build tool that extracts certain resource files from a program's (recursive) import dependencies; for the purposes of this issue, skipping GoRoot packages because they never contain such files and thus avoids unnecessary directory scans that can improve build times. The same tool can benefit from #38445 which is why I commented there in November last year.

@mvdan
Copy link
Member

mvdan commented Jan 5, 2021

is it designed to support some specific build systems (which ones? Bazel? Any others?)

Yes, Bazel is the main other one.

Why would you want to shut down this proposal vs. the three resulting from your issue

They are similar, and I'm not shutting down anything. I'm just saying that if you want a lot of the fields from go list -json, your best bet right now is to just use it directly. go/packages will never expose all the details that go list does.

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jan 6, 2021
@matloob
Copy link
Contributor

matloob commented Jan 13, 2021

Hi, I'm the maintainer of go/packages. We don't want to shut down the proposal, but go/packages was originally designed to be build system agnostic (as in supporting other build systems like Bazel). GOROOT is a concept specific to the go command build system, so adding GOROOT to Packages would be breaking the abstraction. It's arguable that being build system agnostic was the wrong decision to make, (and I've come around to that position) but I think we'd need to think harder to have a strategy about how to reorient go/packages around only one build system if we wanted to do things like this.

For now, it's much easier to use go list to get this information. This is partially why #38445 is still open

@matloob matloob closed this as completed Jan 14, 2021
@ianlancetaylor ianlancetaylor removed this from Incoming in Proposals (old) Mar 22, 2021
@golang golang locked and limited conversation to collaborators Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants