[release-branch.go1.13] cmd/go/internal/modfetch/codehost: work around an apparent...

When 'git fetch' is passed the '--unshallow' flag, it assumes that the
local and remote refs are equal.¹ However, we were fetching an
expanded set of refs explicitly in the same command, violating that
assumption.

Now we first expand the set of refs, then unshallow the repo in a
separate fetch. Empirically, this seems to work, whereas the opposite
order does not.

¹https://github.com/git/git/blob/4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a/transport.c#L1303-L1309

Updates #34266
Fixes #34477

Change-Id: Ie97eb7c1223f944003a1e31d0ec9e69aad0efc0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/196961
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
(cherry picked from commit 1804bbab6285754f69a0683e60fc5590429dc1d1)
Reviewed-on: https://go-review.googlesource.com/c/go/+/197060

diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go
index d382e8ac9a9232c7ff5f7ac006eb39918ccabf66..dc5add906b8d95ee674d8addccd0f7633aed9fc6 100644 (file)
--- a/src/cmd/go/internal/modfetch/codehost/git.go
+++ b/src/cmd/go/internal/modfetch/codehost/git.go
@@ -241,13 +241,6 @@ func (r *gitRepo) findRef(hash string) (ref string, ok bool) {
        return "", false
 }
 
-func unshallow(gitDir string) []string {
-       if _, err := os.Stat(filepath.Join(gitDir, "shallow")); err == nil {
-               return []string{"--unshallow"}
-       }
-       return []string{}
-}
-
 // minHashDigits is the minimum number of digits to require
 // before accepting a hex digit sequence as potentially identifying
 // a specific commit in a git repo. (Of course, users can always
@@ -397,29 +390,27 @@ func (r *gitRepo) stat(rev string) (*RevInfo, error) {
 // fetchRefsLocked requires that r.mu remain locked for the duration of the call.
 func (r *gitRepo) fetchRefsLocked() error {
        if r.fetchLevel < fetchAll {
-               if err := r.fetchUnshallow("refs/heads/*:refs/heads/*", "refs/tags/*:refs/tags/*"); err != nil {
+               // 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
 }
 
-func (r *gitRepo) fetchUnshallow(refSpecs ...string) error {
-       // To work around a protocol version 2 bug that breaks --unshallow,
-       // add -c protocol.version=0.
-       // TODO(rsc): The bug is believed to be server-side, meaning only
-       // on Google's Git servers. Once the servers are fixed, drop the
-       // protocol.version=0. See Google-internal bug b/110495752.
-       var protoFlag []string
-       unshallowFlag := unshallow(r.dir)
-       if len(unshallowFlag) > 0 {
-               protoFlag = []string{"-c", "protocol.version=0"}
-       }
-       _, err := Run(r.dir, "git", protoFlag, "fetch", unshallowFlag, "-f", r.remote, refSpecs)
-       return err
-}
-
 // statLocal returns a RevInfo describing rev in the local git repository.
 // It uses version as info.Version.
 func (r *gitRepo) statLocal(version, rev string) (*RevInfo, error) {
@@ -539,39 +530,10 @@ func (r *gitRepo) ReadFileRevs(revs []string, file string, maxSize int64) (map[s
        }
        defer unlock()
 
-       var refs []string
-       var protoFlag []string
-       var unshallowFlag []string
-       for _, tag := range redo {
-               refs = append(refs, "refs/tags/"+tag+":refs/tags/"+tag)
-       }
-       if len(refs) > 1 {
-               unshallowFlag = unshallow(r.dir)
-               if len(unshallowFlag) > 0 {
-                       // To work around a protocol version 2 bug that breaks --unshallow,
-                       // add -c protocol.version=0.
-                       // TODO(rsc): The bug is believed to be server-side, meaning only
-                       // on Google's Git servers. Once the servers are fixed, drop the
-                       // protocol.version=0. See Google-internal bug b/110495752.
-                       protoFlag = []string{"-c", "protocol.version=0"}
-               }
-       }
-       if _, err := Run(r.dir, "git", protoFlag, "fetch", unshallowFlag, "-f", r.remote, refs); err != nil {
+       if err := r.fetchRefsLocked(); err != nil {
                return nil, err
        }
 
-       // TODO(bcmills): after the 1.11 freeze, replace the block above with:
-       //      if r.fetchLevel <= fetchSome {
-       //              r.fetchLevel = fetchSome
-       //              var refs []string
-       //              for _, tag := range redo {
-       //                      refs = append(refs, "refs/tags/"+tag+":refs/tags/"+tag)
-       //              }
-       //              if _, err := Run(r.dir, "git", "fetch", "--update-shallow", "-f", r.remote, refs); err != nil {
-       //                      return nil, err
-       //              }
-       //      }
-
        if _, err := r.readFileRevs(redo, file, files); err != nil {
                return nil, err
        }

diff --git a/src/cmd/go/testdata/script/mod_get_direct.txt b/src/cmd/go/testdata/script/mod_get_direct.txt
new file mode 100644 (file)
index 0000000..42ccbcd
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_direct.txt
@@ -0,0 +1,20 @@
+# Regression test for golang.org/issue/34092: with an empty module cache,
+# 'GOPROXY=direct go get golang.org/x/tools/gopls@master' did not correctly
+# resolve the pseudo-version for its dependency on golang.org/x/tools.
+
+[short] skip
+[!net] skip
+[!exec:git] skip
+
+env GO111MODULE=on
+env GOPROXY=direct
+env GOSUMDB=off
+
+go list -m cloud.google.com/go@master
+! stdout 'v0.0.0-'
+
+-- go.mod --
+module example.com
+
+go 1.14
+-- go.sum --