Merge pull request #434 from orirawlings/updateLocalRemoteRefsEvenIfNoPackFetched

Update local remote references during fetch even if no pack needs to be received

2 files changed, 69 insertions, 25 deletions

diff --git a/remote.go b/remote.go
index deb8d7f..412eb61 100644
--- a/remote.go
+++ b/remote.go
@@ -47,7 +47,7 @@ func (r *Remote) String() string {
 
 // Fetch fetches references from the remote to the local repository.
 // Returns nil if the operation is successful, NoErrAlreadyUpToDate if there are
-// no changes to be fetched, or an error.
+// no changes to be fetched and no local references to update, or an error.
 func (r *Remote) Fetch(o *FetchOptions) error {
 	_, err := r.fetch(o)
 	return err
@@ -156,24 +156,26 @@ func (r *Remote) fetch(o *FetchOptions) (refs storer.ReferenceStorer, err error)
 	}
 
 	req.Wants, err = getWants(o.RefSpecs, r.s, remoteRefs)
-	if len(req.Wants) == 0 {
-		return remoteRefs, NoErrAlreadyUpToDate
-	}
+	if len(req.Wants) > 0 {
+		req.Haves, err = getHaves(r.s)
+		if err != nil {
+			return nil, err
+		}
 
-	req.Haves, err = getHaves(r.s)
-	if err != nil {
-		return nil, err
+		if err := r.fetchPack(o, s, req); err != nil {
+			return nil, err
+		}
 	}
 
-	if err := r.fetchPack(o, s, req); err != nil {
+	err = r.updateLocalReferenceStorage(o.RefSpecs, remoteRefs)
+	if err != nil && err != NoErrAlreadyUpToDate {
 		return nil, err
 	}
 
-	if err := r.updateLocalReferenceStorage(o.RefSpecs, remoteRefs); err != nil {
-		return nil, err
+	if len(req.Wants) == 0 {
+		return remoteRefs, err
 	}
-
-	return remoteRefs, err
+	return remoteRefs, nil
 }
 
 func newUploadPackSession(url string, auth transport.AuthMethod) (transport.UploadPackSession, error) {
@@ -473,6 +475,7 @@ func buildSidebandIfSupported(l *capability.List, reader io.Reader, p sideband.P
 }
 
 func (r *Remote) updateLocalReferenceStorage(specs []config.RefSpec, refs memory.ReferenceStorage) error {
+	updated := false
 	for _, spec := range specs {
 		for _, ref := range refs {
 			if !spec.Match(ref.Name()) {
@@ -484,38 +487,55 @@ func (r *Remote) updateLocalReferenceStorage(specs []config.RefSpec, refs memory
 			}
 
 			name := spec.Dst(ref.Name())
-			n := plumbing.NewHashReference(name, ref.Hash())
-			if err := r.s.SetReference(n); err != nil {
+			sref, err := r.s.Reference(name)
+			if err != nil && err != plumbing.ErrReferenceNotFound {
 				return err
 			}
+			if err == plumbing.ErrReferenceNotFound || sref.Hash() != ref.Hash() {
+				n := plumbing.NewHashReference(name, ref.Hash())
+				if err := r.s.SetReference(n); err != nil {
+					return err
+				}
+				updated = true
+			}
 		}
 	}
 
-	return r.buildFetchedTags(refs)
-}
-
-func (r *Remote) buildFetchedTags(refs storer.ReferenceStorer) error {
-	iter, err := refs.IterReferences()
-	if err != nil {
+	if err := r.buildFetchedTags(refs); err != nil {
 		return err
 	}
 
-	return iter.ForEach(func(ref *plumbing.Reference) error {
+	if !updated {
+		return NoErrAlreadyUpToDate
+	}
+	return nil
+}
+
+func (r *Remote) buildFetchedTags(refs memory.ReferenceStorage) error {
+	updated := false
+	for _, ref := range refs {
 		if !ref.IsTag() {
-			return nil
+			continue
 		}
 
 		_, err := r.s.EncodedObject(plumbing.AnyObject, ref.Hash())
 		if err == plumbing.ErrObjectNotFound {
-			return nil
+			continue
 		}
 
 		if err != nil {
 			return err
 		}
 
-		return r.s.SetReference(ref)
-	})
+		if err = r.s.SetReference(ref); err != nil {
+			return err
+		}
+		updated = true
+	}
+	if !updated {
+		return NoErrAlreadyUpToDate
+	}
+	return nil
 }
 
 func objectsToPush(commands []*packp.Command) ([]plumbing.Hash, error) {
diff --git a/remote_test.go b/remote_test.go
index 78339d4..4297b81 100644
--- a/remote_test.go
+++ b/remote_test.go
@@ -174,6 +174,30 @@ func (s *RemoteSuite) TestFetchNoErrAlreadyUpToDate(c *C) {
 	s.doTestFetchNoErrAlreadyUpToDate(c, url)
 }
 
+func (s *RemoteSuite) TestFetchNoErrAlreadyUpToDateButStillUpdateLocalRemoteRefs(c *C) {
+	url := s.GetBasicLocalRepositoryURL()
+
+	sto := memory.NewStorage()
+	r := newRemote(sto, &config.RemoteConfig{Name: "foo", URL: url})
+
+	refspec := config.RefSpec("+refs/heads/*:refs/remotes/origin/*")
+	o := &FetchOptions{
+		RefSpecs: []config.RefSpec{refspec},
+	}
+
+	err := r.Fetch(o)
+	c.Assert(err, IsNil)
+
+	// Simulate an out of date remote ref even though we have the new commit locally
+	sto.SetReference(plumbing.NewReferenceFromStrings("refs/remotes/origin/master", "918c48b83bd081e863dbe1b80f8998f058cd8294"))
+
+	err = r.Fetch(o)
+	c.Assert(err, IsNil)
+	exp := plumbing.NewReferenceFromStrings("refs/remotes/origin/master", "6ecf0ef2c2dffb796033e5a02219af86ec6584e5")
+	ref, _ := sto.Reference("refs/remotes/origin/master")
+	c.Assert(exp.String(), Equals, ref.String())
+}
+
 func (s *RemoteSuite) TestFetchNoErrAlreadyUpToDateWithNonCommitObjects(c *C) {
 	fixture := fixtures.ByTag("tags").One()
 	url := s.GetLocalRepositoryURL(fixture)