git: Fix fetching after shallow clone. Fixes #305

Signed-off-by: Arieh Schneier <15041913+AriehSchneier@users.noreply.github.com>

3 files changed, 116 insertions, 44 deletions

diff --git a/common_test.go b/common_test.go
index 3311a17..7f9b84b 100644
--- a/common_test.go
+++ b/common_test.go
@@ -3,10 +3,12 @@ package git
 import (
 	"os"
 	"testing"
+	"time"
 
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/cache"
 	"github.com/go-git/go-git/v5/plumbing/format/packfile"
+	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/go-git/go-git/v5/storage/memory"
 
@@ -212,3 +214,36 @@ func AssertReferencesMissing(c *C, r *Repository, expected []string) {
 		c.Assert(err, Equals, plumbing.ErrReferenceNotFound)
 	}
 }
+
+func CommitNewFile(c *C, repo *Repository, fileName string) plumbing.Hash {
+	wt, err := repo.Worktree()
+	c.Assert(err, IsNil)
+
+	fd, err := wt.Filesystem.Create(fileName)
+	c.Assert(err, IsNil)
+
+	_, err = fd.Write([]byte("# test file"))
+	c.Assert(err, IsNil)
+
+	err = fd.Close()
+	c.Assert(err, IsNil)
+
+	_, err = wt.Add(fileName)
+	c.Assert(err, IsNil)
+
+	sha, err := wt.Commit("test commit", &CommitOptions{
+		Author: &object.Signature{
+			Name:  "test",
+			Email: "test@example.com",
+			When:  time.Now(),
+		},
+		Committer: &object.Signature{
+			Name:  "test",
+			Email: "test@example.com",
+			When:  time.Now(),
+		},
+	})
+	c.Assert(err, IsNil)
+
+	return sha
+}
diff --git a/remote.go b/remote.go
index 7b2741a..0e72aad 100644
--- a/remote.go
+++ b/remote.go
@@ -459,7 +459,7 @@ func (r *Remote) fetch(ctx context.Context, o *FetchOptions) (sto storer.Referen
 
 	req.Wants, err = getWants(r.s, refs)
 	if len(req.Wants) > 0 {
-		req.Haves, err = getHaves(localRefs, remoteRefs, r.s)
+		req.Haves, err = getHaves(localRefs, remoteRefs, r.s, o.Depth)
 		if err != nil {
 			return nil, err
 		}
@@ -837,6 +837,7 @@ func getHavesFromRef(
 	remoteRefs map[plumbing.Hash]bool,
 	s storage.Storer,
 	haves map[plumbing.Hash]bool,
+	depth int,
 ) error {
 	h := ref.Hash()
 	if haves[h] {
@@ -862,7 +863,13 @@ func getHavesFromRef(
 	// commits from the history of each ref.
 	walker := object.NewCommitPreorderIter(commit, haves, nil)
 	toVisit := maxHavesToVisitPerRef
-	return walker.ForEach(func(c *object.Commit) error {
+	// But only need up to the requested depth
+	if depth > 0 && depth < maxHavesToVisitPerRef {
+		toVisit = depth
+	}
+	// It is safe to ignore any error here as we are just trying to find the references that we already have
+	// An example of a legitimate failure is we have a shallow clone and don't have the previous commit(s)
+	_ = walker.ForEach(func(c *object.Commit) error {
 		haves[c.Hash] = true
 		toVisit--
 		// If toVisit starts out at 0 (indicating there is no
@@ -873,12 +880,15 @@ func getHavesFromRef(
 		}
 		return nil
 	})
+
+	return nil
 }
 
 func getHaves(
 	localRefs []*plumbing.Reference,
 	remoteRefStorer storer.ReferenceStorer,
 	s storage.Storer,
+	depth int,
 ) ([]plumbing.Hash, error) {
 	haves := map[plumbing.Hash]bool{}
 
@@ -899,7 +909,7 @@ func getHaves(
 			continue
 		}
 
-		err = getHavesFromRef(ref, remoteRefs, s, haves)
+		err = getHavesFromRef(ref, remoteRefs, s, haves, depth)
 		if err != nil {
 			return nil, err
 		}
diff --git a/remote_test.go b/remote_test.go
index 1bcdb0f..f8a0bdb 100644
--- a/remote_test.go
+++ b/remote_test.go
@@ -14,7 +14,6 @@ import (
 	"github.com/go-git/go-git/v5/config"
 	"github.com/go-git/go-git/v5/plumbing"
 	"github.com/go-git/go-git/v5/plumbing/cache"
-	"github.com/go-git/go-git/v5/plumbing/object"
 	"github.com/go-git/go-git/v5/plumbing/protocol/packp"
 	"github.com/go-git/go-git/v5/plumbing/protocol/packp/capability"
 	"github.com/go-git/go-git/v5/plumbing/storer"
@@ -1175,7 +1174,7 @@ func (s *RemoteSuite) TestGetHaves(c *C) {
 		),
 	}
 
-	l, err := getHaves(localRefs, memory.NewStorage(), sto)
+	l, err := getHaves(localRefs, memory.NewStorage(), sto, 0)
 	c.Assert(err, IsNil)
 	c.Assert(l, HasLen, 2)
 }
@@ -1404,45 +1403,7 @@ func (s *RemoteSuite) TestCanPushShasToReference(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(repo, NotNil)
 
-	fd, err := os.Create(filepath.Join(d, "repo", "README.md"))
-	c.Assert(err, IsNil)
-	if err != nil {
-		return
-	}
-	_, err = fd.WriteString("# test repo")
-	c.Assert(err, IsNil)
-	if err != nil {
-		return
-	}
-	err = fd.Close()
-	c.Assert(err, IsNil)
-	if err != nil {
-		return
-	}
-
-	wt, err := repo.Worktree()
-	c.Assert(err, IsNil)
-	if err != nil {
-		return
-	}
-
-	wt.Add("README.md")
-	sha, err := wt.Commit("test commit", &CommitOptions{
-		Author: &object.Signature{
-			Name:  "test",
-			Email: "test@example.com",
-			When:  time.Now(),
-		},
-		Committer: &object.Signature{
-			Name:  "test",
-			Email: "test@example.com",
-			When:  time.Now(),
-		},
-	})
-	c.Assert(err, IsNil)
-	if err != nil {
-		return
-	}
+	sha := CommitNewFile(c, repo, "README.md")
 
 	gitremote, err := repo.CreateRemote(&config.RemoteConfig{
 		Name: "local",
@@ -1472,3 +1433,69 @@ func (s *RemoteSuite) TestCanPushShasToReference(c *C) {
 	}
 	c.Assert(ref.Hash().String(), Equals, sha.String())
 }
+
+func (s *RemoteSuite) TestFetchAfterShallowClone(c *C) {
+	tempDir, clean := s.TemporalDir()
+	defer clean()
+	remoteUrl := filepath.Join(tempDir, "remote")
+	repoDir := filepath.Join(tempDir, "repo")
+
+	// Create a new repo and add more than 1 commit (so we can have a shallow commit)
+	remote, err := PlainInit(remoteUrl, false)
+	c.Assert(err, IsNil)
+	c.Assert(remote, NotNil)
+
+	_ = CommitNewFile(c, remote, "File1")
+	_ = CommitNewFile(c, remote, "File2")
+
+	// Clone the repo with a depth of 1
+	repo, err := PlainClone(repoDir, false, &CloneOptions{
+		URL:           remoteUrl,
+		Depth:         1,
+		Tags:          NoTags,
+		SingleBranch:  true,
+		ReferenceName: "master",
+	})
+	c.Assert(err, IsNil)
+
+	// Add new commits to the origin (more than 1 so that our next test hits a missing commit)
+	_ = CommitNewFile(c, remote, "File3")
+	sha4 := CommitNewFile(c, remote, "File4")
+
+	// Try fetch with depth of 1 again (note, we need to ensure no remote branch remains pointing at the old commit)
+	r, err := repo.Remote(DefaultRemoteName)
+	c.Assert(err, IsNil)
+	s.testFetch(c, r, &FetchOptions{
+		Depth: 2,
+		Tags:  NoTags,
+
+		RefSpecs: []config.RefSpec{
+			"+refs/heads/master:refs/heads/master",
+			"+refs/heads/master:refs/remotes/origin/master",
+		},
+	}, []*plumbing.Reference{
+		plumbing.NewReferenceFromStrings("refs/heads/master", sha4.String()),
+		plumbing.NewReferenceFromStrings("refs/remotes/origin/master", sha4.String()),
+		plumbing.NewSymbolicReference("HEAD", "refs/heads/master"),
+	})
+
+	// Add another commit to the origin
+	sha5 := CommitNewFile(c, remote, "File5")
+
+	// Try fetch with depth of 2 this time (to reach a commit that we don't have locally)
+	r, err = repo.Remote(DefaultRemoteName)
+	c.Assert(err, IsNil)
+	s.testFetch(c, r, &FetchOptions{
+		Depth: 1,
+		Tags:  NoTags,
+
+		RefSpecs: []config.RefSpec{
+			"+refs/heads/master:refs/heads/master",
+			"+refs/heads/master:refs/remotes/origin/master",
+		},
+	}, []*plumbing.Reference{
+		plumbing.NewReferenceFromStrings("refs/heads/master", sha5.String()),
+		plumbing.NewReferenceFromStrings("refs/remotes/origin/master", sha5.String()),
+		plumbing.NewSymbolicReference("HEAD", "refs/heads/master"),
+	})
+}