cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged #37336

perillo · 2020-02-20T17:52:39Z

What version of Go are you using (`go version`)?

$ go version
go version go1.14rc1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/manlio/sdk/go1.14rc1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/manlio/sdk/go1.14rc1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/manlio/src/go/src/mperillo.test/issue/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build788093506=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14rc1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14rc1
uname -sr: Linux 5.5.4-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.31.
gdb --version: GNU gdb (GDB) 9.1

What did you do?

First, I configured git with:

[url "file:///home/manlio/src/go/src/mperillo.test/"]
	insteadOf = https://mperillo.test/

in order to test with local module paths.

In a new git repository with only 1 commit:

$ go1.14rc1 clean -modcache
$ GOPRIVATE="*" go1.14rc1 mod download -json mperillo.test/issue.git@HEAD
{
	"Path": "mperillo.test/issue.git",
	"Version": "v0.0.0-20200220172602-e96c074fea4f",
}

Now I release a version, without a new commit:

$ git tag -a v0.1.0
$ GOPRIVATE="*" go1.14rc1 mod download -json mperillo.test/issue.git@HEAD
{
	"Path": "mperillo.test/issue.git",
	"Version": "v0.1.1-0.20200220172602-e96c074fea4f",
	"Error": "mperillo.test/issue.git@v0.1.1-0.20200220172602-e96c074fea4f: invalid pseudo-version: tag (v0.1.0) found on revision e96c074fea4f is already canonical, so should not be replaced with a pseudo-version derived from that tag",
}

What did you expect to see?

Not sure.

But it is not clear why it fails, and the error message seems a bit confusing. Why is go trying to replace the canonical v0.1.0 if this is the version that it is processing now?

P.S.

The actual module path, as defined in go.mod, is mperillo.test/issue and not mperillo.test/issue.git. Can this be considered a bug?

The .git extension was added to force go to don't try to get https://mperillo.test/issue?go-get=1.

The text was updated successfully, but these errors were encountered:

bcmills · 2020-02-20T18:05:52Z

The originally-derived pseudo-version v0.0.0-20200220172602-e96c074fea4f should remain correct.

The newly-derived pseudo-version v0.1.1-0.20200220172602-e96c074fea4f really is an incorrect name for that commit. So we need to figure out why go mod download is resolving it to a pseudo-version instead of the tagged version.

perillo · 2020-02-20T18:36:55Z

I tested again with a clean module cache and git repository.

I noted that the pseudo-version v0.1.1-0.20200220182651-2e7323fa45eb is in the module cache, even when go mod download failed.

When I invoke GOPRIVATE="*" go1.14rc1 mod download -json mperillo.test/issue.git@HEAD again, it works fine, and all the versions are in the module cache.

When I use the version latest instead of HEAD it works fine.

bcmills · 2020-02-20T18:48:33Z

