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: cmd/go: Use git describe output as canonical version string #21209

Closed
paranoiacblack opened this issue Jul 28, 2017 · 10 comments
Closed

Comments

@paranoiacblack
Copy link
Contributor

paranoiacblack commented Jul 28, 2017

Background

This is closely related to Issue #21207 where @heschik notes that

there is a clear need for a structured way of determining at least the major/minor Go version in use, even for devel builds.

Currently, the zversion.go file and the output of go version are generated by go/build.findgoversion. This function inspects the underlying git repo and parses the output of git rev-parse to discover the canonical branch name, optionally stripping off release-branch prefixes and adding hash and date information for development versions that do not correspond to a particular tagged release.

This generates version strings of the following form:

  • go1.9.1
  • go1.9rc1 (from parsing release-branch-go1.9.1rc1)
  • devel +835dfef939 Wed Jul 26 13:29:59 2017 +0000

These strings might be parsed by external tooling that needs to optionally rely on certain Go features to exist. For example, from #21207:

  • Gogland needs to know what language features to enable, such as vendoring and type aliases.
  • Delve needs to know details of the runtime, such as where to find the G pointer in a thread's registers, as well as what flags it can pass to the build system to generate good debug information.

If one of the above wanted to test their code with the tip version of Go, for example, it would be difficult to determine the version of Go from go version

Proposal

Instead of parsing the results of git rev-parse to generate version information, this code should use git describe --tags, which describes a commit using the most recent tag reachable from it. This generates version strings of the following form:

  • go1.9.1
  • go1.9rc1
  • go1.9beta2-170-gac29f30dbb (170 commits ahead of go1.9beta2, revision ac29f30)

as documented on gitrevision(7)
which should provide any tools with enough granularity into the go version for any special-cased behavior.

@dr2chase
Copy link
Contributor

Don't forget to "git fetch" before trying this:

git describe --tags
go1.8beta2-2306-g77580771ac

@heschi
Copy link
Contributor

heschi commented Jul 31, 2017

My 2c: I see a lot of merit for tool consumption. I'm less convinced we should show it to users.

I think there's something confusing about this from a user's perspective, but I'm not certain what it is exactly. Maybe the problem is that it's a mix of where we're going and where we came from: 1.8beta2 says "a little before 1.8" but 2306 is "a little after" so you end up with "a little after a little before 1.8" which is a pretty strange way to phrase it.

Separately, getting decent descriptions out of this will require us to set more/different tags than we do today:

$ git checkout go1.9beta1^
HEAD is now at 8c4bec8fb7... net/http: update bundled http2
$ git describe --tags              
go1.8beta2-2045-g8c4bec8fb7

There's no way that's a good description of the commit right before 1.9beta1. I presume the reason it shows 1.8beta is that the 1.8 tag is on the release branch, so we'd need to put some tag on master indicating that the branch had been unfrozen for 1.9. I'm not sure if that would be something with 1.8 in it (say "go1.8done") or 1.9 (say "go1.9-devel"); I find the latter clearer, but inconsistent with the behavior after a beta is tagged.

@rsc
Copy link
Contributor

rsc commented Jul 31, 2017

  1. It's important that the non-released versions be marked by a version string starting with a "devel " prefix. What comes after the prefix is less important.

  2. I'm skeptical that git describe output is actually useful for the given purposes. For feature flags like gogland wants, it would be better to be able to dump out the in-effect release build tags, which can be done by 'go run x.go' where x.go does fmt.Println(build.Default.ReleaseTags) (using the go/build package). For details of runtime guts like delve wants, I don't see any reliable way to infer that from the git describe output. Probably finding the GOROOT and reading the runtime sources is the only way today. We could engineer a custom solution for delve, but it wouldn't be about version strings. (Actually, why does delve not use the DWARF info? These runtime structs are described in the DWARF info, no?)

I don't see a compelling reason to make any changes here.

@paranoiacblack
Copy link
Contributor Author

paranoiacblack commented Jul 31, 2017

I don't understand why "devel" is an interesting prefix to keep in the go version output. I would expect that any prefix of go version would actually start with the word go and be followed by the most recent version it refers to, hence go1.8. I agree that what comes after the prefix is not important; perhaps you might want to name this go1.8 devel +835dfef939 Wed Jul 26 13:29:59 2017 +0000. This proposal is to give the form we've decided on "devel +HASH DATE" a formal output that people can code against. If we choose to stay with this format, at the very least it should be documented as such since it seems that people are relying on its output.

To your second point, it appears that both delve and gogland explicitly do not want run a separate program just to find the version of Go or they would have used build.Default.ReleaseTags. I think that it is valuable to provide this information statically (we already do, it just isn't formalized). Delve would likely just write some code to detect a revision range in which a particular feature exists which they could also do right now.

@josharian
Copy link
Contributor

Inferring tool chain capabilities from a version string seems like a losing battle. Better would be to ask the tool chain (or runtime) directly about the questions you want answered.

This can be done today in the usual (./configure-y) way, by trying invocations on specifically crafted files (containing type aliases) or with specific command line options (DWARF location lists) and inspect the results. (Or getting build.Default.ReleaseTags.) But that is not fun to maintain either.

Perhaps instead capabilities of interest could be added to go env or something similar? Or placed in an easy-to-parse text file in GOROOT?

it appears that both delve and gogland explicitly do not want run a separate program just to find the version of Go

They're already running a separate command (go version), no?

@zolotov
Copy link
Contributor

zolotov commented Aug 1, 2017

They're already running a separate command (go version), no?

Gogland does not run a separate command. It reads zversion.go. Mainly, because the file reading is more predictable than running unknown binary. Also, because the results of reading file and parsing version are easy to cache and the cache is easy to invalidate on file's change. It is not so for running external process.

placed in an easy-to-parse text file in GOROOT?

This one sounds good to me, but what's the process then? Who would decide which capabilities should be on the list? What if one decided not to put a name of implemented feature in the capabilities-file and after a new version released, it turned out that it's needed for someone to have the feature in the capabilities-file? In this case, checking just version for retrieving capabilities of a particular Go SDK seems to be more flexible.

@heschi
Copy link
Contributor

heschi commented Aug 1, 2017

@josharian As a user of Delve, I want it to be able to debug binaries it didn't build, and I don't want to have to go download the runtime that generated that binary. For that to be possible, whatever information used needs to be baked into the binary. Right now, that's pretty much just the version. We could of course add more. But in short, no, it's not running go version.

@rsc: Yes, much of what Delve wants is in the DWARF, but not all. One place that it used the version number was to find the G pointer in TLS, which changed in 1.4. Also, even if the structures themselves are in the DWARF, their interpretation is not. For example, I think the representation of interfaces has changed in ways that wouldn't have been easy to detect without knowing the version. Also, the DWARF itself may change its interpretation, e.g. CL 41873, though in that case it was possible to just check the old way too.

The concerns @zolotov raised above about not knowing what you need to represent in the feature list beforehand also apply to debuggers; we can always go back and fix them, of course. Still, having the client take on that responsibility has been pretty effective so far.

From my perspective, the major reason to use explicit feature flags/capabilities is to allow tools to support new features on devel builds before they're tagged with a version. So far, tools are coming out with feature support well into the release candidate stage, so I don't think that's too important right now.

@rsc
Copy link
Contributor

rsc commented Aug 7, 2017

In the original report:

  • Gogland needs to know what language features to enable, such as vendoring and type aliases.
  • Delve needs to know details of the runtime, such as where to find the G pointer in a thread's registers, ...
  • ... as well as what flags it can pass to the build system to generate good debug information.

(The last two were one bullet in the original.)

Examining a binary (#2) is very different from deciding how to invoke a compiler (#1 and #3), and for examining a binary the debug info is the clear answer. It would probably make sense for delve developers to file a separate issue for #2 explaining what they need that's not in the debug info, and we can work on expanding the debug info to have everything delve needs.

As for #1 and #3, it does seem that knowing the build tags (and maybe also which compiler is being used, like gc vs gccgo) is the answer, and we can make the go command print that. That seems like #21207. So maybe discussion of those two should go back there.

At that point maybe we should close this issue.

@aarzilli
Copy link
Contributor

aarzilli commented Aug 8, 2017

It would probably make sense for delve developers to file a separate issue for #2 explaining what they need that's not in the debug info, and we can work on expanding the debug info to have everything delve needs.

There is nothing at the moment that fits this description. Delve is parsing the output of go version to know which flags to pass to the compiler, that's all.

@paranoiacblack
Copy link
Contributor Author

It seems the literal git description of a Go release doesn't answer the meta-questions the tools consuming go version want the answer to from #21207. It seems that the takeaways from this thread are to focus on providing either build tags (go1.9) or capabilities (type-aliases, plugins) statically to allow tools to reliably toggle features. Closing this and moving discussion there.

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

8 participants