git: remote, Add shallow commits instead of substituting. Fixes #412

updateShallow substituted the previous shallow list with the one
returned by the UploadPackResponse. If the repository had previous
shallow commits these are deleted from the list.

This change adds the new shallow hashes to the old ones.

Signed-off-by: Javi Fontan <jfontan@gmail.com>

3 files changed, 132 insertions, 2 deletions

diff --git a/remote.go b/remote.go
index 666c9f5..4b86955 100644
--- a/remote.go
+++ b/remote.go
@@ -976,9 +976,24 @@ func pushHashes(
 }
 
 func (r *Remote) updateShallow(o *FetchOptions, resp *packp.UploadPackResponse) error {
-	if o.Depth == 0 {
+	if o.Depth == 0 || len(resp.Shallows) == 0 {
 		return nil
 	}
 
-	return r.s.SetShallow(resp.Shallows)
+	shallows, err := r.s.Shallow()
+	if err != nil {
+		return err
+	}
+
+outer:
+	for _, s := range resp.Shallows {
+		for _, oldS := range shallows {
+			if s == oldS {
+				continue outer
+			}
+		}
+		shallows = append(shallows, s)
+	}
+
+	return r.s.SetShallow(shallows)
 }
diff --git a/remote_test.go b/remote_test.go
index e586e7a..82ec1fc 100644
--- a/remote_test.go
+++ b/remote_test.go
@@ -9,6 +9,7 @@ import (
 
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/plumbing/protocol/packp"
 	"gopkg.in/src-d/go-git.v4/plumbing/storer"
 	"gopkg.in/src-d/go-git.v4/storage"
 	"gopkg.in/src-d/go-git.v4/storage/filesystem"
@@ -741,3 +742,54 @@ func (s *RemoteSuite) TestList(c *C) {
 		c.Assert(found, Equals, true)
 	}
 }
+
+func (s *RemoteSuite) TestUpdateShallows(c *C) {
+	hashes := []plumbing.Hash{
+		plumbing.NewHash("0000000000000000000000000000000000000001"),
+		plumbing.NewHash("0000000000000000000000000000000000000002"),
+		plumbing.NewHash("0000000000000000000000000000000000000003"),
+		plumbing.NewHash("0000000000000000000000000000000000000004"),
+		plumbing.NewHash("0000000000000000000000000000000000000005"),
+		plumbing.NewHash("0000000000000000000000000000000000000006"),
+	}
+
+	tests := []struct {
+		hashes []plumbing.Hash
+		result []plumbing.Hash
+	}{
+		// add to empty shallows
+		{hashes[0:2], hashes[0:2]},
+		// add new hashes
+		{hashes[2:4], hashes[0:4]},
+		// add some hashes already in shallow list
+		{hashes[2:6], hashes[0:6]},
+		// add all hashes
+		{hashes[0:6], hashes[0:6]},
+		// add empty list
+		{nil, hashes[0:6]},
+	}
+
+	remote := newRemote(memory.NewStorage(), &config.RemoteConfig{
+		Name: DefaultRemoteName,
+	})
+
+	shallows, err := remote.s.Shallow()
+	c.Assert(err, IsNil)
+	c.Assert(len(shallows), Equals, 0)
+
+	resp := new(packp.UploadPackResponse)
+	o := &FetchOptions{
+		Depth: 1,
+	}
+
+	for _, t := range tests {
+		resp.Shallows = t.hashes
+		err = remote.updateShallow(o, resp)
+		c.Assert(err, IsNil)
+
+		shallow, err := remote.s.Shallow()
+		c.Assert(err, IsNil)
+		c.Assert(len(shallow), Equals, len(t.result))
+		c.Assert(shallow, DeepEquals, t.result)
+	}
+}
diff --git a/repository_test.go b/repository_test.go
index c98e2ac..be7f163 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -1615,3 +1615,66 @@ func executeOnPath(path, cmd string) error {
 
 	return c.Run()
 }
+
+func (s *RepositorySuite) TestBrokenMultipleShallowFetch(c *C) {
+	r, _ := Init(memory.NewStorage(), nil)
+	_, err := r.CreateRemote(&config.RemoteConfig{
+		Name: DefaultRemoteName,
+		URLs: []string{s.GetBasicLocalRepositoryURL()},
+	})
+	c.Assert(err, IsNil)
+
+	c.Assert(r.Fetch(&FetchOptions{
+		Depth:    2,
+		RefSpecs: []config.RefSpec{config.RefSpec("refs/heads/master:refs/heads/master")},
+	}), IsNil)
+
+	shallows, err := r.Storer.Shallow()
+	c.Assert(err, IsNil)
+	c.Assert(len(shallows), Equals, 1)
+
+	ref, err := r.Reference("refs/heads/master", true)
+	c.Assert(err, IsNil)
+	cobj, err := r.CommitObject(ref.Hash())
+	c.Assert(err, IsNil)
+	c.Assert(cobj, NotNil)
+	err = object.NewCommitPreorderIter(cobj, nil, nil).ForEach(func(c *object.Commit) error {
+		for _, ph := range c.ParentHashes {
+			for _, h := range shallows {
+				if ph == h {
+					return storer.ErrStop
+				}
+			}
+		}
+
+		return nil
+	})
+	c.Assert(err, IsNil)
+
+	c.Assert(r.Fetch(&FetchOptions{
+		Depth:    5,
+		RefSpecs: []config.RefSpec{config.RefSpec("refs/heads/*:refs/heads/*")},
+	}), IsNil)
+
+	shallows, err = r.Storer.Shallow()
+	c.Assert(err, IsNil)
+	c.Assert(len(shallows), Equals, 3)
+
+	ref, err = r.Reference("refs/heads/master", true)
+	c.Assert(err, IsNil)
+	cobj, err = r.CommitObject(ref.Hash())
+	c.Assert(err, IsNil)
+	c.Assert(cobj, NotNil)
+	err = object.NewCommitPreorderIter(cobj, nil, nil).ForEach(func(c *object.Commit) error {
+		for _, ph := range c.ParentHashes {
+			for _, h := range shallows {
+				if ph == h {
+					return storer.ErrStop
+				}
+			}
+		}
+
+		return nil
+	})
+	c.Assert(err, IsNil)
+}