From 8d288404016a90c9b17dd8230db2c5da1a1ccfe5 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 21 Dec 2017 12:30:13 +0100 Subject: Add a setRef version that supports non rw fs There are some filesystems that do not support opening the files in read and write modes at the same time. The method SetRef is split in files with an extra version that only writes the reference. It can be activated with -tags norwfs on building. Signed-off-by: Javi Fontan --- storage/filesystem/internal/dotgit/dotgit.go | 31 +--------------- .../filesystem/internal/dotgit/dotgit_setref.go | 43 ++++++++++++++++++++++ .../internal/dotgit/dotgit_setref_norwfs.go | 17 +++++++++ 3 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 storage/filesystem/internal/dotgit/dotgit_setref.go create mode 100644 storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go (limited to 'storage/filesystem') diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go index d644810..643a5d6 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. 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..b22166c --- /dev/null +++ b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go @@ -0,0 +1,17 @@ +// +build norwfs + +package dotgit + +import "gopkg.in/src-d/go-git.v4/plumbing" + +func (d *DotGit) setRef(fileName, content string, old *plumbing.Reference) error { + f, err := d.fs.Create(fileName) + if err != nil { + return err + } + + defer f.Close() + + _, err = f.Write([]byte(content)) + return err +} -- cgit From dae8c68b753943058aa397947e5c318d8db8cab8 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 21 Dec 2017 13:06:56 +0100 Subject: Add comment to the norwfs version of SetRef Signed-off-by: Javi Fontan --- storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'storage/filesystem') diff --git a/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go index b22166c..8b1e662 100644 --- a/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go +++ b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go @@ -4,6 +4,13 @@ package dotgit import "gopkg.in/src-d/go-git.v4/plumbing" +// There are some filesystems tha 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 { f, err := d.fs.Create(fileName) if err != nil { -- cgit From d3e7c9060a14ac579f899fd01c936319132cbc97 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Fri, 22 Dec 2017 17:16:59 +0100 Subject: Check reference also in norwfs SetRef Signed-off-by: Javi Fontan --- .../internal/dotgit/dotgit_setref_norwfs.go | 27 ++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) (limited to 'storage/filesystem') diff --git a/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go index 8b1e662..5695bd3 100644 --- a/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go +++ b/storage/filesystem/internal/dotgit/dotgit_setref_norwfs.go @@ -2,9 +2,13 @@ package dotgit -import "gopkg.in/src-d/go-git.v4/plumbing" +import ( + "fmt" -// There are some filesystems tha don't support opening files in RDWD mode. + "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. // @@ -12,6 +16,25 @@ import "gopkg.in/src-d/go-git.v4/plumbing" // 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 -- cgit From b869eb17b72e80be8e554864db6b6efa6ecb5ebf Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Fri, 22 Dec 2017 17:34:20 +0100 Subject: Add norwfs version of rewritePackedRefsWhileLocked Signed-off-by: Javi Fontan --- storage/filesystem/internal/dotgit/dotgit.go | 5 +++- .../dotgit/dotgit_rewrite_packed_refs_nix.go | 10 +++++-- .../dotgit/dotgit_rewrite_packed_refs_norwfs.go | 34 ++++++++++++++++++++++ .../dotgit/dotgit_rewrite_packed_refs_windows.go | 5 +++- 4 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 storage/filesystem/internal/dotgit/dotgit_rewrite_packed_refs_norwfs.go (limited to 'storage/filesystem') diff --git a/storage/filesystem/internal/dotgit/dotgit.go b/storage/filesystem/internal/dotgit/dotgit.go index 643a5d6..fac7aec 100644 --- a/storage/filesystem/internal/dotgit/dotgit.go +++ b/storage/filesystem/internal/dotgit/dotgit.go @@ -458,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 -- cgit