cmd/go: go get $module@$commit should resolve it to the highest appropriate pseudo-version #27171
Comments
bcmills
added
the
NeedsInvestigation
The I don't know how you got an entry mapping that commit to At any rate, if you |
bcmills
changed the title
|
Another observation. $ export GOPATH=$(mktemp -d)
$ go list -m -json golang.org/x/text@master
$ cat go.mod
module foo
go 1.12
$ go list -m -json golang.org/x/text@master
go: finding golang.org/x/text master
{
"Path": "golang.org/x/text",
"Version": "v0.0.0-20181227161524-e6919f6577db",
"Time": "2018-12-27T16:15:24Z"
}
$ go list -m -json golang.org/x/text@master
go: finding golang.org/x/text master
{
"Path": "golang.org/x/text",
"Version": "v0.3.1-0.20181227161524-e6919f6577db",
"Time": "2018-12-27T16:15:24Z"
}
Note that two subsequent go list commands (back to back) resulted in two different pseudo versions based on the same hash. It seems the pseudo-version generation is sensitive to what is in the module cache. $ go version go version devel +5fae09b738 Tue Jan 15 23:30:58 2019 +0000 darwin/amd64 |
|
Working with docker with Go modules is currently a bit convoluted for multiple reasons (e.g., sirupsen/logrus#799 (comment)). There are solutions (e.g., see #28489 (comment), #28489 (comment), and others), but this issue here adds to the complexity. For example, you can have something like the following version (from early 2018) in one but then because that pseudo-version seems to incorrectly use On top of that, CC @vito-c |
|
This write-up here is based on a older quick look, so please take with grain of salt. Depending on how you count, there might be 2-3 different things going on here:
This issue crops up in multiple repos, including notably docker, but a simplified constructed example showing a bad result vs. a good result:
Separately, I have not looked at the original report in #27171 (comment). Perhaps that falls into one of these three buckets, or perhaps it was just an older |
|
I'm trying to wrap my head around pseudo-versions and how they are intended to work, in particular the vn.n.n-0.date-hash form. The current documentation in pseudo.go states that:
What is the exact definition of "most recent" here? Is it the most recent in the current branch, the most recent that is merged into the current branch or the most recent that exists on the current branch? (or something else). As an example, if I have this tree: .. and runs
|
It is intended to be the semantically-latest tag that appears on any commit that is a (transitive) parent of the commit with the given hash, regardless of branches. (The pseudo-version is supposed to sort after every version — tagged or otherwise — that came before it, but before the next tag that a human might plausibly want to apply to the branch.) In your example, you should expect a pseudo-version that sorts just after Of course, bugs are possible (and, I suspect in this case, likely). 🙂 |
|
Nice, and two more questions for the same context:
|
We don't go out of our way to distinguish. I suspect that we should consider both. |
The major-version handling probably needs work. If there is no If there is a |
|
@leitzler For your example in #27171 (comment), Bryan commented in #27171 (comment) on the expected result, but what do you currently see as the actual result for your specific example with Go 1.12 or tip? In other words, with the current implementation, do you see a pseudo-version that sorts just after one of these, and if so, which one:
|
|
Go 1.12.4 |
|
However, if I ask for |
|
@leitzler OK, Go 1.12 seems to give the wrong answer for In #27171 (comment) above, I had tried to differentiate between what seemed to be at least three different underlying problems for the different examples cited in this issue. It would at least seem from the outside that your example and #31673 are likely a fourth category of problem. |
|
Change https://golang.org/cl/174061 mentions this issue: |
…tags and tags on other branches Pseudoversion determination depends in part on the results from gitRepo.RecentTag, which currently invokes: git describe --first-parent --always --abbrev=0 --match <prefix>v[0-9]*.[0-9]*.[0-9]* --tags <rev> The comment at #27171 (comment) describes some problems with the current approach. One problem is Docker and other repos can have tags that are not valid semver tags but that still match a glob pattern of v[0-9]*.[0-9]*.[0-9]* which are found by 'git describe' but then rejected by cmd/go, and hence those repos currently can end up with v0.0.0 pseudoversions instead of finding a proper semver tag to use as input to building a pseudoversion (when then causes problems when the v0.0.0 pseudoversion is fed into MVS). An example problematic tag is a date-based tag such as 'v18.06.16', which matches the glob pattern, but is not a valid semver tag (due to the leading 0 in '06'). Issues #31673, #31287, and #27171 also describe problems where the '--first-parent' argument to 'git describe' cause the current approach to miss relevant semver tags that were created on a separate branch and then subsequently merged to master. In #27171, Bryan described the base tag that is supposed to be used for pseudoversions as: "It is intended to be the semantically-latest tag that appears on any commit that is a (transitive) parent of the commit with the given hash, regardless of branches. (The pseudo-version is supposed to sort after every version — tagged or otherwise — that came before it, but before the next tag that a human might plausibly want to apply to the branch.)" This CL solves the glob problem and tags-on-other-branches problem more directly than the current approach: this CL gets the full list of tags that have been merged into the specific revision of interest, and then sorts and filters the results in cmd/go to select the semantically-latest valid semver tag. Fixes #31673 Fixes #31287 Updates #27171 Change-Id: I7c3e6b46b2b21dd60562cf2893b6bd2afaae61d5 Reviewed-on: https://go-review.googlesource.com/c/go/+/174061 Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
|
Change https://golang.org/cl/176417 mentions this issue: |
|
@thepudds Yes, definitely related, but the description of that issue isn't very clear to me, so I'm not sure if it's a strict dupe. The underlying problem here , I think, is that the go command is using existing information in |
|
Hi @rogpeppe, agreed not a strict dup. Part of the relationship is #27173 might show how one can put the wrong semver tag within a pseudoversion, and it still works without the go tool changing it to be a correct semver tag. That said, #27173 happens to not be 100% clear to me either.
There might be performance implications always of going to the source as part of something like |
|
@rogpeppe, thanks for the clear reproducer. That confirms my suspicions about the mechanism of the bug. I tend to agree that we should go to version control (or re-query the proxy) to resolve commits to versions. It's not clear to me whether we should try the local cache first when |
|
@thepudds, there should be no performance implication for (And, in fact, it may be important not to re-resolve the version as the underlying tags evolve, since that can affect the MVS result and impact reproducibility.) |
I'd second this. |
I think I misunderstood @rogpeppe's above suggestion then. Based on @bcmills comment, is the suggestion instead:
If that is the suggestion, then I withdraw my comment #27171 (comment) on performance / correctness concern. |
|
The pseudoversion for golang.org/x/text in #27171 (comment) seemed to be sensitive to what was in the cache. From looking at that briefly several months ago and mentioned in #27171 (comment), it seemed to be due to different results for the tag returned by Trying to recreate the problem from #27171 (comment) with current tip no longer seems to exhibit the problem: which consistently returns: Using the commit hash from the initial report in #27171 (comment) also now seems consistent: which consistently returns: I am not sure if that is due to changes in the x/text repo, or other ecosystem changes, or perhaps due to changes in the @hyangah I would be curious to hear if you able to reproduce your original report from #27171 (comment), or perhaps a variation? |
|
@thepudds I just confirmed that my reproducer in #27171 (comment) still shows this issue with Go tip (as of commit e883d00). Perhaps you could check that to see if it works for you? |
|
@thepudds I can't reproduce my report that uses golang.org/x/text either. I tested it again after manually reverting the cl/174061 and still couldn't reproduce the issue. So it's possible there was a different underlying issue, and now it's not reproducible. |
|
|
|
In comment #27171 (comment) from several months ago, I had attempted to tease apart what the different underlying problems might be. At that time, the count was three. At this point, there are probably at least 5-6 underlying problems reported in this issue, which I will attempt to enumerate as:
Given the variety above, it would not be shocking if there were additional underlying issues not yet teased out. |
|
Change https://golang.org/cl/179857 mentions this issue: |
… in (*codeRepo).convert Previously, we used the passed-in statVers as the basis for tag search, but it is not always valid. Instead, use info.Name, which (by precondition) must be valid. Updates #32161 Updates #27171 Change-Id: Iaecb5043bdf2fefd26fbe3f8e3714b07d22f580f Reviewed-on: https://go-review.googlesource.com/c/go/+/179857 Run-TryBot: Bryan C. Mills <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
|
Change https://golang.org/cl/181881 mentions this issue: |
|
Change https://golang.org/cl/182178 mentions this issue: |
|
I've seen several examples in the module index (https://index.golang.org) of invalid pseudo-versions starting with Does anyone have any examples of a real (Perhaps @marwan-at-work, @rogpeppe, @myitcv, or one of the other folks with third-party module-related tools has a clue?) |
No clue here. |
|
I noticed one of my repos in a listing you pasted on slack (or in another issue? Can’t really remember where I saw it). Anything regarding https://github.com/modsdemo is my fault. Have been playing around on purpose with it. |
|
@leitzler, thanks for the heads-up. (The comment you saw it in was #27173 (comment).) |
…tadata Previously, most operations involving pseudo-versions allowed any arbitrary combination of version string and date, and would resolve to the underlying revision (typically a Git commit hash) as long as that revision existed. There are a number of problems with that approach: • The pseudo-version participates in minimal version selection. If its version prefix is inaccurate, the pseudo-version may appear to have higher precedence that the releases that follow it, effectively “pinning” the module to that commit. For release tags, module authors are the ones who make the decision about release tagging; they should also have control over the pseudo-version precedence within their module. • The commit date within the pseudo-version provides a total order among pseudo-versions. If it is not accurate, the pseudo-version will sort into the wrong place relative to other commits with the same version prefix. To address those problems, this change restricts the pseudo-versions that the 'go' command accepts, rendering some previously accepted-but-not-canonical versions invalid. A pseudo-version is now valid only if all of: 1. The tag from which the pseudo-version derives points to the named revision or one of its ancestors as reported by the underlying VCS tool, or the pseudo-version is not derived from any tag (that is, has a "vX.0.0-" prefix before the date string and uses the lowest major version appropriate to the module path). 2. The date string within the pseudo-version matches the UTC timestamp of the revision as reported by the underlying VCS tool. 3. The short name of the revision within the pseudo-version (such as a Git hash prefix) is the same as the short name reported by the underlying cmd/go/internal/modfetch/codehost.Repo. Specifically, if the short name is a SHA-1 prefix, it must use the same number of hex digits (12) as codehost.ShortenSHA1. 4. The pseudo-version includes a '+incompatible' suffix only if it is needed for the corresponding major version, and only if the underlying module does not have a go.mod file. We believe that all releases of the 'go' tool have generated pseudo-versions that meet these constraints. However, a few pseudo-versions edited by hand or generated by third-party tools do not. If we discover invalid-but-benign pseudo-versions in widely-used existing dependencies, we may choose to add a whitelist for those specific path/version combinations. ― To work around invalid dependencies in leaf modules, users may add a 'replace' directive from the invalid version to its valid equivalent. Note that the go command's go.mod parser automatically resolves commit hashes found in 'replace' directives to the appropriate pseudo-versions, so in most cases one can write something like: replace github.com/docker/docker v1.14.0-0.20190319215453-e7b5f7dbe98c => github.com/docker/docker e7b5f7dbe98c and then run any 'go' command (such as 'go list' or 'go mod tidy') to resolve it to an appropriate pseudo-version. Note that the invalid version will still be used in minimal version selection, so this use of 'replace' directives is an incomplete workaround. ― One of the common use cases for higher-than-tagged pseudo-versions is for projects that do parallel development on release branches. For example, if a project cuts a 'v1.2' release branch at v1.2.0, they may want future commits on the main branch to show up as pre-releases for v1.3.0 rather than for v1.2.1 — especially if v1.2.1 is already tagged on the release branch. (On the other hand, a backport of a patch to the v1.2 branch should not show up as a pre-release for v1.3.0.) To address this use-case, module authors can make use of our existing support for pseudo-versions derived from pre-release tags: if the author adds an explicit pre-release tag (such as 'v1.3.0-devel') to the first commit after the branch, then the pseudo-versions for that commit and its descendents will be derived from that tag and will sort appropriately in version selection. ― Updates #27171 Fixes #29262 Fixes #27173 Fixes #32662 Fixes #32695 Change-Id: I0d50a538b6fdb0d3080aca9c9c3df1040da1b329 Reviewed-on: https://go-review.googlesource.com/c/go/+/181881 Run-TryBot: Bryan C. Mills <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
With my existing $GOPATH, I can run
go getto get a particular commit, and it doesn't use the correct version as I'd expect from https://go-review.googlesource.com/c/go/+/124515, but downgrades instead. With a fresh $GOPATH, it works OK, so I suspect that somehow the cached info isn't being updated appropriately.Here's what I did:
Note that despite getting the latest master commit of
go-cmp, it has downgraded the quicktest package inappropriately, and at no point did it run thegit describecommand which would have told it which version to use. If I use@master, it works correctly:If I use a fresh
$GOPATH, it also works OK:The text was updated successfully, but these errors were encountered: