From d541d40da1f11bdfbd80dc65b191d129c028ea0c Mon Sep 17 00:00:00 2001
From: Máximo Cuadros <mcuadros@gmail.com>
Date: Mon, 28 Nov 2016 10:31:06 +0100
Subject: storage: filesystem, clean close when the packfile is not used (#140)

---
 storage/filesystem/internal/dotgit/writers.go      | 58 ++++++++++++++++------
 storage/filesystem/internal/dotgit/writers_test.go | 21 ++++++++
 2 files changed, 65 insertions(+), 14 deletions(-)

(limited to 'storage/filesystem/internal')

diff --git a/storage/filesystem/internal/dotgit/writers.go b/storage/filesystem/internal/dotgit/writers.go
index d3d3b43..8e22a39 100644
--- a/storage/filesystem/internal/dotgit/writers.go
+++ b/storage/filesystem/internal/dotgit/writers.go
@@ -12,6 +12,12 @@ import (
 	"gopkg.in/src-d/go-git.v4/utils/fs"
 )
 
+// PackWriter is a io.Writer that generates the packfile index simultaneously,
+// a packfile.Decoder is used with a file reader to read the file being written
+// this operation is synchronized with the write operations.
+// The packfile is written in a temp file, when Close is called this file
+// is renamed/moved (depends on the Filesystem implementation) to the final
+// location, if the PackWriter is not used, nothing is written
 type PackWriter struct {
 	Notify func(h plumbing.Hash, i idxfile.Idxfile)
 
@@ -72,34 +78,58 @@ func (w *PackWriter) buildIndex() {
 	w.result <- err
 }
 
-func (w *PackWriter) Write(p []byte) (n int, err error) {
+// waitBuildIndex waits until buildIndex function finishes, this can terminate
+// with a packfile.ErrEmptyPackfile, this means that nothing was written so we
+// ignore the error
+func (w *PackWriter) waitBuildIndex() error {
+	err := <-w.result
+	if err == packfile.ErrEmptyPackfile {
+		return nil
+	}
+
+	return err
+}
+
+func (w *PackWriter) Write(p []byte) (int, error) {
 	return w.synced.Write(p)
 }
 
+// Close closes all the file descriptors and save the final packfile, if nothing
+// was written, the tempfiles are deleted without writing a packfile.
 func (w *PackWriter) Close() error {
 	defer func() {
+		if w.Notify != nil {
+			w.Notify(w.checksum, w.index)
+		}
+
 		close(w.result)
 	}()
 
-	pipe := []func() error{
-		w.synced.Close,
-		func() error { return <-w.result },
-		w.fr.Close,
-		w.fw.Close,
-		w.save,
+	if err := w.synced.Close(); err != nil {
+		return err
 	}
 
-	for _, f := range pipe {
-		if err := f(); err != nil {
-			return err
-		}
+	if err := w.waitBuildIndex(); err != nil {
+		return err
 	}
 
-	if w.Notify != nil {
-		w.Notify(w.checksum, w.index)
+	if err := w.fr.Close(); err != nil {
+		return err
 	}
 
-	return nil
+	if err := w.fw.Close(); err != nil {
+		return err
+	}
+
+	if len(w.index.Entries) == 0 {
+		return w.clean()
+	}
+
+	return w.save()
+}
+
+func (w *PackWriter) clean() error {
+	return w.fs.Remove(w.fw.Filename())
 }
 
 func (w *PackWriter) save() error {
diff --git a/storage/filesystem/internal/dotgit/writers_test.go b/storage/filesystem/internal/dotgit/writers_test.go
index 23d3a0e..c66613a 100644
--- a/storage/filesystem/internal/dotgit/writers_test.go
+++ b/storage/filesystem/internal/dotgit/writers_test.go
@@ -44,6 +44,27 @@ func (s *SuiteDotGit) TestNewObjectPack(c *C) {
 	c.Assert(stat.Size(), Equals, int64(1940))
 }
 
+func (s *SuiteDotGit) TestNewObjectPackUnused(c *C) {
+	dir, err := ioutil.TempDir("", "example")
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	defer os.RemoveAll(dir)
+
+	fs := osfs.New(dir)
+	dot := New(fs)
+
+	w, err := dot.NewObjectPack()
+	c.Assert(err, IsNil)
+
+	c.Assert(w.Close(), IsNil)
+
+	info, err := fs.ReadDir("objects/pack")
+	c.Assert(err, IsNil)
+	c.Assert(info, HasLen, 0)
+}
+
 func (s *SuiteDotGit) TestSyncedReader(c *C) {
 	tmpw, err := ioutil.TempFile("", "example")
 	c.Assert(err, IsNil)
-- 
cgit