My best guess (and, to be clear, it's just a guess) is that we may have another bug in either the git client binary itself or the go command's interaction with it.

...speaking of which, what git client version are you using?

perillo · 2020-02-20T18:53:05Z

git version 2.25.1

perillo · 2020-02-20T19:31:32Z

In src/cmd/go/internal/modfetch/codehost/git.go, I added some tracing statements:
https://gist.github.com/perillo/789078ef6af7611a34879c9eb301b891

When I invoke go mod download for the first time, the trace is

r.local: false
r.local: false
r.local: false

After releasing version v0.1.0:

r.local: false
r.local: false
r.local: false
found tag tag: v0.1.0,
added tags [v0.1.0]

And after calling go mod download again, without any changes:

r.Local: false
found tag tag: v0.1.0,
added tags [v0.1.0]
r.Local: false
found tag tag: v0.1.0,
added tags [v0.1.0]

bcmills · 2020-02-20T21:38:26Z

Ok, I think I understand what's going on.

During the first fetch, the tag doesn't exist, so the statLocal call (correctly) returns an empty tag set.

During the second fetch, statLocal thinks that it has enough information about the commit to resolve the query from the cache. And that's (maybe) arguably correct, but then we have a problem. When we don't find an exact tag on the commit, we call RecentTag to find a pseudo-version base, and RecentTag fetches more history (including the actual tag).

After it fetches that history, RecentTag (correctly) identifies the tag on the commit itself as the most recent one and returns that as the pseudo-version base. Then we go to verify that pseudo-version and notice that it's based on a tag on the commit itself, which is not allowed.

bcmills · 2020-02-20T21:44:08Z

I'm not sure what the right fix here is. We certainly need to fetch tags more aggressively, but we might not want to do that for every repo that we Stat, especially if the version we're requesting is already canonical.

bcmills · 2020-02-20T21:45:17Z

#27171 is somewhat related, in that we should fetch more data about tags even if we can theoretically produce a valid answer using only local information.

bcmills · 2020-02-20T21:45:50Z

CC @jayconrod @matloob

perillo · 2020-02-20T22:03:58Z

Is there a reason why go does not fetch all the tags and heads as soon as possible?
It should, unless the version we're requesting is already canonical, as you wrote.

bcmills · 2020-02-21T14:33:59Z

In the past we have tried to make the go command fetch as little of the upstream repositories as possible, so that we don't consume extra disk space on the user's machine without some corresponding benefit.

However, that has exposed a lot of git client bugs, and a lot of subtleties in git client usage that have led to bugs in the go command itself. Especially given the default GOPROXY in Go 1.13, we can probably afford to be a little less parsimonious now in order to have a simpler (and, hopefully, more correct) implementation.

perillo · 2020-02-21T17:10:30Z

In gopath mode, the go command clones the entire repository. In module mode it only fetches the heads and the tags, unless the user explicitly require a commit ID. I don't think the extra disk space is an issue.

perillo · 2020-02-21T17:12:08Z

However I remember a message from Russ where he mentioned that, if possible, he wanted to avoid receiving code from a remote repository due to possible security bugs with the git client.

bcmills · 2020-02-25T18:41:56Z

At one point we did only fetch the heads and tags. However, the current version of the code fetches the refs and tags along with all ancestors of those commits:

go/src/cmd/go/internal/modfetch/codehost/git.go

Lines 407 to 437 in 2ed96d0

    
           // fetchRefsLocked fetches all heads and tags from the origin, along with the 
        
           // ancestors of those commits. 
        
           // 
        
           // We only fetch heads and tags, not arbitrary other commits: we don't want to 
        
           // pull in off-branch commits (such as rejected GitHub pull requests) that the 
        
           // server may be willing to provide. (See the comments within the stat method 
        
           // for more detail.) 
        
           // 
        
           // fetchRefsLocked requires that r.mu remain locked for the duration of the call. 
        
           func (r *gitRepo) fetchRefsLocked() error { 
        
           	if r.fetchLevel < fetchAll { 
        
           		// NOTE: To work around a bug affecting Git clients up to at least 2.23.0 
        
           		// (2019-08-16), we must first expand the set of local refs, and only then 
        
           		// unshallow the repository as a separate fetch operation. (See 
        
           		// golang.org/issue/34266 and 
        
           		// https://github.com/git/git/blob/4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a/transport.c#L1303-L1309.) 
        
           		if _, err := Run(r.dir, "git", "fetch", "-f", r.remote, "refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"); err != nil { 
        
           			return err 
        
           		} 
        
           		if _, err := os.Stat(filepath.Join(r.dir, "shallow")); err == nil { 
        
           			if _, err := Run(r.dir, "git", "fetch", "--unshallow", "-f", r.remote); err != nil { 
        
           				return err 
        
           			} 
        
           		} 
        
           		r.fetchLevel = fetchAll 
        
           	} 
        
           	return nil 
        
           }

That's potentially a lot of data, but given the default GOPROXY usage it's probably fine for non-canonical versions.

bcmills changed the title ~~cmd/go: mod download modpath@HEAD with git has problems when HEAD is tagged later~~ cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is a tagged version Feb 20, 2020

bcmills added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 20, 2020

bcmills added this to the Backlog milestone Feb 20, 2020

bcmills changed the title ~~cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is a tagged version~~ cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged Feb 20, 2020

bcmills added the help wanted label Feb 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged #37336

cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged #37336

perillo commented Feb 20, 2020

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020 •

edited

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020

perillo commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020 •

edited

bcmills commented Feb 21, 2020

perillo commented Feb 21, 2020

perillo commented Feb 21, 2020 •

edited

bcmills commented Feb 25, 2020

cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged #37336

cmd/go: mod download modpath@HEAD erroneously resolves to a pseudo-version when HEAD is subsequently tagged #37336

Comments

perillo commented Feb 20, 2020

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

P.S.

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020 • edited

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020

perillo commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

bcmills commented Feb 20, 2020

perillo commented Feb 20, 2020 • edited

bcmills commented Feb 21, 2020

perillo commented Feb 21, 2020

perillo commented Feb 21, 2020 • edited

bcmills commented Feb 25, 2020

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

perillo commented Feb 20, 2020 •

edited

perillo commented Feb 20, 2020 •

edited

perillo commented Feb 21, 2020 •

edited