Merge pull request #700 from jfontan/improvement/support-non-rw-filesystems

Add a setRef and rewritePackedRefsWhileLocked versions that supports non rw fs

6 files changed, 142 insertions, 33 deletions

diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go
index d644810..fac7aec 100644
--- a/storage/filesystem/internal/dotgit/dotgit.go
+++ b/storage/filesystem/internal/dotgit/dotgit.go
@@ -323,36 +323,9 @@ func (d *DotGit) SetRef(r, old *plumbing.Reference) error {
 		content = fmt.Sprintln(r.Hash().String())
 	}
 
-	// If we are not checking an old ref, just truncate the file.
-	mode := os.O_RDWR | os.O_CREATE
-	if old == nil {
-		mode |= os.O_TRUNC
-	}
-
-	f, err := d.fs.OpenFile(r.Name().String(), mode, 0666)
-	if err != nil {
-		return err
-	}
-
-	defer ioutil.CheckClose(f, &err)
-
-	// Lock is unlocked by the deferred Close above. This is because Unlock
-	// does not imply a fsync and thus there would be a race between
-	// Unlock+Close and other concurrent writers. Adding Sync to go-billy
-	// could work, but this is better (and avoids superfluous syncs).
-	err = f.Lock()
-	if err != nil {
-		return err
-	}
-
-	// this is a no-op to call even when old is nil.
-	err = d.checkReferenceAndTruncate(f, old)
-	if err != nil {
-		return err
-	}
+	fileName := r.Name().String()
 
-	_, err = f.Write([]byte(content))
-	return err
+	return d.setRef(fileName, content, old)
 }
 
 // Refs scans the git directory collecting references, which it returns.
@@ -485,7 +458,10 @@ func (d *DotGit) openAndLockPackedRefs(doCreate bool) (
 		}
 	}()
 
-	openFlags := os.O_RDWR
+	// File mode is retrieved from a constant defined in the target specific
+	// files (dotgit_rewrite_packed_refs_*). Some modes are not available
+	// in all filesystems.
+	openFlags := openAndLockPackedRefsMode
 	if doCreate {
 		openFlags |= os.O_CREATE
 	}
diff --git a/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_nix.go b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_nix.go
index af96196..c760793 100644
--- a/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_nix.go
+++ b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_nix.go
@@ -1,8 +1,14 @@
-// +build !windows
+// +build !windows,!norwfs
 
 package dotgit
 
-import "gopkg.in/src-d/go-billy.v4"
+import (
+	"os"
+
+	"gopkg.in/src-d/go-billy.v4"
+)
+
+const openAndLockPackedRefsMode = os.O_RDWR
 
 func (d *DotGit) rewritePackedRefsWhileLocked(
 	tmp billy.File, pr billy.File) error {
diff --git a/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_norwfs.go b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_norwfs.go
new file mode 100644
index 0000000..6e43b42
--- /dev/null
+++ b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_norwfs.go
@@ -0,0 +1,34 @@
+// +build norwfs
+
+package dotgit
+
+import (
+	"io"
+	"os"
+
+	"gopkg.in/src-d/go-billy.v4"
+)
+
+const openAndLockPackedRefsMode = os.O_RDONLY
+
+// Instead of renaming that can not be supported in simpler filesystems
+// a full copy is done.
+func (d *DotGit) rewritePackedRefsWhileLocked(
+	tmp billy.File, pr billy.File) error {
+
+	prWrite, err := d.fs.Create(pr.Name())
+	if err != nil {
+		return err
+	}
+
+	defer prWrite.Close()
+
+	_, err = tmp.Seek(0, io.SeekStart)
+	if err != nil {
+		return err
+	}
+
+	_, err = io.Copy(prWrite, tmp)
+
+	return err
+}
diff --git a/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_windows.go b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_windows.go
index bcdb93e..897d2c9 100644
--- a/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_windows.go
+++ b/storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_windows.go
@@ -1,13 +1,16 @@
-// +build windows
+// +build windows,!norwfs
 
 package dotgit
 
 import (
 	"io"
+	"os"
 
 	"gopkg.in/src-d/go-billy.v4"
 )
 
+const openAndLockPackedRefsMode = os.O_RDWR
+
 func (d *DotGit) rewritePackedRefsWhileLocked(
 	tmp billy.File, pr billy.File) error {
 	// If we aren't using the bare Windows filesystem as the storage
diff --git a/storage/filesystem/internal/dotgit/dotgit_setref.go b/storage/filesystem/internal/dotgit/dotgit_setref.go
new file mode 100644
index 0000000..c732c9f
--- /dev/null
+++ b/storage/filesystem/internal/dotgit/dotgit_setref.go
@@ -0,0 +1,43 @@
+// +build !norwfs
+
+package dotgit
+
+import (
+	"os"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+	"gopkg.in/src-d/go-git.v4/utils/ioutil"
+)
+
+func (d *DotGit) setRef(fileName, content string, old *plumbing.Reference) error {
+	// If we are not checking an old ref, just truncate the file.
+	mode := os.O_RDWR | os.O_CREATE
+	if old == nil {
+		mode |= os.O_TRUNC
+	}
+
+	f, err := d.fs.OpenFile(fileName, mode, 0666)
+	if err != nil {
+		return err
+	}
+
+	defer ioutil.CheckClose(f, &err)
+
+	// Lock is unlocked by the deferred Close above. This is because Unlock
+	// does not imply a fsync and thus there would be a race between
+	// Unlock+Close and other concurrent writers. Adding Sync to go-billy
+	// could work, but this is better (and avoids superfluous syncs).
+	err = f.Lock()
+	if err != nil {
+		return err
+	}
+
+	// this is a no-op to call even when old is nil.
+	err = d.checkReferenceAndTruncate(f, old)
+	if err != nil {
+		return err
+	}
+
+	_, err = f.Write([]byte(content))
+	return err
+}
diff --git a/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go
new file mode 100644
index 0000000..5695bd3
--- /dev/null
+++ b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go
@@ -0,0 +1,47 @@
+// +build norwfs
+
+package dotgit
+
+import (
+	"fmt"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
+)
+
+// There are some filesystems that don't support opening files in RDWD mode.
+// In these filesystems the standard SetRef function can not be used as i
+// reads the reference file to check that it's not modified before updating it.
+//
+// This version of the function writes the reference without extra checks
+// making it compatible with these simple filesystems. This is usually not
+// a problem as they should be accessed by only one process at a time.
+func (d *DotGit) setRef(fileName, content string, old *plumbing.Reference) error {
+	_, err := d.fs.Stat(fileName)
+	if err == nil && old != nil {
+		fRead, err := d.fs.Open(fileName)
+		if err != nil {
+			return err
+		}
+
+		ref, err := d.readReferenceFrom(fRead, old.Name().String())
+		fRead.Close()
+
+		if err != nil {
+			return err
+		}
+
+		if ref.Hash() != old.Hash() {
+			return fmt.Errorf("reference has changed concurrently")
+		}
+	}
+
+	f, err := d.fs.Create(fileName)
+	if err != nil {
+		return err
+	}
+
+	defer f.Close()
+
+	_, err = f.Write([]byte(content))
+	return err
+}