-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
Comments
Don't forget to "git fetch" before trying this:
|
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:
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. |
I don't see a compelling reason to make any changes here. |
I don't understand why "devel" is an interesting prefix to keep in the 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 |
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 Perhaps instead capabilities of interest could be added to
They're already running a separate command ( |
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.
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. |
@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 @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. |
In the original report:
(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. |
There is nothing at the moment that fits this description. Delve is parsing the output of |
It seems the literal git description of a Go release doesn't answer the meta-questions the tools consuming |
Background
This is closely related to Issue #21207 where @heschik notes that
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 ofgit 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:
These strings might be parsed by external tooling that needs to optionally rely on certain Go features to exist. For example, from #21207:
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 usegit describe --tags
, which describes a commit using the most recent tag reachable from it. This generates version strings of the following form:as documented on gitrevision(7)
which should provide any tools with enough granularity into the go version for any special-cased behavior.
The text was updated successfully, but these errors were encountered: