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 <[email protected]>

Author: jfontan

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)
 }

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)
+	}
+}

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)
+}