Merge pull request #666 from keybase/strib/delete-from-packed-ref

dotgit: handle refs that exist in both packed-refs and a loose ref file

1 files changed, 36 insertions, 6 deletions

diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go
index 0c9cd33..8e5381c 100644
--- a/storage/filesystem/internal/dotgit/dotgit.go
+++ b/storage/filesystem/internal/dotgit/dotgit.go
@@ -268,7 +268,7 @@ func (d *DotGit) Refs() ([]*plumbing.Reference, error) {
 		return nil, err
 	}
 
-	if err := d.addRefsFromPackedRefs(&refs); err != nil {
+	if err := d.addRefsFromPackedRefs(&refs, seen); err != nil {
 		return nil, err
 	}
 
@@ -336,7 +336,8 @@ func (d *DotGit) RemoveRef(name plumbing.ReferenceName) error {
 	path := d.fs.Join(".", name.String())
 	_, err := d.fs.Stat(path)
 	if err == nil {
-		return d.fs.Remove(path)
+		err = d.fs.Remove(path)
+		// Drop down to remove it from the packed refs file, too.
 	}
 
 	if err != nil && !os.IsNotExist(err) {
@@ -346,13 +347,18 @@ func (d *DotGit) RemoveRef(name plumbing.ReferenceName) error {
 	return d.rewritePackedRefsWithoutRef(name)
 }
 
-func (d *DotGit) addRefsFromPackedRefs(refs *[]*plumbing.Reference) (err error) {
+func (d *DotGit) addRefsFromPackedRefs(refs *[]*plumbing.Reference, seen map[plumbing.ReferenceName]bool) (err error) {
 	packedRefs, err := d.findPackedRefs()
 	if err != nil {
 		return err
 	}
 
-	*refs = append(*refs, packedRefs...)
+	for _, ref := range packedRefs {
+		if !seen[ref.Name()] {
+			*refs = append(*refs, ref)
+			seen[ref.Name()] = true
+		}
+	}
 	return nil
 }
 
@@ -365,6 +371,17 @@ func (d *DotGit) rewritePackedRefsWithoutRef(name plumbing.ReferenceName) (err e
 
 		return err
 	}
+	doCloseF := true
+	defer func() {
+		if doCloseF {
+			ioutil.CheckClose(f, &err)
+		}
+	}()
+
+	err = f.Lock()
+	if err != nil {
+		return err
+	}
 
 	// Creating the temp file in the same directory as the target file
 	// improves our chances for rename operation to be atomic.
@@ -372,6 +389,12 @@ func (d *DotGit) rewritePackedRefsWithoutRef(name plumbing.ReferenceName) (err e
 	if err != nil {
 		return err
 	}
+	doCloseTmp := true
+	defer func() {
+		if doCloseTmp {
+			ioutil.CheckClose(tmp, &err)
+		}
+	}()
 
 	s := bufio.NewScanner(f)
 	found := false
@@ -397,14 +420,21 @@ func (d *DotGit) rewritePackedRefsWithoutRef(name plumbing.ReferenceName) (err e
 	}
 
 	if !found {
-		return nil
+		doCloseTmp = false
+		ioutil.CheckClose(tmp, &err)
+		if err != nil {
+			return err
+		}
+		// Delete the temp file if nothing needed to be removed.
+		return d.fs.Remove(tmp.Name())
 	}
 
+	doCloseF = false
 	if err := f.Close(); err != nil {
-		ioutil.CheckClose(tmp, &err)
 		return err
 	}
 
+	doCloseTmp = false
 	if err := tmp.Close(); err != nil {
 		return err
 	}