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/dist: show which GOOS/GOARCH combinations are first class ports #38874

Closed
mvdan opened this issue May 5, 2020 · 21 comments
Closed

cmd/dist: show which GOOS/GOARCH combinations are first class ports #38874

mvdan opened this issue May 5, 2020 · 21 comments
Labels
FeatureRequest FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented May 5, 2020

go tool dist list, with or without the -json flag, is very useful to list all supported ports. It was proposed and done in #12270.

However, there's no way to query the Go tool which of those combinations are first class ports. The current list is:

linux/amd64
linux/386
linux/arm
linux/arm64
darwin/amd64
windows/amd64
windows/386

Why do I want this list? These ports have binary releases available on golang.org, they are better tested and supported, and they are overall much more common platforms to run Go on. For example, if I'm publishing prebuilt releases for a Go program of mine, I probably want to use Go's list of first-class ports as a starting point, and only add extras if I know about a specific group of users of the prorgram.

Copy-pasting the list from the Wiki is an OK immediate workaround, but it can quickly get out of date. For example, 1.15 adds linux/arm64 to the list, so having copied the list just a few months ago would already be out of date today. The list on the wiki also reflects master; if I look at it now, I might get the impression that linux/arm64 was a first class port at the time that Go 1.14 came out, but that's not the case. go1.14 tool dist list -json would be able to tell me that decisively.

The command already has a -json flag to produce a JSON list, so it should be easy to add another field. For example:

$ go tool dist list -json
[
	{
		"GOOS": "aix",
		"GOARCH": "ppc64",
		"CgoSupported": true,
		"FirstClass": false
	},
[...]

/cc @bradfitz @minux @golang/osp-team

@mvdan mvdan added GoCommand cmd/go NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository. labels May 5, 2020
@dmitshur
Copy link
Contributor

dmitshur commented May 6, 2020

Thanks for the report. I have thoughts on two aspects of this.

One is making the first-class port information programmatically accessible. I think this is reasonable to consider doing. This information is already public, so it is just a matter of making it accessible to computers in addition to humans. The Go project itself would benefit from this, as it could re-use the canonical list of first-class ports in all the places where it's needed. Right now there is some inconsistency, as reported in issue #27689.

Two is about how to version this information. Right now there is only a "master" snapshot, on the wiki, plus its edit history as a rough indicator of what happened over time. Suppose this feature was implemented since Go 1.10 (released on 2018/02/16), what would it mean that go1.10 tool dist list -json prints "FirstClass": false for linux/arm64 and "FirstClass": true for darwin/386? I understand it'd be a rough proxy for "at the time that major version of Go was released, this was the state of first-class ports". What about what go1.10.8 tool dist list -json prints, which was released 2019/01/23? Based on how I understand it now, bundling the historical information into cmd/go (whose releases might not align with policy changes) this way doesn't seem like a good approach.

In the end, I think #27689 needs to get past NeedsDecision state before we can make progress here.

@dmitshur
Copy link
Contributor

dmitshur commented May 6, 2020

Suppose this feature was implemented since Go 1.10 (released on 2018/02/16), what would it mean that go1.10 tool dist list -json prints "FirstClass": false for linux/arm64 and "FirstClass": true for darwin/386? I understand it'd be a rough proxy for "at the time that major version of Go was released, this was the state of first-class ports". What about what go1.10.8 tool dist list -json prints, which was released 2019/01/23?

I think I understand it better now. You're looking to make it so that a first-class port is never added or removed mid-release, but rather aligned with major release boundaries, and to make it possible to query that (via cmd/go or elsewhere).

For example, if a port is added to being considered first-class, it should be possible to (easily) answer "this port became first-class as of Go 1.N", rather than having to implicitly compute that from release dates of major Go versions and the first-class addition date.

Is my understanding right?

@mvdan
Copy link
Member Author

mvdan commented May 7, 2020

Correct. When a major release is getting ready, like 1.15, we should make sure that go tool dist list contains up-to-date information about the first class ports. Those should match with what first class port rules are going to be used for the 1.15 release.

Minor (aka bugfix) releases like 1.15.1 should never change this information, because first class ports are only added or removed in major versions like 1.15 and 1.16, as far as I understand. It would be weird for 1.15.1 to add another first class port, or to remove one.

@toothrot
Copy link
Contributor

toothrot commented May 7, 2020

If you're using this data for creating new releases, is it ok for you to query "what is a first-class port right now for Go" vs "what is a first-class port for the latest release of Go"?

Somewhat related, as part of work to move the build dashboard into the coordinator (#34744), and have the "first-class ports" checkbox do the right thing on the UI, I hard-coded this field:
https://go.googlesource.com/build/+/refs/heads/master/cmd/coordinator/internal/dashboard/handler.go#29.

Having that somewhere query-able would be useful for the coordinator as well, as we want this information in a couple tools.

Another thought: Go releases binaries for non-first-class ports as well (freebsd-386/amd64, linux-ppc64le/s390x). I don't think we have a way of knowing for certain the most common platforms for running Go on, as you mentioned, but I have a guess.

@mvdan
Copy link
Member Author

mvdan commented May 7, 2020

If you're using this data for creating new releases, is it ok for you to query "what is a first-class port right now for Go" vs "what is a first-class port for the latest release of Go"?

No, because I build my releases with a stable release of Go, not master. For example, the upcoming Go 1.15 adds linux/arm64 as a first-class port, but I'll build releases with Go 1.14.x, where linux/arm64 was not a first class port (i.e. at the time of 1.14's release).

Another thought: Go releases binaries for non-first-class ports as well

I think that's a requirement for first class ports, but I guess not all downloads have to be first class ports. Maybe the wiki could be clarified.

I don't think we have a way of knowing for certain the most common platforms for running Go on, as you mentioned, but I have a guess.

I think this is up to each use case. For my use case, "what are the platforms with first-class support in Go" is an OK starting point.

@mvdan
Copy link
Member Author

mvdan commented Oct 30, 2020

Would a patch for this be welcome? I ran into another use-case where this would have been helpful, and having this in the upcoming 1.16 would be nice.

I get that the freeze is here, but I imagine this sort of change might be okay early in the freeze given that go tool dist is not used directly by the vast majority of Go users.

@mvdan
Copy link
Member Author

mvdan commented Oct 30, 2020

For context, here's the use case: https://github.com/mvdan/dotfiles/blob/47c16f7b56bf8e441e9c760f7664e5a89244a39d/.bin/go-cross. For now, I'm just hard-coding the list, and I have to trust that I'll remember to check https://github.com/golang/go/wiki/PortingPolicy#first-class-ports for updates every release.

@dmitshur
Copy link
Contributor

dmitshur commented Oct 30, 2020

I'm really not sure that cmd/go is the right place to put this information. If we add it there right away, it'll be hard to change it.

I'm okay with placing this information in x/build (which is v0 and can be modified anytime without time pressure of the freeze) though so it can be accessed programmatically, and not have to download many historic cmd/go binaries to get info about older releases. I suggest we start there, and consider promoting to cmd/go later on if we're happy with the experience in x/build and still think there's a need to also make it available from cmd/go.

For x/build, the internal table can perhaps be, inspired by golang.org/x/website/internal/history and golang.org/x/build/cmd/release, something like:

map[string][]string{
	">= go1.15": {
		"linux/amd64",
		"linux/386",
		"linux/arm",
		"linux/arm64",
		"darwin/amd64",
		"windows/amd64",
		"windows/386",
	},

	"< go1.15": {
		"linux/amd64",
		"linux/386",
		"linux/arm",
		"darwin/amd64",
		"windows/amd64",
		"windows/386",
	},

	// ...
}

With some API to get the ports for a specified major Go version.

@mvdan
Copy link
Member Author

mvdan commented Oct 30, 2020

If we add it there right away, it'll be hard to change it.

I don't follow. The whole point is that, just like the rest of the information given by go tool dist -json, it will change as the list of ports in each Go version change.

With some API to get the ports for a specified major Go version.

For what it's worth, this doesn't seem useful to me, and it also adds Go as a dependency to extract the information. go tool dist -json exists for a reason: to query the detailed list of ports from an installed Go tree.

Such a pure Go API could be useful within x/build perhaps, but it doesn't fix the go tool dist use case. Perhaps file a separate issue for it.

@dmitshur
Copy link
Contributor

If we add it there right away, it'll be hard to change it.

I meant the API. If we add a field to cmd/go now, we can't easily change or remove it in the future. So it needs more thought before we commit to it.

I'm going to set the milestone to Go 1.17, we can revisit this issue then. I don't think this is important for Go 1.16.

@dmitshur dmitshur added this to the Go1.17 milestone Oct 30, 2020
@mvdan
Copy link
Member Author

mvdan commented Oct 30, 2020

Sure. I really hope a decision can be made for 1.17, though, because I honestly don't see why this request needs years of paused thinking.

@dmitshur dmitshur added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Oct 30, 2020
@gopherbot
Copy link
Contributor

This issue is currently labeled as early-in-cycle for Go 1.17.
That time is now, so a friendly reminder to look at it again.

@mvdan
Copy link
Member Author

mvdan commented Mar 8, 2021

Friendly ping. It's not clear to me who can make a decision here or what the process is, but there's just eight weeks left in this merge window, and it would be a shame to miss another release.

@ianlancetaylor
Copy link
Member

A few comments above mention cmd/go but I'm not clear on why. Currently all of the information for go tool dist list lives in cmd/dist. Specifically, it all comes from the table cgoEnabled in cmd/dist/build.go. So I think that we are talking about here is adding another, smaller, table that lists the first class ports, and printing that information in the cmdlist function, presumably only when it is run with the -json flag. That seems pretty straightforward and considering how slowly the information changes I think it would be easy enough to keep up to date manually, with a comment on the wiki page to remind us.

@dmitshur Is there something I am missing? Thanks.

@mvdan
Copy link
Member Author

mvdan commented Apr 27, 2021

I agree with @ianlancetaylor. This is exposed via go tool dist, but it's still just fairly simple changes to cmd/dist.

I really hope we can reach a decision here for 1.17. This issue was marked early-in-cycle, but I don't think it needs to be postponed once again.

@dmitshur
Copy link
Contributor

@ianlancetaylor I think your understanding of how this could be implemented is accurate. I agree referring to cmd/dist is more precise; I was mentioning cmd/go only because I expected it's more likely dist would be accessed via go tool dist rather than directly.

Back when this issue was originally filed, we had an unresolved issue of not knowing which of two lists was authoritative. That's been resolved (the list on the PortingPolicy wiki page is the authoritative one). By now we've also started using the proposal process for making any changes to the list of first class port (for recent examples, see #35593 and #43814), so I think also updating a table in cmd/dist isn't much extra work. (As @toothrot mentioned, we need to maintain a copy of this information in x/build for dashboard needs, but that's not a big deal.)

Comment #38874 (comment) made a suggestion that this information should not change across minor releases, only major ones. If that suggestion is used, then proposals to add or remove first-class ports should begin to be tied to a major release milestone.

@dmitshur dmitshur removed the early-in-cycle A change that should be done early in the 3 month dev cycle. label Apr 28, 2021
@dmitshur
Copy link
Contributor

As another data point, this might be useful for issue #14892, if we'd like to arrange that all first-class ports (but not necessarily all ports) are automatically specified as contexts for the purpose of the API check.

@mvdan
Copy link
Member Author

mvdan commented May 4, 2021

Can we remove the NeedsDecision label, then? It seems clear that we want to do this, and there don't seem to be any blockers.

Comment #38874 (comment) made a suggestion that this information should not change across minor releases, only major ones. If that suggestion is used, then proposals to add or remove first-class ports should begin to be tied to a major release milestone.

I always assumed that to be the case. It would be a problem for a 1.16.2 release to remove a first-class port, for example. It would also be weird for a first-class port to be added. I don't think either has happened in the past.

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label May 4, 2021
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label May 4, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/320069 mentions this issue: cmd/dist: display first class port status in json output

@iwdgo
Copy link
Contributor

iwdgo commented May 14, 2021

Format of the proposed CL is a simple extension of the existing one with a table of first class ports.
Output of one entry of go tool dist list -json becomes:

{
        "GOOS": "windows",
        "GOARCH": "amd64",
        "CgoSupported": true,
        "FirstClass": true
},

@mvdan
Copy link
Member Author

mvdan commented May 14, 2021

Looks fine to me, though I'm not the one who can review or approve this :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants