From e1c269422ab209443b80f05095627c2795e32fd5 Mon Sep 17 00:00:00 2001 From: Mark Bartel Date: Mon, 2 Jul 2018 14:34:22 -0400 Subject: [PATCH 01/30] worktree: add test for correct tree sorting (issue #881) Signed-off-by: Mark Bartel --- worktree_commit_test.go | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/worktree_commit_test.go b/worktree_commit_test.go index 5575bca1a..2100626a4 100644 --- a/worktree_commit_test.go +++ b/worktree_commit_test.go @@ -8,9 +8,15 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/memory" + "bytes" . "gopkg.in/check.v1" "gopkg.in/src-d/go-billy.v4/memfs" + "gopkg.in/src-d/go-billy.v4/osfs" "gopkg.in/src-d/go-billy.v4/util" + "gopkg.in/src-d/go-git.v4/storage/filesystem" + "io/ioutil" + "os" + "os/exec" ) func (s *WorktreeSuite) TestCommitInvalidOptions(c *C) { @@ -135,6 +141,54 @@ func (s *WorktreeSuite) TestRemoveAndCommitAll(c *C) { assertStorageStatus(c, s.Repository, 13, 11, 11, expected) } +func (s *WorktreeSuite) TestCommitTreeSort(c *C) { + path, err := ioutil.TempDir(os.TempDir(), "test-commit-tree-sort") + c.Assert(err, IsNil) + fs := osfs.New(path) + st, err := filesystem.NewStorage(fs) + c.Assert(err, IsNil) + r, err := Init(st, nil) + c.Assert(err, IsNil) + + r, err = Clone(memory.NewStorage(), memfs.New(), &CloneOptions{ + URL: path, + }) + + w, err := r.Worktree() + c.Assert(err, IsNil) + + mfs := w.Filesystem + + err = mfs.MkdirAll("delta", 0755) + c.Assert(err, IsNil) + + for _, p := range []string{"delta_last", "Gamma", "delta/middle", "Beta", "delta-first", "alpha"} { + util.WriteFile(mfs, p, []byte("foo"), 0644) + _, err = w.Add(p) + c.Assert(err, IsNil) + } + + _, err = w.Commit("foo\n", &CommitOptions{ + All: true, + Author: defaultSignature(), + }) + c.Assert(err, IsNil) + + err = r.Push(&PushOptions{}) + c.Assert(err, IsNil) + + cmd := exec.Command("git", "fsck") + cmd.Dir = path + cmd.Env = os.Environ() + buf := &bytes.Buffer{} + cmd.Stderr = buf + cmd.Stdout = buf + + err = cmd.Run() + + c.Assert(err, IsNil, Commentf("%s", buf.Bytes())) +} + func assertStorageStatus( c *C, r *Repository, treesCount, blobCount, commitCount int, head plumbing.Hash, From 7ff71b5a66eea98983c610d33523041a5a829e2f Mon Sep 17 00:00:00 2001 From: Mark Bartel Date: Tue, 3 Jul 2018 17:20:57 -0400 Subject: [PATCH 02/30] worktree: sort the tree object. Fixes #881 Signed-off-by: Mark Bartel --- worktree_commit.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/worktree_commit.go b/worktree_commit.go index 5fa63ab0a..ee5cd8240 100644 --- a/worktree_commit.go +++ b/worktree_commit.go @@ -11,6 +11,7 @@ import ( "gopkg.in/src-d/go-git.v4/storage" "gopkg.in/src-d/go-billy.v4" + "sort" ) // Commit stores the current contents of the index in a new commit along with @@ -162,7 +163,20 @@ func (h *buildTreeHelper) doBuildTree(e *index.Entry, parent, fullpath string) { h.trees[parent].Entries = append(h.trees[parent].Entries, te) } +type sortableEntries []object.TreeEntry + +func (sortableEntries) sortName(te object.TreeEntry) string { + if te.Mode == filemode.Dir { + return te.Name + "/" + } + return te.Name +} +func (se sortableEntries) Len() int { return len(se) } +func (se sortableEntries) Less(i int, j int) bool { return se.sortName(se[i]) < se.sortName(se[j]) } +func (se sortableEntries) Swap(i int, j int) { se[i], se[j] = se[j], se[i] } + func (h *buildTreeHelper) copyTreeToStorageRecursive(parent string, t *object.Tree) (plumbing.Hash, error) { + sort.Sort(sortableEntries(t.Entries)) for i, e := range t.Entries { if e.Mode != filemode.Dir && !e.Hash.IsZero() { continue From f0c4318e7b8d5cbf91723c71a4ba20d1bd0cfaf5 Mon Sep 17 00:00:00 2001 From: Mark Bartel Date: Sat, 7 Jul 2018 23:20:05 -0400 Subject: [PATCH 03/30] worktree: address PR comments: sort imports appropriately Signed-off-by: Mark Bartel --- worktree_commit.go | 2 +- worktree_commit_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/worktree_commit.go b/worktree_commit.go index ee5cd8240..f0e0b4244 100644 --- a/worktree_commit.go +++ b/worktree_commit.go @@ -2,6 +2,7 @@ package git import ( "path" + "sort" "strings" "gopkg.in/src-d/go-git.v4/plumbing" @@ -11,7 +12,6 @@ import ( "gopkg.in/src-d/go-git.v4/storage" "gopkg.in/src-d/go-billy.v4" - "sort" ) // Commit stores the current contents of the index in a new commit along with diff --git a/worktree_commit_test.go b/worktree_commit_test.go index 2100626a4..5ca9b511a 100644 --- a/worktree_commit_test.go +++ b/worktree_commit_test.go @@ -1,22 +1,22 @@ package git import ( + "bytes" + "io/ioutil" + "os" + "os/exec" "time" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/memory" + "gopkg.in/src-d/go-git.v4/storage/filesystem" - "bytes" . "gopkg.in/check.v1" "gopkg.in/src-d/go-billy.v4/memfs" "gopkg.in/src-d/go-billy.v4/osfs" "gopkg.in/src-d/go-billy.v4/util" - "gopkg.in/src-d/go-git.v4/storage/filesystem" - "io/ioutil" - "os" - "os/exec" ) func (s *WorktreeSuite) TestCommitInvalidOptions(c *C) { From ae5501623169ec981091a5f1cfb56ab8e7688031 Mon Sep 17 00:00:00 2001 From: noxora Date: Tue, 14 Aug 2018 14:02:26 -0500 Subject: [PATCH 04/30] added hook support Signed-off-by: noxora trying a possible fix to the delete test Signed-off-by: noxora still trying to fix this test Signed-off-by: noxora fixes did not work, seems to be a windows env problem Signed-off-by: noxora --- storage/filesystem/dotgit/dotgit.go | 53 ++++++++++++++++++++++-- storage/filesystem/dotgit/dotgit_test.go | 49 +++++++++++++++++++++- 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index dc12f23cf..d16a77daf 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -279,19 +279,66 @@ func (d *DotGit) objectPath(h plumbing.Hash) string { return d.fs.Join(objectsPath, hash[0:2], hash[2:40]) } +//incomingObjectPath is intended to add support for a git pre-recieve hook to be written +//it adds support for go-git to find objects in an "incoming" directory, so that the library +//can be used to write a pre-recieve hook that deals with the incoming objects. +//More on git hooks found here : https://git-scm.com/docs/githooks +//More on 'quarantine'/incoming directory here : https://git-scm.com/docs/git-receive-pack +func (d *DotGit) incomingObjectPath(h plumbing.Hash) string { + hString := h.String() + directoryContents, err := d.fs.ReadDir(objectsPath) + if err != nil { + return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) + } + var incomingDirName string + for _, file := range directoryContents { + if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() { + incomingDirName = file.Name() + } + } + if incomingDirName == "" { + return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) + } + return d.fs.Join(objectsPath, incomingDirName, hString[0:2], hString[2:40]) +} + // Object returns a fs.File pointing the object file, if exists func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { - return d.fs.Open(d.objectPath(h)) + obj1, err1 := d.fs.Open(d.objectPath(h)) + if os.IsNotExist(err1) { + obj2, err2 := d.fs.Open(d.incomingObjectPath(h)) + if err2 != nil { + return obj1, err1 + } + return obj2, err2 + } + return obj1, err1 } // ObjectStat returns a os.FileInfo pointing the object file, if exists func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { - return d.fs.Stat(d.objectPath(h)) + obj1, err1 := d.fs.Stat(d.objectPath(h)) + if os.IsNotExist(err1) { + obj2, err2 := d.fs.Stat(d.incomingObjectPath(h)) + if err2 != nil { + return obj1, err1 + } + return obj2, err2 + } + return obj1, err1 } // ObjectDelete removes the object file, if exists func (d *DotGit) ObjectDelete(h plumbing.Hash) error { - return d.fs.Remove(d.objectPath(h)) + err1 := d.fs.Remove(d.objectPath(h)) + if os.IsNotExist(err1) { + err2 := d.fs.Remove(d.incomingObjectPath(h)) + if err2 != nil { + return err1 + } + return err2 + } + return err1 } func (d *DotGit) readReferenceFrom(rd io.Reader, name string) (ref *plumbing.Reference, err error) { diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index 7733eef78..ce3050039 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -9,11 +9,11 @@ import ( "strings" "testing" + fixtures "gopkg.in/src-d/go-git-fixtures.v3" "gopkg.in/src-d/go-git.v4/plumbing" . "gopkg.in/check.v1" "gopkg.in/src-d/go-billy.v4/osfs" - "gopkg.in/src-d/go-git-fixtures.v3" ) func Test(t *testing.T) { TestingT(t) } @@ -537,6 +537,53 @@ func (s *SuiteDotGit) TestObject(c *C) { file.Name(), fs.Join("objects", "03", "db8e1fbe133a480f2867aac478fd866686d69e")), Equals, true, ) + incomingHash := "9d25e0f9bde9f82882b49fe29117b9411cb157b7" //made up hash + incomingDirPath := fs.Join("objects", "incoming-123456") + incomingFilePath := fs.Join(incomingDirPath, incomingHash[0:2], incomingHash[2:40]) + fs.MkdirAll(incomingDirPath, os.FileMode(0755)) + fs.Create(incomingFilePath) + + file, err = dir.Object(plumbing.NewHash(incomingHash)) + c.Assert(err, IsNil) +} + +func (s *SuiteDotGit) TestObjectStat(c *C) { + fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit() + dir := New(fs) + + hash := plumbing.NewHash("03db8e1fbe133a480f2867aac478fd866686d69e") + _, err := dir.ObjectStat(hash) + c.Assert(err, IsNil) + incomingHash := "9d25e0f9bde9f82882b49fe29117b9411cb157b7" //made up hash + incomingDirPath := fs.Join("objects", "incoming-123456") + incomingFilePath := fs.Join(incomingDirPath, incomingHash[0:2], incomingHash[2:40]) + fs.MkdirAll(incomingDirPath, os.FileMode(0755)) + fs.Create(incomingFilePath) + + _, err = dir.ObjectStat(plumbing.NewHash(incomingHash)) + c.Assert(err, IsNil) +} + +func (s *SuiteDotGit) TestObjectDelete(c *C) { + fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit() + dir := New(fs) + + hash := plumbing.NewHash("03db8e1fbe133a480f2867aac478fd866686d69e") + err := dir.ObjectDelete(hash) + c.Assert(err, IsNil) + //incomingHash := "9d25e0f9bde9f82882b49fe29117b9411cb157b7" //made up hash + //incomingDirPath := fs.Join("objects", "incoming-123456") + //incomingSubDirPath := fs.Join(incomingDirPath, incomingHash[0:2]) + //incomingFilePath := fs.Join(incomingDirPath, incomingHash[2:40]) + //err = fs.MkdirAll(incomingDirPath, os.FileMode(0755)) + //c.Assert(err, IsNil) + //err = fs.MkdirAll(incomingSubDirPath, os.FileMode(0755)) + //c.Assert(err, IsNil) + //_, err = fs.Create(incomingFilePath) + //c.Assert(err, IsNil) + + //err = dir.ObjectDelete(plumbing.NewHash(incomingHash)) + //c.Assert(err, IsNil) } func (s *SuiteDotGit) TestObjectNotFound(c *C) { From 8cf7edbc99282245bc8803a322dbf499ab77575d Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Fri, 17 Aug 2018 12:18:06 +0200 Subject: [PATCH 05/30] dotgit: fix object delete test Signed-off-by: Santiago M. Mola --- storage/filesystem/dotgit/dotgit_test.go | 32 +++++++++++++----------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index ce3050039..64c2aeead 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -9,11 +9,11 @@ import ( "strings" "testing" - fixtures "gopkg.in/src-d/go-git-fixtures.v3" "gopkg.in/src-d/go-git.v4/plumbing" . "gopkg.in/check.v1" "gopkg.in/src-d/go-billy.v4/osfs" + "gopkg.in/src-d/go-git-fixtures.v3" ) func Test(t *testing.T) { TestingT(t) } @@ -571,19 +571,23 @@ func (s *SuiteDotGit) TestObjectDelete(c *C) { hash := plumbing.NewHash("03db8e1fbe133a480f2867aac478fd866686d69e") err := dir.ObjectDelete(hash) c.Assert(err, IsNil) - //incomingHash := "9d25e0f9bde9f82882b49fe29117b9411cb157b7" //made up hash - //incomingDirPath := fs.Join("objects", "incoming-123456") - //incomingSubDirPath := fs.Join(incomingDirPath, incomingHash[0:2]) - //incomingFilePath := fs.Join(incomingDirPath, incomingHash[2:40]) - //err = fs.MkdirAll(incomingDirPath, os.FileMode(0755)) - //c.Assert(err, IsNil) - //err = fs.MkdirAll(incomingSubDirPath, os.FileMode(0755)) - //c.Assert(err, IsNil) - //_, err = fs.Create(incomingFilePath) - //c.Assert(err, IsNil) - - //err = dir.ObjectDelete(plumbing.NewHash(incomingHash)) - //c.Assert(err, IsNil) + + incomingHash := "9d25e0f9bde9f82882b49fe29117b9411cb157b7" //made up hash + incomingDirPath := fs.Join("objects", "incoming-123456") + incomingSubDirPath := fs.Join(incomingDirPath, incomingHash[0:2]) + incomingFilePath := fs.Join(incomingSubDirPath, incomingHash[2:40]) + + err = fs.MkdirAll(incomingSubDirPath, os.FileMode(0755)) + c.Assert(err, IsNil) + + f, err := fs.Create(incomingFilePath) + c.Assert(err, IsNil) + + err = f.Close() + c.Assert(err, IsNil) + + err = dir.ObjectDelete(plumbing.NewHash(incomingHash)) + c.Assert(err, IsNil) } func (s *SuiteDotGit) TestObjectNotFound(c *C) { From 166623633e285e17b0582443c9d03b842b6370fa Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Fri, 17 Aug 2018 18:52:18 +0200 Subject: [PATCH 06/30] object: fix panic when reading object header When the first line of the pgp signature is an empty line or some header is malformed it crashes as there's no data for the header element. For example, if author name is "\n". Signed-off-by: Javi Fontan --- plumbing/object/commit.go | 16 +++++++++++----- plumbing/object/commit_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/plumbing/object/commit.go b/plumbing/object/commit.go index 00ae3f144..e2543426a 100644 --- a/plumbing/object/commit.go +++ b/plumbing/object/commit.go @@ -199,17 +199,23 @@ func (c *Commit) Decode(o plumbing.EncodedObject) (err error) { } split := bytes.SplitN(line, []byte{' '}, 2) + + var data []byte + if len(split) == 2 { + data = split[1] + } + switch string(split[0]) { case "tree": - c.TreeHash = plumbing.NewHash(string(split[1])) + c.TreeHash = plumbing.NewHash(string(data)) case "parent": - c.ParentHashes = append(c.ParentHashes, plumbing.NewHash(string(split[1]))) + c.ParentHashes = append(c.ParentHashes, plumbing.NewHash(string(data))) case "author": - c.Author.Decode(split[1]) + c.Author.Decode(data) case "committer": - c.Committer.Decode(split[1]) + c.Committer.Decode(data) case headerpgp: - c.PGPSignature += string(split[1]) + "\n" + c.PGPSignature += string(data) + "\n" pgpsig = true } } else { diff --git a/plumbing/object/commit_test.go b/plumbing/object/commit_test.go index b5dfbe325..e72b703d1 100644 --- a/plumbing/object/commit_test.go +++ b/plumbing/object/commit_test.go @@ -325,6 +325,22 @@ RUysgqjcpT8+iQM1PblGfHR4XAhuOqN5Fx06PSaFZhqvWFezJ28/CLyX5q+oIVk= c.Assert(err, IsNil) c.Assert(decoded.PGPSignature, Equals, pgpsignature) + // signature with extra empty line, it caused "index out of range" when + // parsing it + + pgpsignature2 := "\n" + pgpsignature + + commit.PGPSignature = pgpsignature2 + encoded = &plumbing.MemoryObject{} + decoded = &Commit{} + + err = commit.Encode(encoded) + c.Assert(err, IsNil) + + err = decoded.Decode(encoded) + c.Assert(err, IsNil) + c.Assert(decoded.PGPSignature, Equals, pgpsignature2) + // signature in author name commit.PGPSignature = "" @@ -461,3 +477,21 @@ func (s *SuiteCommit) TestPatchCancel(c *C) { c.Assert(err, ErrorMatches, "operation canceled") } + +func (s *SuiteCommit) TestMalformedHeader(c *C) { + encoded := &plumbing.MemoryObject{} + decoded := &Commit{} + commit := *s.Commit + + commit.PGPSignature = "\n" + commit.Author.Name = "\n" + commit.Author.Email = "\n" + commit.Committer.Name = "\n" + commit.Committer.Email = "\n" + + err := commit.Encode(encoded) + c.Assert(err, IsNil) + + err = decoded.Decode(encoded) + c.Assert(err, IsNil) +} From 7b4a8379327653167e87e9e46a2d397f8fd9cfc8 Mon Sep 17 00:00:00 2001 From: Fedor Korotkov Date: Sun, 19 Aug 2018 11:51:21 -0400 Subject: [PATCH 07/30] Fixed an edge case for .gitignore Fixes #923 Signed-off-by: Fedor Korotkov --- plumbing/format/gitignore/pattern.go | 3 +++ plumbing/format/gitignore/pattern_test.go | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/plumbing/format/gitignore/pattern.go b/plumbing/format/gitignore/pattern.go index 26033522b..098cb5021 100644 --- a/plumbing/format/gitignore/pattern.go +++ b/plumbing/format/gitignore/pattern.go @@ -133,6 +133,9 @@ func (p *pattern) globMatch(path []string, isDir bool) bool { } else if match { matched = true break + } else if len(path) == 0 { + // if nothing left then fail + matched = false } } } else { diff --git a/plumbing/format/gitignore/pattern_test.go b/plumbing/format/gitignore/pattern_test.go index f94cef37a..c410442b6 100644 --- a/plumbing/format/gitignore/pattern_test.go +++ b/plumbing/format/gitignore/pattern_test.go @@ -281,3 +281,9 @@ func (s *PatternSuite) TestGlobMatch_wrongPattern_onTraversal_mismatch(c *C) { r := p.Match([]string{"value", "head", "vol["}, false) c.Assert(r, Equals, NoMatch) } + +func (s *PatternSuite) TestGlobMatch_issue_923(c *C) { + p := ParsePattern("**/android/**/GeneratedPluginRegistrant.java", nil) + r := p.Match([]string{"packages", "flutter_tools", "lib", "src", "android", "gradle.dart"}, false) + c.Assert(r, Equals, NoMatch) +} From f84c6b194f2eced0c068b9cdc9264d30e6d2021b Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Tue, 21 Aug 2018 17:35:52 +0200 Subject: [PATCH 08/30] plumbing/idxfile: object iterators returns entries in offset order In the latest change the order was changed from offset order in packfiles to hash order. This makes reading all the objects not as efficient as before. It also created problems when the previous order was expected. Also added EntriesByOffset to indexes. Signed-off-by: Javi Fontan --- plumbing/format/idxfile/idxfile.go | 69 +++++++++++++++++++++++++ plumbing/format/idxfile/idxfile_test.go | 15 ++++++ plumbing/format/packfile/packfile.go | 2 +- 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/plumbing/format/idxfile/idxfile.go b/plumbing/format/idxfile/idxfile.go index c977beedb..5fed278b1 100644 --- a/plumbing/format/idxfile/idxfile.go +++ b/plumbing/format/idxfile/idxfile.go @@ -3,6 +3,7 @@ package idxfile import ( "bytes" "io" + "sort" "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/utils/binary" @@ -34,6 +35,9 @@ type Index interface { Count() (int64, error) // Entries returns an iterator to retrieve all index entries. Entries() (EntryIter, error) + // EntriesByOffset returns an iterator to retrieve all index entries ordered + // by offset. + EntriesByOffset() (EntryIter, error) } // MemoryIndex is the in memory representation of an idx file. @@ -215,6 +219,36 @@ func (idx *MemoryIndex) Entries() (EntryIter, error) { return &idxfileEntryIter{idx, 0, 0, 0}, nil } +// EntriesByOffset implements the Index interface. +func (idx *MemoryIndex) EntriesByOffset() (EntryIter, error) { + count, err := idx.Count() + if err != nil { + return nil, err + } + + iter := &idxfileEntryOffsetIter{ + entries: make(entriesByOffset, count), + } + + entries, err := idx.Entries() + if err != nil { + return nil, err + } + + for pos := 0; int64(pos) < count; pos++ { + entry, err := entries.Next() + if err != nil { + return nil, err + } + + iter.entries[pos] = entry + } + + sort.Sort(iter.entries) + + return iter, nil +} + // EntryIter is an iterator that will return the entries in a packfile index. type EntryIter interface { // Next returns the next entry in the packfile index. @@ -276,3 +310,38 @@ type Entry struct { CRC32 uint32 Offset uint64 } + +type idxfileEntryOffsetIter struct { + entries entriesByOffset + pos int +} + +func (i *idxfileEntryOffsetIter) Next() (*Entry, error) { + if i.pos >= len(i.entries) { + return nil, io.EOF + } + + entry := i.entries[i.pos] + i.pos++ + + return entry, nil +} + +func (i *idxfileEntryOffsetIter) Close() error { + i.pos = len(i.entries) + 1 + return nil +} + +type entriesByOffset []*Entry + +func (o entriesByOffset) Len() int { + return len(o) +} + +func (o entriesByOffset) Less(i int, j int) bool { + return o[i].Offset < o[j].Offset +} + +func (o entriesByOffset) Swap(i int, j int) { + o[i], o[j] = o[j], o[i] +} diff --git a/plumbing/format/idxfile/idxfile_test.go b/plumbing/format/idxfile/idxfile_test.go index d15accf3a..0e0ca2a22 100644 --- a/plumbing/format/idxfile/idxfile_test.go +++ b/plumbing/format/idxfile/idxfile_test.go @@ -115,6 +115,21 @@ func (s *IndexSuite) TestFindHash(c *C) { } } +func (s *IndexSuite) TestEntriesByOffset(c *C) { + idx, err := fixtureIndex() + c.Assert(err, IsNil) + + entries, err := idx.EntriesByOffset() + c.Assert(err, IsNil) + + for _, pos := range fixtureOffsets { + e, err := entries.Next() + c.Assert(err, IsNil) + + c.Assert(e.Offset, Equals, uint64(pos)) + } +} + var fixtureHashes = []plumbing.Hash{ plumbing.NewHash("303953e5aa461c203a324821bc1717f9b4fff895"), plumbing.NewHash("5296768e3d9f661387ccbff18c4dea6c997fd78c"), diff --git a/plumbing/format/packfile/packfile.go b/plumbing/format/packfile/packfile.go index 5feb78142..18fcca7e8 100644 --- a/plumbing/format/packfile/packfile.go +++ b/plumbing/format/packfile/packfile.go @@ -394,7 +394,7 @@ func (p *Packfile) GetByType(typ plumbing.ObjectType) (storer.EncodedObjectIter, plumbing.TreeObject, plumbing.CommitObject, plumbing.TagObject: - entries, err := p.Entries() + entries, err := p.EntriesByOffset() if err != nil { return nil, err } From 790191ef92ec6382ce65cc30286c901863b3b7a3 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Wed, 22 Aug 2018 16:46:50 +0200 Subject: [PATCH 09/30] plumbing, storage: add bases to the common cache After clone only resolved deltas were added to the cache. This caused slowdowns in small repositories where most objects can be held in cache. It also makes packfiles reuse delta cache from the store. Previously it created a new delta cache each time a packfile object was created. This also slowed down a bit accessing objects and had an impact on memory consumption when bases are added to the cache. Signed-off-by: Javi Fontan --- plumbing/format/packfile/fsobject.go | 10 ++++++++++ plumbing/format/packfile/packfile.go | 15 +++++++++++++++ storage/filesystem/object.go | 18 ++++++++++++++++-- 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/plumbing/format/packfile/fsobject.go b/plumbing/format/packfile/fsobject.go index 6fd3ca54d..330cb73c9 100644 --- a/plumbing/format/packfile/fsobject.go +++ b/plumbing/format/packfile/fsobject.go @@ -47,6 +47,16 @@ func NewFSObject( // Reader implements the plumbing.EncodedObject interface. func (o *FSObject) Reader() (io.ReadCloser, error) { + obj, ok := o.cache.Get(o.hash) + if ok { + reader, err := obj.Reader() + if err != nil { + return nil, err + } + + return reader, nil + } + f, err := o.fs.Open(o.path) if err != nil { return nil, err diff --git a/plumbing/format/packfile/packfile.go b/plumbing/format/packfile/packfile.go index 18fcca7e8..852a8344b 100644 --- a/plumbing/format/packfile/packfile.go +++ b/plumbing/format/packfile/packfile.go @@ -258,6 +258,19 @@ func (p *Packfile) nextObject() (plumbing.EncodedObject, error) { } func (p *Packfile) getObjectContent(offset int64) (io.ReadCloser, error) { + ref, err := p.FindHash(offset) + if err == nil { + obj, ok := p.cacheGet(ref) + if ok { + reader, err := obj.Reader() + if err != nil { + return nil, err + } + + return reader, nil + } + } + if _, err := p.s.SeekFromStart(offset); err != nil { return nil, err } @@ -306,6 +319,8 @@ func (p *Packfile) fillRegularObjectContent(obj plumbing.EncodedObject) error { } _, _, err = p.s.NextObject(w) + p.cachePut(obj) + return err } diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 6958e3217..3a3a2bd97 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -295,7 +295,14 @@ func (s *ObjectStorage) decodeObjectAt( return nil, err } - return packfile.NewPackfile(idx, s.dir.Fs(), f).GetByOffset(offset) + var p *packfile.Packfile + if s.deltaBaseCache != nil { + p = packfile.NewPackfileWithCache(idx, s.dir.Fs(), f, s.deltaBaseCache) + } else { + p = packfile.NewPackfile(idx, s.dir.Fs(), f) + } + + return p.GetByOffset(offset) } func (s *ObjectStorage) decodeDeltaObjectAt( @@ -486,7 +493,14 @@ func newPackfileIter( index idxfile.Index, cache cache.Object, ) (storer.EncodedObjectIter, error) { - iter, err := packfile.NewPackfile(index, fs, f).GetByType(t) + var p *packfile.Packfile + if cache != nil { + p = packfile.NewPackfileWithCache(index, fs, f, cache) + } else { + p = packfile.NewPackfile(index, fs, f) + } + + iter, err := p.GetByType(t) if err != nil { return nil, err } From c7a4011d78a00bd93a2f82a39bb67c2dda5453f5 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Sat, 25 Aug 2018 19:17:45 +0200 Subject: [PATCH 10/30] storage/dotgit: search for incoming dir only once Search for incoming object directory was done once each time objects were accessed. This means a ReadDir of the objects path that is expensive. Now incoming directory is searched the first time an object is accessed and its name kept in DotGit to be reused. Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit.go | 41 ++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index d0a14aebe..addb64c91 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -61,6 +61,10 @@ var ( // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { fs billy.Filesystem + + // incoming object directory information + incomingChecked bool + incomingDirName string } // New returns a DotGit value ready to be used. The path argument must @@ -286,26 +290,37 @@ func (d *DotGit) objectPath(h plumbing.Hash) string { //More on 'quarantine'/incoming directory here : https://git-scm.com/docs/git-receive-pack func (d *DotGit) incomingObjectPath(h plumbing.Hash) string { hString := h.String() - directoryContents, err := d.fs.ReadDir(objectsPath) - if err != nil { + + if d.incomingDirName == "" { return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) } - var incomingDirName string - for _, file := range directoryContents { - if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() { - incomingDirName = file.Name() + + return d.fs.Join(objectsPath, d.incomingDirName, hString[0:2], hString[2:40]) +} + +// hasIncomingObjects searches for an incoming directory and keeps its name +// so it doesn't have to be found each time an object is accessed. +func (d *DotGit) hasIncomingObjects() bool { + if !d.incomingChecked { + directoryContents, err := d.fs.ReadDir(objectsPath) + if err == nil { + for _, file := range directoryContents { + if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() { + d.incomingDirName = file.Name() + } + } } + + d.incomingChecked = true } - if incomingDirName == "" { - return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) - } - return d.fs.Join(objectsPath, incomingDirName, hString[0:2], hString[2:40]) + + return d.incomingDirName != "" } // Object returns a fs.File pointing the object file, if exists func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { obj1, err1 := d.fs.Open(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Open(d.incomingObjectPath(h)) if err2 != nil { return obj1, err1 @@ -318,7 +333,7 @@ func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { // ObjectStat returns a os.FileInfo pointing the object file, if exists func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { obj1, err1 := d.fs.Stat(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Stat(d.incomingObjectPath(h)) if err2 != nil { return obj1, err1 @@ -331,7 +346,7 @@ func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { // ObjectDelete removes the object file, if exists func (d *DotGit) ObjectDelete(h plumbing.Hash) error { err1 := d.fs.Remove(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { err2 := d.fs.Remove(d.incomingObjectPath(h)) if err2 != nil { return err1 From b1da90b0dde34b521cb252bc28c59e4ffd840d1d Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Mon, 27 Aug 2018 14:45:24 +0200 Subject: [PATCH 11/30] storage/dotgit: use HasPrefix instead of Split Also reformatted function comment and fixed some typos. Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index addb64c91..df4f75691 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -283,11 +283,14 @@ func (d *DotGit) objectPath(h plumbing.Hash) string { return d.fs.Join(objectsPath, hash[0:2], hash[2:40]) } -//incomingObjectPath is intended to add support for a git pre-recieve hook to be written -//it adds support for go-git to find objects in an "incoming" directory, so that the library -//can be used to write a pre-recieve hook that deals with the incoming objects. -//More on git hooks found here : https://git-scm.com/docs/githooks -//More on 'quarantine'/incoming directory here : https://git-scm.com/docs/git-receive-pack +// incomingObjectPath is intended to add support for a git pre-receive hook +// to be written it adds support for go-git to find objects in an "incoming" +// directory, so that the library can be used to write a pre-receive hook +// that deals with the incoming objects. +// +// More on git hooks found here : https://git-scm.com/docs/githooks +// More on 'quarantine'/incoming directory here: +// https://git-scm.com/docs/git-receive-pack func (d *DotGit) incomingObjectPath(h plumbing.Hash) string { hString := h.String() @@ -305,7 +308,7 @@ func (d *DotGit) hasIncomingObjects() bool { directoryContents, err := d.fs.ReadDir(objectsPath) if err == nil { for _, file := range directoryContents { - if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() { + if strings.HasPrefix(file.Name(), "incoming-") && file.IsDir() { d.incomingDirName = file.Name() } } From 0167dabb78412ed5fb76cb4b174a6708c3be52b8 Mon Sep 17 00:00:00 2001 From: kuba-- Date: Fri, 24 Aug 2018 23:44:44 +0200 Subject: [PATCH 12/30] Remove empty dirs when cleaning with Dir opt. Signed-off-by: kuba-- --- storage/filesystem/dotgit/dotgit.go | 54 ++++++++++++++++++--------- worktree.go | 58 ++++++++++++++++++++++------- worktree_test.go | 9 +++++ 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index d0a14aebe..df4f75691 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -61,6 +61,10 @@ var ( // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { fs billy.Filesystem + + // incoming object directory information + incomingChecked bool + incomingDirName string } // New returns a DotGit value ready to be used. The path argument must @@ -279,33 +283,47 @@ func (d *DotGit) objectPath(h plumbing.Hash) string { return d.fs.Join(objectsPath, hash[0:2], hash[2:40]) } -//incomingObjectPath is intended to add support for a git pre-recieve hook to be written -//it adds support for go-git to find objects in an "incoming" directory, so that the library -//can be used to write a pre-recieve hook that deals with the incoming objects. -//More on git hooks found here : https://git-scm.com/docs/githooks -//More on 'quarantine'/incoming directory here : https://git-scm.com/docs/git-receive-pack +// incomingObjectPath is intended to add support for a git pre-receive hook +// to be written it adds support for go-git to find objects in an "incoming" +// directory, so that the library can be used to write a pre-receive hook +// that deals with the incoming objects. +// +// More on git hooks found here : https://git-scm.com/docs/githooks +// More on 'quarantine'/incoming directory here: +// https://git-scm.com/docs/git-receive-pack func (d *DotGit) incomingObjectPath(h plumbing.Hash) string { hString := h.String() - directoryContents, err := d.fs.ReadDir(objectsPath) - if err != nil { + + if d.incomingDirName == "" { return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) } - var incomingDirName string - for _, file := range directoryContents { - if strings.Split(file.Name(), "-")[0] == "incoming" && file.IsDir() { - incomingDirName = file.Name() + + return d.fs.Join(objectsPath, d.incomingDirName, hString[0:2], hString[2:40]) +} + +// hasIncomingObjects searches for an incoming directory and keeps its name +// so it doesn't have to be found each time an object is accessed. +func (d *DotGit) hasIncomingObjects() bool { + if !d.incomingChecked { + directoryContents, err := d.fs.ReadDir(objectsPath) + if err == nil { + for _, file := range directoryContents { + if strings.HasPrefix(file.Name(), "incoming-") && file.IsDir() { + d.incomingDirName = file.Name() + } + } } + + d.incomingChecked = true } - if incomingDirName == "" { - return d.fs.Join(objectsPath, hString[0:2], hString[2:40]) - } - return d.fs.Join(objectsPath, incomingDirName, hString[0:2], hString[2:40]) + + return d.incomingDirName != "" } // Object returns a fs.File pointing the object file, if exists func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { obj1, err1 := d.fs.Open(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Open(d.incomingObjectPath(h)) if err2 != nil { return obj1, err1 @@ -318,7 +336,7 @@ func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { // ObjectStat returns a os.FileInfo pointing the object file, if exists func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { obj1, err1 := d.fs.Stat(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Stat(d.incomingObjectPath(h)) if err2 != nil { return obj1, err1 @@ -331,7 +349,7 @@ func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { // ObjectDelete removes the object file, if exists func (d *DotGit) ObjectDelete(h plumbing.Hash) error { err1 := d.fs.Remove(d.objectPath(h)) - if os.IsNotExist(err1) { + if os.IsNotExist(err1) && d.hasIncomingObjects() { err2 := d.fs.Remove(d.incomingObjectPath(h)) if err2 != nil { return err1 diff --git a/worktree.go b/worktree.go index 99b2cd124..921e600a0 100644 --- a/worktree.go +++ b/worktree.go @@ -713,29 +713,56 @@ func (w *Worktree) readGitmodulesFile() (*config.Modules, error) { } // Clean the worktree by removing untracked files. +// An empty dir could be removed - this is what `git clean -f -d .` does. func (w *Worktree) Clean(opts *CleanOptions) error { s, err := w.Status() if err != nil { return err } - // Check Worktree status to be Untracked, obtain absolute path and delete. - for relativePath, status := range s { - // Check if the path contains a directory and if Dir options is false, - // skip the path. - if relativePath != filepath.Base(relativePath) && !opts.Dir { + root := "" + files, err := w.Filesystem.ReadDir(root) + if err != nil { + return err + } + return w.doClean(s, opts, root, files) +} + +func (w *Worktree) doClean(status Status, opts *CleanOptions, dir string, files []os.FileInfo) error { + for _, fi := range files { + if fi.Name() == ".git" { continue } - // Remove the file only if it's an untracked file. - if status.Worktree == Untracked { - absPath := filepath.Join(w.Filesystem.Root(), relativePath) - if err := os.Remove(absPath); err != nil { + // relative path under the root + path := filepath.Join(dir, fi.Name()) + if fi.IsDir() { + if !opts.Dir { + continue + } + + subfiles, err := w.Filesystem.ReadDir(path) + if err != nil { + return err + } + err = w.doClean(status, opts, path, subfiles) + if err != nil { return err } + } else { + // check if file is 'Untracked' + s, ok := (status)[filepath.ToSlash(path)] + if ok && s.Worktree == Untracked { + if err := w.Filesystem.Remove(path); err != nil { + return err + } + } } } + if opts.Dir { + return doCleanDirectories(w.Filesystem, dir) + } return nil } @@ -881,15 +908,18 @@ func rmFileAndDirIfEmpty(fs billy.Filesystem, name string) error { return err } - path := filepath.Dir(name) - files, err := fs.ReadDir(path) + dir := filepath.Dir(name) + return doCleanDirectories(fs, dir) +} + +// doCleanDirectories removes empty subdirs (without files) +func doCleanDirectories(fs billy.Filesystem, dir string) error { + files, err := fs.ReadDir(dir) if err != nil { return err } - if len(files) == 0 { - fs.Remove(path) + return fs.Remove(dir) } - return nil } diff --git a/worktree_test.go b/worktree_test.go index df191b0a0..c714011c0 100644 --- a/worktree_test.go +++ b/worktree_test.go @@ -1591,6 +1591,10 @@ func (s *WorktreeSuite) TestClean(c *C) { c.Assert(len(status), Equals, 1) + fi, err := fs.Lstat("pkgA") + c.Assert(err, IsNil) + c.Assert(fi.IsDir(), Equals, true) + // Clean with Dir: true. err = wt.Clean(&CleanOptions{Dir: true}) c.Assert(err, IsNil) @@ -1599,6 +1603,11 @@ func (s *WorktreeSuite) TestClean(c *C) { c.Assert(err, IsNil) c.Assert(len(status), Equals, 0) + + // An empty dir should be deleted, as well. + _, err = fs.Lstat("pkgA") + c.Assert(err, ErrorMatches, ".*(no such file or directory.*|.*file does not exist)*.") + } func (s *WorktreeSuite) TestAlternatesRepo(c *C) { From 75fa41d21c8d27ee0d5d7c7cb7ceeb2b765be330 Mon Sep 17 00:00:00 2001 From: kuba-- Date: Wed, 29 Aug 2018 14:56:25 +0200 Subject: [PATCH 13/30] Add Status.IsUntracked function Signed-off-by: kuba-- --- status.go | 13 +++++++++++-- worktree.go | 4 +--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/status.go b/status.go index ef8a500f2..ecbf79350 100644 --- a/status.go +++ b/status.go @@ -1,7 +1,10 @@ package git -import "fmt" -import "bytes" +import ( + "bytes" + "fmt" + "path/filepath" +) // Status represents the current status of a Worktree. // The key of the map is the path of the file. @@ -17,6 +20,12 @@ func (s Status) File(path string) *FileStatus { return s[path] } +// IsUntracked checks if file for given path is 'Untracked' +func (s Status) IsUntracked(path string) bool { + stat, ok := (s)[filepath.ToSlash(path)] + return ok && stat.Worktree == Untracked +} + // IsClean returns true if all the files aren't in Unmodified status. func (s Status) IsClean() bool { for _, status := range s { diff --git a/worktree.go b/worktree.go index 921e600a0..e45d81548 100644 --- a/worktree.go +++ b/worktree.go @@ -750,9 +750,7 @@ func (w *Worktree) doClean(status Status, opts *CleanOptions, dir string, files return err } } else { - // check if file is 'Untracked' - s, ok := (status)[filepath.ToSlash(path)] - if ok && s.Worktree == Untracked { + if status.IsUntracked(path) { if err := w.Filesystem.Remove(path); err != nil { return err } From ba3ee05efbdeb11364d585ec4dfa84fe07e64430 Mon Sep 17 00:00:00 2001 From: Taru Karttunen Date: Wed, 29 Aug 2018 12:43:23 +0000 Subject: [PATCH 14/30] plumbing: object: Clamp object timestamps before unix epoch to unix epoch Signed-off-by: Taru Karttunen --- plumbing/object/object.go | 6 +++++- plumbing/object/tag_test.go | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plumbing/object/object.go b/plumbing/object/object.go index 4b59aba7a..e960e50c9 100644 --- a/plumbing/object/object.go +++ b/plumbing/object/object.go @@ -152,7 +152,11 @@ func (s *Signature) decodeTimeAndTimeZone(b []byte) { } func (s *Signature) encodeTimeAndTimeZone(w io.Writer) error { - _, err := fmt.Fprintf(w, "%d %s", s.When.Unix(), s.When.Format("-0700")) + u := s.When.Unix() + if u < 0 { + u = 0 + } + _, err := fmt.Fprintf(w, "%d %s", u, s.When.Format("-0700")) return err } diff --git a/plumbing/object/tag_test.go b/plumbing/object/tag_test.go index 9900093e7..e7dd06e21 100644 --- a/plumbing/object/tag_test.go +++ b/plumbing/object/tag_test.go @@ -265,7 +265,7 @@ func (s *TagSuite) TestStringNonCommit(c *C) { c.Assert(tag.String(), Equals, "tag TAG TWO\n"+ "Tagger: <>\n"+ - "Date: Mon Jan 01 00:00:00 0001 +0000\n"+ + "Date: Thu Jan 01 00:00:00 1970 +0000\n"+ "\n"+ "tag two\n") } From 14d9faa61e2d38d31593c8536121d814947240fc Mon Sep 17 00:00:00 2001 From: Zaq? Wiedmann Date: Tue, 28 Aug 2018 17:32:29 -0700 Subject: [PATCH 15/30] config: add commentChar to core config struct Signed-off-by: Zaq? Wiedmann --- config/config.go | 5 +++++ config/config_test.go | 2 ++ 2 files changed, 7 insertions(+) diff --git a/config/config.go b/config/config.go index ce6506dae..a637f6d70 100644 --- a/config/config.go +++ b/config/config.go @@ -40,6 +40,9 @@ type Config struct { IsBare bool // Worktree is the path to the root of the working tree. Worktree string + // CommentChar is the character indicating the start of a + // comment for commands like commit and tag + CommentChar string } Pack struct { @@ -113,6 +116,7 @@ const ( urlKey = "url" bareKey = "bare" worktreeKey = "worktree" + commentCharKey = "commentChar" windowKey = "window" mergeKey = "merge" @@ -151,6 +155,7 @@ func (c *Config) unmarshalCore() { } c.Core.Worktree = s.Options.Get(worktreeKey) + c.Core.CommentChar = s.Options.Get(commentCharKey) } func (c *Config) unmarshalPack() error { diff --git a/config/config_test.go b/config/config_test.go index 5cd713e45..fe73de872 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -13,6 +13,7 @@ func (s *ConfigSuite) TestUnmarshall(c *C) { input := []byte(`[core] bare = true worktree = foo + commentchar = bar [pack] window = 20 [remote "origin"] @@ -38,6 +39,7 @@ func (s *ConfigSuite) TestUnmarshall(c *C) { c.Assert(cfg.Core.IsBare, Equals, true) c.Assert(cfg.Core.Worktree, Equals, "foo") + c.Assert(cfg.Core.CommentChar, Equals, "bar") c.Assert(cfg.Pack.Window, Equals, uint(20)) c.Assert(cfg.Remotes, HasLen, 2) c.Assert(cfg.Remotes["origin"].Name, Equals, "origin") From 1e1a7d0623459807d6f1e871492147f971f7540c Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 30 Aug 2018 15:29:51 +0200 Subject: [PATCH 16/30] git: add Static option to PlainOpen Also adds Static configuration to Storage and DotGit. This option means that the git repository is not expected to be modified while open and enables some optimizations. Each time a file is accessed the storer tries to open an object file for the requested hash. When this is done for a lot of objects it is expensive. With Static option a list of object files is generated the first time an object is accessed and used to check if exists instead of using system calls. A similar optimization is done for packfiles. Signed-off-by: Javi Fontan --- options.go | 2 + repository.go | 11 +- repository_test.go | 19 +++ storage/filesystem/dotgit/dotgit.go | 182 +++++++++++++++++++++++++++- storage/filesystem/storage.go | 27 ++++- storage/filesystem/storage_test.go | 18 +++ 6 files changed, 249 insertions(+), 10 deletions(-) diff --git a/options.go b/options.go index 7b1570f7b..7b551460c 100644 --- a/options.go +++ b/options.go @@ -431,6 +431,8 @@ type PlainOpenOptions struct { // DetectDotGit defines whether parent directories should be // walked until a .git directory or file is found. DetectDotGit bool + // Static means that the repository won't be modified while open. + Static bool } // Validate validates the fields and sets the default values. diff --git a/repository.go b/repository.go index 818cfb3c3..d99d6eb21 100644 --- a/repository.go +++ b/repository.go @@ -235,9 +235,8 @@ func PlainOpen(path string) (*Repository, error) { return PlainOpenWithOptions(path, &PlainOpenOptions{}) } -// PlainOpen opens a git repository from the given path. It detects if the -// repository is bare or a normal one. If the path doesn't contain a valid -// repository ErrRepositoryNotExists is returned +// PlainOpenWithOptions opens a git repository from the given path with specific +// options. See PlainOpen for more info. func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) { dot, wt, err := dotGitToOSFilesystems(path, o.DetectDotGit) if err != nil { @@ -252,7 +251,11 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - s, err := filesystem.NewStorage(dot) + so := filesystem.StorageOptions{ + Static: o.Static, + } + + s, err := filesystem.NewStorageWithOptions(dot, so) if err != nil { return nil, err } diff --git a/repository_test.go b/repository_test.go index 261af7a7b..b891413f4 100644 --- a/repository_test.go +++ b/repository_test.go @@ -550,6 +550,25 @@ func (s *RepositorySuite) TestPlainOpenNotExistsDetectDotGit(c *C) { c.Assert(r, IsNil) } +func (s *RepositorySuite) TestPlainOpenStatic(c *C) { + dir, err := ioutil.TempDir("", "plain-open") + c.Assert(err, IsNil) + defer os.RemoveAll(dir) + + r, err := PlainInit(dir, true) + c.Assert(err, IsNil) + c.Assert(r, NotNil) + + op := &PlainOpenOptions{Static: true} + r, err = PlainOpenWithOptions(dir, op) + c.Assert(err, IsNil) + c.Assert(r, NotNil) + + sto, ok := r.Storer.(*filesystem.Storage) + c.Assert(ok, Equals, true) + c.Assert(sto.StorageOptions.Static, Equals, true) +} + func (s *RepositorySuite) TestPlainClone(c *C) { r, err := PlainClone(c.MkDir(), false, &CloneOptions{ URL: s.GetBasicLocalRepositoryURL(), diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index df4f75691..2048ddc29 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -60,18 +60,39 @@ var ( // The DotGit type represents a local git repository on disk. This // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { + DotGitOptions fs billy.Filesystem // incoming object directory information incomingChecked bool incomingDirName string + + objectList []plumbing.Hash + objectMap map[plumbing.Hash]struct{} + packList []plumbing.Hash + packMap map[plumbing.Hash]struct{} +} + +// DotGitOptions holds configuration options for new DotGit objects. +type DotGitOptions struct { + // Static means that the filesystem won't be changed while the repo is open. + Static bool } // New returns a DotGit value ready to be used. The path argument must // be the absolute path of a git repository directory (e.g. // "/foo/bar/.git"). func New(fs billy.Filesystem) *DotGit { - return &DotGit{fs: fs} + return NewWithOptions(fs, DotGitOptions{}) +} + +// NewWithOptions creates a new DotGit and sets non default configuration +// options. See New for complete help. +func NewWithOptions(fs billy.Filesystem, o DotGitOptions) *DotGit { + return &DotGit{ + DotGitOptions: o, + fs: fs, + } } // Initialize creates all the folder scaffolding. @@ -143,11 +164,25 @@ func (d *DotGit) Shallow() (billy.File, error) { // NewObjectPack return a writer for a new packfile, it saves the packfile to // disk and also generates and save the index for the given packfile. func (d *DotGit) NewObjectPack() (*PackWriter, error) { + d.cleanPackList() return newPackWrite(d.fs) } // ObjectPacks returns the list of availables packfiles func (d *DotGit) ObjectPacks() ([]plumbing.Hash, error) { + if !d.Static { + return d.objectPacks() + } + + err := d.genPackList() + if err != nil { + return nil, err + } + + return d.packList, nil +} + +func (d *DotGit) objectPacks() ([]plumbing.Hash, error) { packDir := d.fs.Join(objectsPath, packPath) files, err := d.fs.ReadDir(packDir) if err != nil { @@ -181,6 +216,11 @@ func (d *DotGit) objectPackPath(hash plumbing.Hash, extension string) string { } func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + pack, err := d.fs.Open(d.objectPackPath(hash, extension)) if err != nil { if os.IsNotExist(err) { @@ -195,15 +235,27 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil // ObjectPack returns a fs.File of the given packfile func (d *DotGit) ObjectPack(hash plumbing.Hash) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + return d.objectPackOpen(hash, `pack`) } // ObjectPackIdx returns a fs.File of the index file for a given packfile func (d *DotGit) ObjectPackIdx(hash plumbing.Hash) (billy.File, error) { + err := d.hasPack(hash) + if err != nil { + return nil, err + } + return d.objectPackOpen(hash, `idx`) } func (d *DotGit) DeleteOldObjectPackAndIndex(hash plumbing.Hash, t time.Time) error { + d.cleanPackList() + path := d.objectPackPath(hash, `pack`) if !t.IsZero() { fi, err := d.fs.Stat(path) @@ -224,12 +276,23 @@ func (d *DotGit) DeleteOldObjectPackAndIndex(hash plumbing.Hash, t time.Time) er // NewObject return a writer for a new object file. func (d *DotGit) NewObject() (*ObjectWriter, error) { + d.cleanObjectList() + return newObjectWriter(d.fs) } // Objects returns a slice with the hashes of objects found under the // .git/objects/ directory. func (d *DotGit) Objects() ([]plumbing.Hash, error) { + if d.Static { + err := d.genObjectList() + if err != nil { + return nil, err + } + + return d.objectList, nil + } + var objects []plumbing.Hash err := d.ForEachObjectHash(func(hash plumbing.Hash) error { objects = append(objects, hash) @@ -241,9 +304,29 @@ func (d *DotGit) Objects() ([]plumbing.Hash, error) { return objects, nil } -// Objects returns a slice with the hashes of objects found under the -// .git/objects/ directory. +// ForEachObjectHash iterates over the hashes of objects found under the +// .git/objects/ directory and executes the provided . func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { + if !d.Static { + return d.forEachObjectHash(fun) + } + + err := d.genObjectList() + if err != nil { + return err + } + + for _, h := range d.objectList { + err := fun(h) + if err != nil { + return err + } + } + + return nil +} + +func (d *DotGit) forEachObjectHash(fun func(plumbing.Hash) error) error { files, err := d.fs.ReadDir(objectsPath) if err != nil { if os.IsNotExist(err) { @@ -278,6 +361,87 @@ func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { return nil } +func (d *DotGit) cleanObjectList() { + d.objectMap = nil + d.objectList = nil +} + +func (d *DotGit) genObjectList() error { + if d.objectMap != nil { + return nil + } + + d.objectMap = make(map[plumbing.Hash]struct{}) + return d.forEachObjectHash(func(h plumbing.Hash) error { + d.objectList = append(d.objectList, h) + d.objectMap[h] = struct{}{} + + return nil + }) +} + +func (d *DotGit) hasObject(h plumbing.Hash) error { + if !d.Static { + return nil + } + + err := d.genObjectList() + if err != nil { + return err + } + + _, ok := d.objectMap[h] + if !ok { + return plumbing.ErrObjectNotFound + } + + return nil +} + +func (d *DotGit) cleanPackList() { + d.packMap = nil + d.packList = nil +} + +func (d *DotGit) genPackList() error { + if d.packMap != nil { + return nil + } + + op, err := d.objectPacks() + if err != nil { + return err + } + + d.packMap = make(map[plumbing.Hash]struct{}) + d.packList = nil + + for _, h := range op { + d.packList = append(d.packList, h) + d.packMap[h] = struct{}{} + } + + return nil +} + +func (d *DotGit) hasPack(h plumbing.Hash) error { + if !d.Static { + return nil + } + + err := d.genPackList() + if err != nil { + return err + } + + _, ok := d.packMap[h] + if !ok { + return ErrPackfileNotFound + } + + return nil +} + func (d *DotGit) objectPath(h plumbing.Hash) string { hash := h.String() return d.fs.Join(objectsPath, hash[0:2], hash[2:40]) @@ -322,6 +486,11 @@ func (d *DotGit) hasIncomingObjects() bool { // Object returns a fs.File pointing the object file, if exists func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { + err := d.hasObject(h) + if err != nil { + return nil, err + } + obj1, err1 := d.fs.Open(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Open(d.incomingObjectPath(h)) @@ -335,6 +504,11 @@ func (d *DotGit) Object(h plumbing.Hash) (billy.File, error) { // ObjectStat returns a os.FileInfo pointing the object file, if exists func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { + err := d.hasObject(h) + if err != nil { + return nil, err + } + obj1, err1 := d.fs.Stat(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { obj2, err2 := d.fs.Stat(d.incomingObjectPath(h)) @@ -348,6 +522,8 @@ func (d *DotGit) ObjectStat(h plumbing.Hash) (os.FileInfo, error) { // ObjectDelete removes the object file, if exists func (d *DotGit) ObjectDelete(h plumbing.Hash) error { + d.cleanObjectList() + err1 := d.fs.Remove(d.objectPath(h)) if os.IsNotExist(err1) && d.hasIncomingObjects() { err2 := d.fs.Remove(d.incomingObjectPath(h)) diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 622bb4a8d..a969a1fbc 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -11,6 +11,8 @@ import ( // standard git format (this is, the .git directory). Zero values of this type // are not safe to use, see the NewStorage function below. type Storage struct { + StorageOptions + fs billy.Filesystem dir *dotgit.DotGit @@ -22,17 +24,36 @@ type Storage struct { ModuleStorage } +// StorageOptions holds configuration for the storage. +type StorageOptions struct { + // Static means that the filesystem is not modified while the repo is open. + Static bool +} + // NewStorage returns a new Storage backed by a given `fs.Filesystem` func NewStorage(fs billy.Filesystem) (*Storage, error) { - dir := dotgit.New(fs) + return NewStorageWithOptions(fs, StorageOptions{}) +} + +// NewStorageWithOptions returns a new Storage backed by a given `fs.Filesystem` +func NewStorageWithOptions( + fs billy.Filesystem, + ops StorageOptions, +) (*Storage, error) { + dOps := dotgit.DotGitOptions{ + Static: ops.Static, + } + + dir := dotgit.NewWithOptions(fs, dOps) o, err := NewObjectStorage(dir) if err != nil { return nil, err } return &Storage{ - fs: fs, - dir: dir, + StorageOptions: ops, + fs: fs, + dir: dir, ObjectStorage: o, ReferenceStorage: ReferenceStorage{dir: dir}, diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index 4d9ba6fec..d7ebf7122 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -26,6 +26,10 @@ func (s *StorageSuite) SetUpTest(c *C) { storage, err := NewStorage(osfs.New(s.dir)) c.Assert(err, IsNil) + setUpTest(s, c, storage) +} + +func setUpTest(s *StorageSuite, c *C, storage *Storage) { // ensure that right interfaces are implemented var _ storer.EncodedObjectStorer = storage var _ storer.IndexStorer = storage @@ -51,3 +55,17 @@ func (s *StorageSuite) TestNewStorageShouldNotAddAnyContentsToDir(c *C) { c.Assert(err, IsNil) c.Assert(fis, HasLen, 0) } + +type StorageStaticSuite struct { + StorageSuite +} + +var _ = Suite(&StorageStaticSuite{}) + +func (s *StorageStaticSuite) SetUpTest(c *C) { + s.dir = c.MkDir() + storage, err := NewStorageWithOptions(osfs.New(s.dir), StorageOptions{Static: true}) + c.Assert(err, IsNil) + + setUpTest(&s.StorageSuite, c, storage) +} From 82945e31dd8bce5fc51d4fd16d696a6d326e5f44 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 30 Aug 2018 18:33:37 +0200 Subject: [PATCH 17/30] git, storer: use a common storer.Options for storer and PlainOpen Signed-off-by: Javi Fontan --- options.go | 6 ++++-- plumbing/storer/storer.go | 6 ++++++ repository.go | 6 +----- repository_test.go | 7 +++++-- storage/filesystem/dotgit/dotgit.go | 17 ++++++----------- storage/filesystem/storage.go | 25 ++++++++----------------- storage/filesystem/storage_test.go | 4 +++- 7 files changed, 33 insertions(+), 38 deletions(-) diff --git a/options.go b/options.go index 7b551460c..f67a454d5 100644 --- a/options.go +++ b/options.go @@ -9,6 +9,7 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/sideband" + "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/plumbing/transport" ) @@ -428,11 +429,12 @@ func (o *GrepOptions) Validate(w *Worktree) error { // PlainOpenOptions describes how opening a plain repository should be // performed. type PlainOpenOptions struct { + // Storage layer options. + Storage storer.Options + // DetectDotGit defines whether parent directories should be // walked until a .git directory or file is found. DetectDotGit bool - // Static means that the repository won't be modified while open. - Static bool } // Validate validates the fields and sets the default values. diff --git a/plumbing/storer/storer.go b/plumbing/storer/storer.go index c7bc65a0c..1b7d2266f 100644 --- a/plumbing/storer/storer.go +++ b/plumbing/storer/storer.go @@ -13,3 +13,9 @@ type Initializer interface { // any. Init() error } + +// Options holds configuration for the storage. +type Options struct { + // Static means that the filesystem is not modified while the repo is open. + Static bool +} diff --git a/repository.go b/repository.go index d99d6eb21..4ad525279 100644 --- a/repository.go +++ b/repository.go @@ -251,11 +251,7 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - so := filesystem.StorageOptions{ - Static: o.Static, - } - - s, err := filesystem.NewStorageWithOptions(dot, so) + s, err := filesystem.NewStorageWithOptions(dot, o.Storage) if err != nil { return nil, err } diff --git a/repository_test.go b/repository_test.go index b891413f4..8956a9d3e 100644 --- a/repository_test.go +++ b/repository_test.go @@ -559,14 +559,17 @@ func (s *RepositorySuite) TestPlainOpenStatic(c *C) { c.Assert(err, IsNil) c.Assert(r, NotNil) - op := &PlainOpenOptions{Static: true} + op := &PlainOpenOptions{ + Storage: storer.Options{Static: true}, + } + r, err = PlainOpenWithOptions(dir, op) c.Assert(err, IsNil) c.Assert(r, NotNil) sto, ok := r.Storer.(*filesystem.Storage) c.Assert(ok, Equals, true) - c.Assert(sto.StorageOptions.Static, Equals, true) + c.Assert(sto.Options.Static, Equals, true) } func (s *RepositorySuite) TestPlainClone(c *C) { diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index 2048ddc29..41e5c7543 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -14,6 +14,7 @@ import ( "gopkg.in/src-d/go-billy.v4/osfs" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/utils/ioutil" "gopkg.in/src-d/go-billy.v4" @@ -60,7 +61,7 @@ var ( // The DotGit type represents a local git repository on disk. This // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { - DotGitOptions + storer.Options fs billy.Filesystem // incoming object directory information @@ -73,25 +74,19 @@ type DotGit struct { packMap map[plumbing.Hash]struct{} } -// DotGitOptions holds configuration options for new DotGit objects. -type DotGitOptions struct { - // Static means that the filesystem won't be changed while the repo is open. - Static bool -} - // New returns a DotGit value ready to be used. The path argument must // be the absolute path of a git repository directory (e.g. // "/foo/bar/.git"). func New(fs billy.Filesystem) *DotGit { - return NewWithOptions(fs, DotGitOptions{}) + return NewWithOptions(fs, storer.Options{}) } // NewWithOptions creates a new DotGit and sets non default configuration // options. See New for complete help. -func NewWithOptions(fs billy.Filesystem, o DotGitOptions) *DotGit { +func NewWithOptions(fs billy.Filesystem, o storer.Options) *DotGit { return &DotGit{ - DotGitOptions: o, - fs: fs, + Options: o, + fs: fs, } } diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index a969a1fbc..24e64545e 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -2,6 +2,7 @@ package filesystem import ( + "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem/dotgit" "gopkg.in/src-d/go-billy.v4" @@ -11,7 +12,7 @@ import ( // standard git format (this is, the .git directory). Zero values of this type // are not safe to use, see the NewStorage function below. type Storage struct { - StorageOptions + storer.Options fs billy.Filesystem dir *dotgit.DotGit @@ -24,36 +25,26 @@ type Storage struct { ModuleStorage } -// StorageOptions holds configuration for the storage. -type StorageOptions struct { - // Static means that the filesystem is not modified while the repo is open. - Static bool -} - // NewStorage returns a new Storage backed by a given `fs.Filesystem` func NewStorage(fs billy.Filesystem) (*Storage, error) { - return NewStorageWithOptions(fs, StorageOptions{}) + return NewStorageWithOptions(fs, storer.Options{}) } // NewStorageWithOptions returns a new Storage backed by a given `fs.Filesystem` func NewStorageWithOptions( fs billy.Filesystem, - ops StorageOptions, + ops storer.Options, ) (*Storage, error) { - dOps := dotgit.DotGitOptions{ - Static: ops.Static, - } - - dir := dotgit.NewWithOptions(fs, dOps) + dir := dotgit.NewWithOptions(fs, ops) o, err := NewObjectStorage(dir) if err != nil { return nil, err } return &Storage{ - StorageOptions: ops, - fs: fs, - dir: dir, + Options: ops, + fs: fs, + dir: dir, ObjectStorage: o, ReferenceStorage: ReferenceStorage{dir: dir}, diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index d7ebf7122..23628c7a7 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -64,7 +64,9 @@ var _ = Suite(&StorageStaticSuite{}) func (s *StorageStaticSuite) SetUpTest(c *C) { s.dir = c.MkDir() - storage, err := NewStorageWithOptions(osfs.New(s.dir), StorageOptions{Static: true}) + storage, err := NewStorageWithOptions( + osfs.New(s.dir), + storer.Options{Static: true}) c.Assert(err, IsNil) setUpTest(&s.StorageSuite, c, storage) From d7e6cf5b73947108d0c16b9c04b38891de47ef5d Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 30 Aug 2018 18:35:39 +0200 Subject: [PATCH 18/30] dotgit: fix typo in comment Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index 41e5c7543..c42ed88cd 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -300,7 +300,7 @@ func (d *DotGit) Objects() ([]plumbing.Hash, error) { } // ForEachObjectHash iterates over the hashes of objects found under the -// .git/objects/ directory and executes the provided . +// .git/objects/ directory and executes the provided function. func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { if !d.Static { return d.forEachObjectHash(fun) From 2a7c664b62dd0d87f7ab67b30b1952727788cffa Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Fri, 31 Aug 2018 12:29:27 +0200 Subject: [PATCH 19/30] git: do not expose storage options in PlainOpen Signed-off-by: Javi Fontan --- options.go | 4 ---- repository.go | 2 +- repository_test.go | 22 ---------------------- 3 files changed, 1 insertion(+), 27 deletions(-) diff --git a/options.go b/options.go index f67a454d5..7b1570f7b 100644 --- a/options.go +++ b/options.go @@ -9,7 +9,6 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp/sideband" - "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/plumbing/transport" ) @@ -429,9 +428,6 @@ func (o *GrepOptions) Validate(w *Worktree) error { // PlainOpenOptions describes how opening a plain repository should be // performed. type PlainOpenOptions struct { - // Storage layer options. - Storage storer.Options - // DetectDotGit defines whether parent directories should be // walked until a .git directory or file is found. DetectDotGit bool diff --git a/repository.go b/repository.go index 4ad525279..f619934a4 100644 --- a/repository.go +++ b/repository.go @@ -251,7 +251,7 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - s, err := filesystem.NewStorageWithOptions(dot, o.Storage) + s, err := filesystem.NewStorage(dot) if err != nil { return nil, err } diff --git a/repository_test.go b/repository_test.go index 8956a9d3e..261af7a7b 100644 --- a/repository_test.go +++ b/repository_test.go @@ -550,28 +550,6 @@ func (s *RepositorySuite) TestPlainOpenNotExistsDetectDotGit(c *C) { c.Assert(r, IsNil) } -func (s *RepositorySuite) TestPlainOpenStatic(c *C) { - dir, err := ioutil.TempDir("", "plain-open") - c.Assert(err, IsNil) - defer os.RemoveAll(dir) - - r, err := PlainInit(dir, true) - c.Assert(err, IsNil) - c.Assert(r, NotNil) - - op := &PlainOpenOptions{ - Storage: storer.Options{Static: true}, - } - - r, err = PlainOpenWithOptions(dir, op) - c.Assert(err, IsNil) - c.Assert(r, NotNil) - - sto, ok := r.Storer.(*filesystem.Storage) - c.Assert(ok, Equals, true) - c.Assert(sto.Options.Static, Equals, true) -} - func (s *RepositorySuite) TestPlainClone(c *C) { r, err := PlainClone(c.MkDir(), false, &CloneOptions{ URL: s.GetBasicLocalRepositoryURL(), From cf626677508238893c7c88c3c786a02f17afcc4c Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Fri, 31 Aug 2018 14:56:23 +0200 Subject: [PATCH 20/30] plumbing/storer: rename Static option to ExclusiveAccess Signed-off-by: Javi Fontan --- plumbing/storer/storer.go | 5 +++-- storage/filesystem/dotgit/dotgit.go | 10 +++++----- storage/filesystem/storage_test.go | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/plumbing/storer/storer.go b/plumbing/storer/storer.go index 1b7d2266f..9bbb44fe4 100644 --- a/plumbing/storer/storer.go +++ b/plumbing/storer/storer.go @@ -16,6 +16,7 @@ type Initializer interface { // Options holds configuration for the storage. type Options struct { - // Static means that the filesystem is not modified while the repo is open. - Static bool + // ExclusiveAccess means that the filesystem is not modified externally + // while the repo is open. + ExclusiveAccess bool } diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index c42ed88cd..7626078df 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -165,7 +165,7 @@ func (d *DotGit) NewObjectPack() (*PackWriter, error) { // ObjectPacks returns the list of availables packfiles func (d *DotGit) ObjectPacks() ([]plumbing.Hash, error) { - if !d.Static { + if !d.ExclusiveAccess { return d.objectPacks() } @@ -279,7 +279,7 @@ func (d *DotGit) NewObject() (*ObjectWriter, error) { // Objects returns a slice with the hashes of objects found under the // .git/objects/ directory. func (d *DotGit) Objects() ([]plumbing.Hash, error) { - if d.Static { + if d.ExclusiveAccess { err := d.genObjectList() if err != nil { return nil, err @@ -302,7 +302,7 @@ func (d *DotGit) Objects() ([]plumbing.Hash, error) { // ForEachObjectHash iterates over the hashes of objects found under the // .git/objects/ directory and executes the provided function. func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { - if !d.Static { + if !d.ExclusiveAccess { return d.forEachObjectHash(fun) } @@ -376,7 +376,7 @@ func (d *DotGit) genObjectList() error { } func (d *DotGit) hasObject(h plumbing.Hash) error { - if !d.Static { + if !d.ExclusiveAccess { return nil } @@ -420,7 +420,7 @@ func (d *DotGit) genPackList() error { } func (d *DotGit) hasPack(h plumbing.Hash) error { - if !d.Static { + if !d.ExclusiveAccess { return nil } diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index 23628c7a7..11bf4fc3c 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -66,7 +66,7 @@ func (s *StorageStaticSuite) SetUpTest(c *C) { s.dir = c.MkDir() storage, err := NewStorageWithOptions( osfs.New(s.dir), - storer.Options{Static: true}) + storer.Options{ExclusiveAccess: true}) c.Assert(err, IsNil) setUpTest(&s.StorageSuite, c, storage) From 95acbf6c3958b7540a8549aa049051325fcecd8b Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Mon, 3 Sep 2018 11:17:22 +0200 Subject: [PATCH 21/30] storage/filesystem: make Storage options private Signed-off-by: Javi Fontan --- storage/filesystem/storage.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 24e64545e..d2c528747 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -12,7 +12,7 @@ import ( // standard git format (this is, the .git directory). Zero values of this type // are not safe to use, see the NewStorage function below. type Storage struct { - storer.Options + options storer.Options fs billy.Filesystem dir *dotgit.DotGit @@ -42,7 +42,7 @@ func NewStorageWithOptions( } return &Storage{ - Options: ops, + options: ops, fs: fs, dir: dir, From 874f669becc25489081306bbbcbbc27b970f6295 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Mon, 3 Sep 2018 19:40:22 +0200 Subject: [PATCH 22/30] storage/filesystem: move Options to filesytem and dotgit Signed-off-by: Javi Fontan --- plumbing/storer/storer.go | 7 ------- storage/filesystem/dotgit/dotgit.go | 28 +++++++++++++++++----------- storage/filesystem/object.go | 12 ++++++++++++ storage/filesystem/storage.go | 27 +++++++++++++++++---------- storage/filesystem/storage_test.go | 8 ++++---- 5 files changed, 50 insertions(+), 32 deletions(-) diff --git a/plumbing/storer/storer.go b/plumbing/storer/storer.go index 9bbb44fe4..c7bc65a0c 100644 --- a/plumbing/storer/storer.go +++ b/plumbing/storer/storer.go @@ -13,10 +13,3 @@ type Initializer interface { // any. Init() error } - -// Options holds configuration for the storage. -type Options struct { - // ExclusiveAccess means that the filesystem is not modified externally - // while the repo is open. - ExclusiveAccess bool -} diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index 7626078df..00dd2a459 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -14,7 +14,6 @@ import ( "gopkg.in/src-d/go-billy.v4/osfs" "gopkg.in/src-d/go-git.v4/plumbing" - "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/utils/ioutil" "gopkg.in/src-d/go-billy.v4" @@ -58,11 +57,18 @@ var ( ErrSymRefTargetNotFound = errors.New("symbolic reference target not found") ) +// Options holds configuration for the storage. +type Options struct { + // ExclusiveAccess means that the filesystem is not modified externally + // while the repo is open. + ExclusiveAccess bool +} + // The DotGit type represents a local git repository on disk. This // type is not zero-value-safe, use the New function to initialize it. type DotGit struct { - storer.Options - fs billy.Filesystem + options Options + fs billy.Filesystem // incoming object directory information incomingChecked bool @@ -78,14 +84,14 @@ type DotGit struct { // be the absolute path of a git repository directory (e.g. // "/foo/bar/.git"). func New(fs billy.Filesystem) *DotGit { - return NewWithOptions(fs, storer.Options{}) + return NewWithOptions(fs, Options{}) } // NewWithOptions creates a new DotGit and sets non default configuration // options. See New for complete help. -func NewWithOptions(fs billy.Filesystem, o storer.Options) *DotGit { +func NewWithOptions(fs billy.Filesystem, o Options) *DotGit { return &DotGit{ - Options: o, + options: o, fs: fs, } } @@ -165,7 +171,7 @@ func (d *DotGit) NewObjectPack() (*PackWriter, error) { // ObjectPacks returns the list of availables packfiles func (d *DotGit) ObjectPacks() ([]plumbing.Hash, error) { - if !d.ExclusiveAccess { + if !d.options.ExclusiveAccess { return d.objectPacks() } @@ -279,7 +285,7 @@ func (d *DotGit) NewObject() (*ObjectWriter, error) { // Objects returns a slice with the hashes of objects found under the // .git/objects/ directory. func (d *DotGit) Objects() ([]plumbing.Hash, error) { - if d.ExclusiveAccess { + if d.options.ExclusiveAccess { err := d.genObjectList() if err != nil { return nil, err @@ -302,7 +308,7 @@ func (d *DotGit) Objects() ([]plumbing.Hash, error) { // ForEachObjectHash iterates over the hashes of objects found under the // .git/objects/ directory and executes the provided function. func (d *DotGit) ForEachObjectHash(fun func(plumbing.Hash) error) error { - if !d.ExclusiveAccess { + if !d.options.ExclusiveAccess { return d.forEachObjectHash(fun) } @@ -376,7 +382,7 @@ func (d *DotGit) genObjectList() error { } func (d *DotGit) hasObject(h plumbing.Hash) error { - if !d.ExclusiveAccess { + if !d.options.ExclusiveAccess { return nil } @@ -420,7 +426,7 @@ func (d *DotGit) genPackList() error { } func (d *DotGit) hasPack(h plumbing.Hash) error { - if !d.ExclusiveAccess { + if !d.options.ExclusiveAccess { return nil } diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 3a3a2bd97..3519385f6 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -18,6 +18,8 @@ import ( ) type ObjectStorage struct { + options Options + // deltaBaseCache is an object cache uses to cache delta's bases when deltaBaseCache cache.Object @@ -27,7 +29,17 @@ type ObjectStorage struct { // NewObjectStorage creates a new ObjectStorage with the given .git directory. func NewObjectStorage(dir *dotgit.DotGit) (ObjectStorage, error) { + return NewObjectStorageWithOptions(dir, Options{}) +} + +// NewObjectStorageWithOptions creates a new ObjectStorage with the given .git +// directory and sets its options. +func NewObjectStorageWithOptions( + dir *dotgit.DotGit, + ops Options, +) (ObjectStorage, error) { s := ObjectStorage{ + options: ops, deltaBaseCache: cache.NewObjectLRUDefault(), dir: dir, } diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index d2c528747..25b3653c0 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -2,7 +2,6 @@ package filesystem import ( - "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem/dotgit" "gopkg.in/src-d/go-billy.v4" @@ -12,8 +11,6 @@ import ( // standard git format (this is, the .git directory). Zero values of this type // are not safe to use, see the NewStorage function below. type Storage struct { - options storer.Options - fs billy.Filesystem dir *dotgit.DotGit @@ -25,26 +22,36 @@ type Storage struct { ModuleStorage } +// Options holds configuration for the storage. +type Options struct { + // ExclusiveAccess means that the filesystem is not modified externally + // while the repo is open. + ExclusiveAccess bool +} + // NewStorage returns a new Storage backed by a given `fs.Filesystem` func NewStorage(fs billy.Filesystem) (*Storage, error) { - return NewStorageWithOptions(fs, storer.Options{}) + return NewStorageWithOptions(fs, Options{}) } // NewStorageWithOptions returns a new Storage backed by a given `fs.Filesystem` func NewStorageWithOptions( fs billy.Filesystem, - ops storer.Options, + ops Options, ) (*Storage, error) { - dir := dotgit.NewWithOptions(fs, ops) - o, err := NewObjectStorage(dir) + dirOps := dotgit.Options{ + ExclusiveAccess: ops.ExclusiveAccess, + } + + dir := dotgit.NewWithOptions(fs, dirOps) + o, err := NewObjectStorageWithOptions(dir, ops) if err != nil { return nil, err } return &Storage{ - options: ops, - fs: fs, - dir: dir, + fs: fs, + dir: dir, ObjectStorage: o, ReferenceStorage: ReferenceStorage{dir: dir}, diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index 11bf4fc3c..7f85ef548 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -56,17 +56,17 @@ func (s *StorageSuite) TestNewStorageShouldNotAddAnyContentsToDir(c *C) { c.Assert(fis, HasLen, 0) } -type StorageStaticSuite struct { +type StorageExclusiveSuite struct { StorageSuite } -var _ = Suite(&StorageStaticSuite{}) +var _ = Suite(&StorageExclusiveSuite{}) -func (s *StorageStaticSuite) SetUpTest(c *C) { +func (s *StorageExclusiveSuite) SetUpTest(c *C) { s.dir = c.MkDir() storage, err := NewStorageWithOptions( osfs.New(s.dir), - storer.Options{ExclusiveAccess: true}) + Options{ExclusiveAccess: true}) c.Assert(err, IsNil) setUpTest(&s.StorageSuite, c, storage) From 659ec443b4a975e3adf78f24e59ad69d210d2c0b Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Mon, 3 Sep 2018 20:02:25 +0200 Subject: [PATCH 23/30] storage/dotgit: add ExclusiveAccess tests in dotgit This functionality was already tested in storage/filesystem. The coverage tool only takes into account files from the same package of the test. Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index 64c2aeead..c34543e1d 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "gopkg.in/src-d/go-billy.v4" "gopkg.in/src-d/go-git.v4/plumbing" . "gopkg.in/check.v1" @@ -424,6 +425,18 @@ func (s *SuiteDotGit) TestObjectPacks(c *C) { fs := f.DotGit() dir := New(fs) + testObjectPacks(c, fs, dir, f) +} + +func (s *SuiteDotGit) TestObjectPacksExclusive(c *C) { + f := fixtures.Basic().ByTag(".git").One() + fs := f.DotGit() + dir := NewWithOptions(fs, Options{ExclusiveAccess: true}) + + testObjectPacks(c, fs, dir, f) +} + +func testObjectPacks(c *C, fs billy.Filesystem, dir *DotGit, f *fixtures.Fixture) { hashes, err := dir.ObjectPacks() c.Assert(err, IsNil) c.Assert(hashes, HasLen, 1) @@ -506,6 +519,17 @@ func (s *SuiteDotGit) TestObjects(c *C) { fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit() dir := New(fs) + testObjects(c, fs, dir) +} + +func (s *SuiteDotGit) TestObjectsExclusive(c *C) { + fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit() + dir := NewWithOptions(fs, Options{ExclusiveAccess: true}) + + testObjects(c, fs, dir) +} + +func testObjects(c *C, fs billy.Filesystem, dir *DotGit) { hashes, err := dir.Objects() c.Assert(err, IsNil) c.Assert(hashes, HasLen, 187) From 6384ab93a2dbac9045ee19099455cbcfe82d0201 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 30 Aug 2018 20:28:40 +0200 Subject: [PATCH 24/30] storage/dotgit: add KeepDescriptors option This option maintains packfile file descriptors opened after reading objects from them. It improves performance as it does not have to be opening packfiles each time an object is needed. Also adds Close to EncodedObjectStorer to close all the files manualy. Signed-off-by: Javi Fontan --- plumbing/storer/object.go | 2 ++ plumbing/storer/object_test.go | 4 +++ storage/filesystem/dotgit/dotgit.go | 43 +++++++++++++++++++++++- storage/filesystem/dotgit/dotgit_test.go | 28 +++++++++++++++ storage/filesystem/object.go | 10 +++++- storage/filesystem/storage.go | 4 +++ storage/memory/storage.go | 3 ++ 7 files changed, 92 insertions(+), 2 deletions(-) diff --git a/plumbing/storer/object.go b/plumbing/storer/object.go index 92aa62918..806b6bb78 100644 --- a/plumbing/storer/object.go +++ b/plumbing/storer/object.go @@ -40,6 +40,8 @@ type EncodedObjectStorer interface { // HasEncodedObject returns ErrObjNotFound if the object doesn't // exist. If the object does exist, it returns nil. HasEncodedObject(plumbing.Hash) error + // Close any opened files. + Close() error } // DeltaObjectStorer is an EncodedObjectStorer that can return delta diff --git a/plumbing/storer/object_test.go b/plumbing/storer/object_test.go index 6b4fe0fb6..adddec45f 100644 --- a/plumbing/storer/object_test.go +++ b/plumbing/storer/object_test.go @@ -157,3 +157,7 @@ func (o *MockObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (EncodedOb func (o *MockObjectStorage) Begin() Transaction { return nil } + +func (o *MockObjectStorage) Close() error { + return nil +} diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index 00dd2a459..df5cd10d7 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -62,6 +62,9 @@ type Options struct { // ExclusiveAccess means that the filesystem is not modified externally // while the repo is open. ExclusiveAccess bool + // KeepDescriptors makes the file descriptors to be reused but they will + // need to be manually closed calling Close(). + KeepDescriptors bool } // The DotGit type represents a local git repository on disk. This @@ -78,6 +81,8 @@ type DotGit struct { objectMap map[plumbing.Hash]struct{} packList []plumbing.Hash packMap map[plumbing.Hash]struct{} + + files map[string]billy.File } // New returns a DotGit value ready to be used. The path argument must @@ -123,6 +128,28 @@ func (d *DotGit) Initialize() error { return nil } +// Close closes all opened files. +func (d *DotGit) Close() error { + var firstError error + if d.files != nil { + for _, f := range d.files { + err := f.Close() + if err != nil && firstError == nil { + firstError = err + continue + } + } + + d.files = nil + } + + if firstError != nil { + return firstError + } + + return nil +} + // ConfigWriter returns a file pointer for write to the config file func (d *DotGit) ConfigWriter() (billy.File, error) { return d.fs.Create(configPath) @@ -217,12 +244,22 @@ func (d *DotGit) objectPackPath(hash plumbing.Hash, extension string) string { } func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.File, error) { + if d.files == nil { + d.files = make(map[string]billy.File) + } + err := d.hasPack(hash) if err != nil { return nil, err } - pack, err := d.fs.Open(d.objectPackPath(hash, extension)) + path := d.objectPackPath(hash, extension) + f, ok := d.files[path] + if ok { + return f, nil + } + + pack, err := d.fs.Open(path) if err != nil { if os.IsNotExist(err) { return nil, ErrPackfileNotFound @@ -231,6 +268,10 @@ func (d *DotGit) objectPackOpen(hash plumbing.Hash, extension string) (billy.Fil return nil, err } + if d.options.KeepDescriptors && extension == "pack" { + d.files[path] = pack + } + return pack, nil } diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index c34543e1d..50f8e649a 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -465,6 +465,34 @@ func (s *SuiteDotGit) TestObjectPack(c *C) { c.Assert(filepath.Ext(pack.Name()), Equals, ".pack") } +func (s *SuiteDotGit) TestObjectPackWithKeepDescriptors(c *C) { + f := fixtures.Basic().ByTag(".git").One() + fs := f.DotGit() + dir := NewWithOptions(fs, Options{KeepDescriptors: true}) + + pack, err := dir.ObjectPack(f.PackfileHash) + c.Assert(err, IsNil) + c.Assert(filepath.Ext(pack.Name()), Equals, ".pack") + + pack2, err := dir.ObjectPack(f.PackfileHash) + c.Assert(err, IsNil) + c.Assert(pack, Equals, pack2) + + err = dir.Close() + c.Assert(err, IsNil) + + pack2, err = dir.ObjectPack(f.PackfileHash) + c.Assert(err, IsNil) + c.Assert(pack, Not(Equals), pack2) + + err = pack2.Close() + c.Assert(err, IsNil) + + err = dir.Close() + c.Assert(err, NotNil) + +} + func (s *SuiteDotGit) TestObjectPackIdx(c *C) { f := fixtures.Basic().ByTag(".git").One() fs := f.DotGit() diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 3519385f6..3545e2751 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -74,6 +74,7 @@ func (s *ObjectStorage) loadIdxFile(h plumbing.Hash) (err error) { } defer ioutil.CheckClose(f, &err) + idxf := idxfile.NewMemoryIndex() d := idxfile.NewDecoder(f) if err = d.Decode(idxf); err != nil { @@ -280,7 +281,9 @@ func (s *ObjectStorage) getFromPackfile(h plumbing.Hash, canBeDelta bool) ( return nil, err } - defer ioutil.CheckClose(f, &err) + if !s.options.KeepDescriptors { + defer ioutil.CheckClose(f, &err) + } idx := s.index[pack] if canBeDelta { @@ -423,6 +426,11 @@ func (s *ObjectStorage) buildPackfileIters(t plumbing.ObjectType, seen map[plumb }, nil } +// Close closes all opened files. +func (s *ObjectStorage) Close() error { + return s.dir.Close() +} + type lazyPackfilesIter struct { hashes []plumbing.Hash open func(h plumbing.Hash) (storer.EncodedObjectIter, error) diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 25b3653c0..7fae7897c 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -27,6 +27,9 @@ type Options struct { // ExclusiveAccess means that the filesystem is not modified externally // while the repo is open. ExclusiveAccess bool + // KeepDescriptors makes the file descriptors to be reused but they will + // need to be manually closed calling Close(). + KeepDescriptors bool } // NewStorage returns a new Storage backed by a given `fs.Filesystem` @@ -41,6 +44,7 @@ func NewStorageWithOptions( ) (*Storage, error) { dirOps := dotgit.Options{ ExclusiveAccess: ops.ExclusiveAccess, + KeepDescriptors: ops.KeepDescriptors, } dir := dotgit.NewWithOptions(fs, dirOps) diff --git a/storage/memory/storage.go b/storage/memory/storage.go index 2e3250905..3d3b348bf 100644 --- a/storage/memory/storage.go +++ b/storage/memory/storage.go @@ -183,6 +183,9 @@ func (o *ObjectStorage) ObjectPacks() ([]plumbing.Hash, error) { func (o *ObjectStorage) DeleteOldObjectPackAndIndex(plumbing.Hash, time.Time) error { return nil } +func (s *ObjectStorage) Close() error { + return nil +} var errNotSupported = fmt.Errorf("Not supported") From 5260b87fd08f70df6f1eb297ccb04d74d32dd6c8 Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Tue, 4 Sep 2018 18:26:59 +0200 Subject: [PATCH 25/30] plumbing/storer: do not expose Close in EncodedObjectStorer interface Signed-off-by: Javi Fontan --- plumbing/storer/object.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/plumbing/storer/object.go b/plumbing/storer/object.go index 806b6bb78..92aa62918 100644 --- a/plumbing/storer/object.go +++ b/plumbing/storer/object.go @@ -40,8 +40,6 @@ type EncodedObjectStorer interface { // HasEncodedObject returns ErrObjNotFound if the object doesn't // exist. If the object does exist, it returns nil. HasEncodedObject(plumbing.Hash) error - // Close any opened files. - Close() error } // DeltaObjectStorer is an EncodedObjectStorer that can return delta From 9013dde72d0387a74b728ee336019728ba159d1c Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Wed, 5 Sep 2018 11:01:06 +0200 Subject: [PATCH 26/30] storage/filesystem: add KeepDescriptors test Also delete Close from MockObjectStorage and memory storer. Signed-off-by: Javi Fontan --- plumbing/storer/object_test.go | 4 ---- storage/filesystem/object_test.go | 31 +++++++++++++++++++++++++++++++ storage/memory/storage.go | 3 --- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/plumbing/storer/object_test.go b/plumbing/storer/object_test.go index adddec45f..6b4fe0fb6 100644 --- a/plumbing/storer/object_test.go +++ b/plumbing/storer/object_test.go @@ -157,7 +157,3 @@ func (o *MockObjectStorage) IterEncodedObjects(t plumbing.ObjectType) (EncodedOb func (o *MockObjectStorage) Begin() Transaction { return nil } - -func (o *MockObjectStorage) Close() error { - return nil -} diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go index b1408b7c2..6feb6ae14 100644 --- a/storage/filesystem/object_test.go +++ b/storage/filesystem/object_test.go @@ -48,6 +48,37 @@ func (s *FsSuite) TestGetFromPackfile(c *C) { }) } +func (s *FsSuite) TestGetFromPackfileKeepDescriptors(c *C) { + fixtures.Basic().ByTag(".git").Test(c, func(f *fixtures.Fixture) { + fs := f.DotGit() + dg := dotgit.NewWithOptions(fs, dotgit.Options{KeepDescriptors: true}) + o, err := NewObjectStorageWithOptions(dg, Options{KeepDescriptors: true}) + c.Assert(err, IsNil) + + expected := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") + obj, err := o.EncodedObject(plumbing.AnyObject, expected) + c.Assert(err, IsNil) + c.Assert(obj.Hash(), Equals, expected) + + packfiles, err := dg.ObjectPacks() + c.Assert(err, IsNil) + + pack1, err := dg.ObjectPack(packfiles[0]) + c.Assert(err, IsNil) + + err = o.Close() + c.Assert(err, IsNil) + + pack2, err := dg.ObjectPack(packfiles[0]) + c.Assert(err, IsNil) + c.Assert(pack1, Not(Equals), pack2) + + err = o.Close() + c.Assert(err, IsNil) + + }) +} + func (s *FsSuite) TestGetFromPackfileMultiplePackfiles(c *C) { fs := fixtures.ByTag(".git").ByTag("multi-packfile").One().DotGit() o, err := NewObjectStorage(dotgit.New(fs)) diff --git a/storage/memory/storage.go b/storage/memory/storage.go index 3d3b348bf..2e3250905 100644 --- a/storage/memory/storage.go +++ b/storage/memory/storage.go @@ -183,9 +183,6 @@ func (o *ObjectStorage) ObjectPacks() ([]plumbing.Hash, error) { func (o *ObjectStorage) DeleteOldObjectPackAndIndex(plumbing.Hash, time.Time) error { return nil } -func (s *ObjectStorage) Close() error { - return nil -} var errNotSupported = fmt.Errorf("Not supported") From 8176f084d861891d1846a2d46bf669d0d3463ebd Mon Sep 17 00:00:00 2001 From: Javi Fontan Date: Thu, 6 Sep 2018 19:09:02 +0200 Subject: [PATCH 27/30] storage/filesystem: compare files using offset in test Using equals to compare files it uses diff to do so. This can potentially consume lots of ram. Changed the comparison to use file offsets. If the descriptor is reused the offset is maintained. Signed-off-by: Javi Fontan --- storage/filesystem/dotgit/dotgit_test.go | 15 +++++++++++++-- storage/filesystem/object_test.go | 8 +++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/storage/filesystem/dotgit/dotgit_test.go b/storage/filesystem/dotgit/dotgit_test.go index 50f8e649a..308c6b70f 100644 --- a/storage/filesystem/dotgit/dotgit_test.go +++ b/storage/filesystem/dotgit/dotgit_test.go @@ -474,16 +474,27 @@ func (s *SuiteDotGit) TestObjectPackWithKeepDescriptors(c *C) { c.Assert(err, IsNil) c.Assert(filepath.Ext(pack.Name()), Equals, ".pack") + // Move to an specific offset + pack.Seek(42, os.SEEK_SET) + pack2, err := dir.ObjectPack(f.PackfileHash) c.Assert(err, IsNil) - c.Assert(pack, Equals, pack2) + + // If the file is the same the offset should be the same + offset, err := pack2.Seek(0, os.SEEK_CUR) + c.Assert(err, IsNil) + c.Assert(offset, Equals, int64(42)) err = dir.Close() c.Assert(err, IsNil) pack2, err = dir.ObjectPack(f.PackfileHash) c.Assert(err, IsNil) - c.Assert(pack, Not(Equals), pack2) + + // If the file is opened again its offset should be 0 + offset, err = pack2.Seek(0, os.SEEK_CUR) + c.Assert(err, IsNil) + c.Assert(offset, Equals, int64(0)) err = pack2.Close() c.Assert(err, IsNil) diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go index 6feb6ae14..4a921a9e6 100644 --- a/storage/filesystem/object_test.go +++ b/storage/filesystem/object_test.go @@ -2,6 +2,7 @@ package filesystem import ( "io/ioutil" + "os" "testing" "gopkg.in/src-d/go-git.v4/plumbing" @@ -66,12 +67,17 @@ func (s *FsSuite) TestGetFromPackfileKeepDescriptors(c *C) { pack1, err := dg.ObjectPack(packfiles[0]) c.Assert(err, IsNil) + pack1.Seek(42, os.SEEK_SET) + err = o.Close() c.Assert(err, IsNil) pack2, err := dg.ObjectPack(packfiles[0]) c.Assert(err, IsNil) - c.Assert(pack1, Not(Equals), pack2) + + offset, err := pack2.Seek(0, os.SEEK_CUR) + c.Assert(err, IsNil) + c.Assert(offset, Equals, int64(0)) err = o.Close() c.Assert(err, IsNil) From a4b12e4161738af6f724776c0c8c55f90542f06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= Date: Fri, 7 Sep 2018 10:25:23 +0200 Subject: [PATCH 28/30] plumbing/transport: ssh check if list of known_hosts files is empty Signed-off-by: kuba-- --- plumbing/transport/ssh/auth_method.go | 14 ++--- plumbing/transport/ssh/auth_method_test.go | 62 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/plumbing/transport/ssh/auth_method.go b/plumbing/transport/ssh/auth_method.go index 84cfab2a6..dbb47c56b 100644 --- a/plumbing/transport/ssh/auth_method.go +++ b/plumbing/transport/ssh/auth_method.go @@ -236,7 +236,7 @@ func (a *PublicKeysCallback) ClientConfig() (*ssh.ClientConfig, error) { // NewKnownHostsCallback returns ssh.HostKeyCallback based on a file based on a // known_hosts file. http://man.openbsd.org/sshd#SSH_KNOWN_HOSTS_FILE_FORMAT // -// If files is empty, the list of files will be read from the SSH_KNOWN_HOSTS +// If list of files is empty, then it will be read from the SSH_KNOWN_HOSTS // environment variable, example: // /home/foo/custom_known_hosts_file:/etc/custom_known/hosts_file // @@ -244,13 +244,15 @@ func (a *PublicKeysCallback) ClientConfig() (*ssh.ClientConfig, error) { // ~/.ssh/known_hosts // /etc/ssh/ssh_known_hosts func NewKnownHostsCallback(files ...string) (ssh.HostKeyCallback, error) { - files, err := getDefaultKnownHostsFiles() - if err != nil { - return nil, err + var err error + + if len(files) == 0 { + if files, err = getDefaultKnownHostsFiles(); err != nil { + return nil, err + } } - files, err = filterKnownHostsFiles(files...) - if err != nil { + if files, err = filterKnownHostsFiles(files...); err != nil { return nil, err } diff --git a/plumbing/transport/ssh/auth_method_test.go b/plumbing/transport/ssh/auth_method_test.go index 00256694f..0cde61eea 100644 --- a/plumbing/transport/ssh/auth_method_test.go +++ b/plumbing/transport/ssh/auth_method_test.go @@ -1,16 +1,30 @@ package ssh import ( + "bufio" "fmt" "io/ioutil" "os" + "strings" + "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/testdata" . "gopkg.in/check.v1" ) -type SuiteCommon struct{} +type ( + SuiteCommon struct{} + + mockKnownHosts struct{} +) + +func (mockKnownHosts) host() string { return "github.com" } +func (mockKnownHosts) knownHosts() []byte { + return []byte(`github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==`) +} +func (mockKnownHosts) Network() string { return "tcp" } +func (mockKnownHosts) String() string { return "github.com:22" } var _ = Suite(&SuiteCommon{}) @@ -149,3 +163,49 @@ func (*SuiteCommon) TestNewPublicKeysWithInvalidPEM(c *C) { c.Assert(err, NotNil) c.Assert(auth, IsNil) } + +func (*SuiteCommon) TestNewKnownHostsCallback(c *C) { + var mock = mockKnownHosts{} + + f, err := ioutil.TempFile("", "known-hosts") + c.Assert(err, IsNil) + + _, err = f.Write(mock.knownHosts()) + c.Assert(err, IsNil) + + err = f.Close() + c.Assert(err, IsNil) + + defer os.RemoveAll(f.Name()) + + f, err = os.Open(f.Name()) + c.Assert(err, IsNil) + + defer f.Close() + + var hostKey ssh.PublicKey + scanner := bufio.NewScanner(f) + for scanner.Scan() { + fields := strings.Split(scanner.Text(), " ") + if len(fields) != 3 { + continue + } + if strings.Contains(fields[0], mock.host()) { + var err error + hostKey, _, _, _, err = ssh.ParseAuthorizedKey(scanner.Bytes()) + if err != nil { + c.Fatalf("error parsing %q: %v", fields[2], err) + } + break + } + } + if hostKey == nil { + c.Fatalf("no hostkey for %s", mock.host()) + } + + clb, err := NewKnownHostsCallback(f.Name()) + c.Assert(err, IsNil) + + err = clb(mock.String(), mock, hostKey) + c.Assert(err, IsNil) +} From 80170bd73d5d6298ea6d40c66987fcde8148f1e8 Mon Sep 17 00:00:00 2001 From: Antonio Jesus Navarro Perez Date: Fri, 7 Sep 2018 10:50:31 +0200 Subject: [PATCH 29/30] Fix fatal corrupt patch in unified diff format Signed-off-by: Antonio Jesus Navarro Perez --- plumbing/format/diff/unified_encoder.go | 8 +++-- plumbing/format/diff/unified_encoder_test.go | 37 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/plumbing/format/diff/unified_encoder.go b/plumbing/format/diff/unified_encoder.go index 58edd9516..8bd6d8abc 100644 --- a/plumbing/format/diff/unified_encoder.go +++ b/plumbing/format/diff/unified_encoder.go @@ -237,9 +237,13 @@ func (c *hunksGenerator) addLineNumbers(la, lb int, linesBefore int, i int, op O // we need to search for a reference for the next diff switch { case linesBefore != 0 && c.ctxLines != 0: - clb = lb - c.ctxLines + 1 + if lb > c.ctxLines { + clb = lb - c.ctxLines + 1 + } else { + clb = 1 + } case c.ctxLines == 0: - clb = lb - c.ctxLines + clb = lb case i != len(c.chunks)-1: next := c.chunks[i+1] if next.Type() == op || next.Type() == Equal { diff --git a/plumbing/format/diff/unified_encoder_test.go b/plumbing/format/diff/unified_encoder_test.go index 0e419ca0d..7736af19f 100644 --- a/plumbing/format/diff/unified_encoder_test.go +++ b/plumbing/format/diff/unified_encoder_test.go @@ -150,6 +150,43 @@ var oneChunkPatchInverted Patch = testPatch{ } var fixtures []*fixture = []*fixture{{ + patch: testPatch{ + message: "", + filePatches: []testFilePatch{{ + from: &testFile{ + mode: filemode.Regular, + path: "README.md", + seed: "hello\nworld\n", + }, + to: &testFile{ + mode: filemode.Regular, + path: "README.md", + seed: "hello\nbug\n", + }, + chunks: []testChunk{{ + content: "hello", + op: Equal, + }, { + content: "world", + op: Delete, + }, { + content: "bug", + op: Add, + }}, + }}, + }, + desc: "positive negative number", + context: 2, + diff: `diff --git a/README.md b/README.md +index 94954abda49de8615a048f8d2e64b5de848e27a1..f3dad9514629b9ff9136283ae331ad1fc95748a8 100644 +--- a/README.md ++++ b/README.md +@@ -1,2 +1,2 @@ + hello +-world ++bug +`, +}, { patch: testPatch{ message: "", filePatches: []testFilePatch{{ From 8f6b3127c1ff7661113fff2662416c328971a285 Mon Sep 17 00:00:00 2001 From: kuba-- Date: Fri, 7 Sep 2018 09:27:35 +0200 Subject: [PATCH 30/30] Expose Storage cache. Signed-off-by: kuba-- --- common_test.go | 11 ++---- .../format/packfile/encoder_advanced_test.go | 7 ++-- plumbing/object/change_adaptor_test.go | 4 +-- plumbing/object/change_test.go | 7 ++-- plumbing/object/commit_test.go | 4 +-- plumbing/object/difftree_test.go | 4 +-- plumbing/object/file_test.go | 17 ++++----- plumbing/object/object_test.go | 4 +-- plumbing/object/patch_test.go | 5 +-- plumbing/object/tag_test.go | 5 ++- plumbing/object/tree_test.go | 4 +-- plumbing/revlist/revlist_test.go | 12 +++---- plumbing/transport/server/loader.go | 3 +- plumbing/transport/server/server_test.go | 4 +-- prune_test.go | 4 +-- remote_test.go | 35 +++++++------------ repository.go | 11 ++---- repository_test.go | 30 ++++++---------- storage/filesystem/dotgit/dotgit.go | 4 +-- storage/filesystem/module.go | 3 +- storage/filesystem/object.go | 29 ++++++--------- storage/filesystem/object_test.go | 25 +++++-------- storage/filesystem/storage.go | 29 ++++++++------- storage/filesystem/storage_test.go | 11 +++--- worktree_commit_test.go | 4 +-- 25 files changed, 111 insertions(+), 165 deletions(-) diff --git a/common_test.go b/common_test.go index efe1ecc92..dad0a3777 100644 --- a/common_test.go +++ b/common_test.go @@ -4,6 +4,7 @@ import ( "testing" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" "gopkg.in/src-d/go-git.v4/plumbing/transport" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -59,10 +60,7 @@ func (s *BaseSuite) NewRepository(f *fixtures.Fixture) *Repository { dotgit = f.DotGit() worktree = memfs.New() - st, err := filesystem.NewStorage(dotgit) - if err != nil { - panic(err) - } + st := filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()) r, err := Open(st, worktree) if err != nil { @@ -89,10 +87,7 @@ func (s *BaseSuite) NewRepositoryWithEmptyWorktree(f *fixtures.Fixture) *Reposit worktree := memfs.New() - st, err := filesystem.NewStorage(dotgit) - if err != nil { - panic(err) - } + st := filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()) r, err := Open(st, worktree) if err != nil { diff --git a/plumbing/format/packfile/encoder_advanced_test.go b/plumbing/format/packfile/encoder_advanced_test.go index fc1419eea..e15126e66 100644 --- a/plumbing/format/packfile/encoder_advanced_test.go +++ b/plumbing/format/packfile/encoder_advanced_test.go @@ -8,6 +8,7 @@ import ( "gopkg.in/src-d/go-billy.v4/memfs" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/format/idxfile" . "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" "gopkg.in/src-d/go-git.v4/plumbing/storer" @@ -32,8 +33,7 @@ func (s *EncoderAdvancedSuite) TestEncodeDecode(c *C) { fixs = append(fixs, fixtures.ByURL("https://github.com/src-d/go-git.git"). ByTag("packfile").ByTag(".git").One()) fixs.Test(c, func(f *fixtures.Fixture) { - storage, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + storage := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) s.testEncodeDecode(c, storage, 10) }) } @@ -47,8 +47,7 @@ func (s *EncoderAdvancedSuite) TestEncodeDecodeNoDeltaCompression(c *C) { fixs = append(fixs, fixtures.ByURL("https://github.com/src-d/go-git.git"). ByTag("packfile").ByTag(".git").One()) fixs.Test(c, func(f *fixtures.Fixture) { - storage, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + storage := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) s.testEncodeDecode(c, storage, 0) }) } diff --git a/plumbing/object/change_adaptor_test.go b/plumbing/object/change_adaptor_test.go index 803c3b89a..c7c003b96 100644 --- a/plumbing/object/change_adaptor_test.go +++ b/plumbing/object/change_adaptor_test.go @@ -4,6 +4,7 @@ import ( "sort" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -23,8 +24,7 @@ type ChangeAdaptorSuite struct { func (s *ChangeAdaptorSuite) SetUpSuite(c *C) { s.Suite.SetUpSuite(c) s.Fixture = fixtures.Basic().One() - sto, err := filesystem.NewStorage(s.Fixture.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(s.Fixture.DotGit(), cache.NewObjectLRUDefault()) s.Storer = sto } diff --git a/plumbing/object/change_test.go b/plumbing/object/change_test.go index b0e89c711..e2f0a2346 100644 --- a/plumbing/object/change_test.go +++ b/plumbing/object/change_test.go @@ -5,6 +5,7 @@ import ( "sort" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/format/diff" "gopkg.in/src-d/go-git.v4/plumbing/storer" @@ -25,8 +26,7 @@ func (s *ChangeSuite) SetUpSuite(c *C) { s.Suite.SetUpSuite(c) s.Fixture = fixtures.ByURL("https://github.com/src-d/go-git.git"). ByTag(".git").One() - sto, err := filesystem.NewStorage(s.Fixture.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(s.Fixture.DotGit(), cache.NewObjectLRUDefault()) s.Storer = sto } @@ -253,8 +253,7 @@ func (s *ChangeSuite) TestNoFileFilemodes(c *C) { s.Suite.SetUpSuite(c) f := fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) iter, err := sto.IterEncodedObjects(plumbing.AnyObject) c.Assert(err, IsNil) diff --git a/plumbing/object/commit_test.go b/plumbing/object/commit_test.go index e72b703d1..c9acf42f3 100644 --- a/plumbing/object/commit_test.go +++ b/plumbing/object/commit_test.go @@ -8,6 +8,7 @@ import ( "time" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" . "gopkg.in/check.v1" "gopkg.in/src-d/go-git-fixtures.v3" @@ -247,8 +248,7 @@ func (s *SuiteCommit) TestStringMultiLine(c *C) { hash := plumbing.NewHash("e7d896db87294e33ca3202e536d4d9bb16023db3") f := fixtures.ByURL("https://github.com/src-d/go-git.git").One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) o, err := sto.EncodedObject(plumbing.CommitObject, hash) c.Assert(err, IsNil) diff --git a/plumbing/object/difftree_test.go b/plumbing/object/difftree_test.go index ff9ecbc3f..4af86840a 100644 --- a/plumbing/object/difftree_test.go +++ b/plumbing/object/difftree_test.go @@ -4,6 +4,7 @@ import ( "sort" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" "gopkg.in/src-d/go-git.v4/plumbing/storer" @@ -25,8 +26,7 @@ type DiffTreeSuite struct { func (s *DiffTreeSuite) SetUpSuite(c *C) { s.Suite.SetUpSuite(c) s.Fixture = fixtures.Basic().One() - sto, err := filesystem.NewStorage(s.Fixture.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(s.Fixture.DotGit(), cache.NewObjectLRUDefault()) s.Storer = sto s.cache = make(map[string]storer.EncodedObjectStorer) } diff --git a/plumbing/object/file_test.go b/plumbing/object/file_test.go index edb82d01c..4b92749cb 100644 --- a/plumbing/object/file_test.go +++ b/plumbing/object/file_test.go @@ -4,6 +4,7 @@ import ( "io" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -44,8 +45,7 @@ var fileIterTests = []struct { func (s *FileSuite) TestIter(c *C) { for i, t := range fileIterTests { f := fixtures.ByURL(t.repo).One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) h := plumbing.NewHash(t.commit) commit, err := GetCommit(sto, h) @@ -106,8 +106,7 @@ hs_err_pid* func (s *FileSuite) TestContents(c *C) { for i, t := range contentsTests { f := fixtures.ByURL(t.repo).One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) h := plumbing.NewHash(t.commit) commit, err := GetCommit(sto, h) @@ -160,8 +159,7 @@ var linesTests = []struct { func (s *FileSuite) TestLines(c *C) { for i, t := range linesTests { f := fixtures.ByURL(t.repo).One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) h := plumbing.NewHash(t.commit) commit, err := GetCommit(sto, h) @@ -195,8 +193,7 @@ var ignoreEmptyDirEntriesTests = []struct { func (s *FileSuite) TestIgnoreEmptyDirEntries(c *C) { for i, t := range ignoreEmptyDirEntriesTests { f := fixtures.ByURL(t.repo).One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) h := plumbing.NewHash(t.commit) commit, err := GetCommit(sto, h) @@ -251,9 +248,7 @@ func (s *FileSuite) TestFileIter(c *C) { func (s *FileSuite) TestFileIterSubmodule(c *C) { dotgit := fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One().DotGit() - st, err := filesystem.NewStorage(dotgit) - - c.Assert(err, IsNil) + st := filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()) hash := plumbing.NewHash("b685400c1f9316f350965a5993d350bc746b0bf4") commit, err := GetCommit(st, hash) diff --git a/plumbing/object/object_test.go b/plumbing/object/object_test.go index 68aa1a13e..8f0eededd 100644 --- a/plumbing/object/object_test.go +++ b/plumbing/object/object_test.go @@ -7,6 +7,7 @@ import ( "time" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -26,8 +27,7 @@ type BaseObjectsSuite struct { func (s *BaseObjectsSuite) SetUpSuite(c *C) { s.Suite.SetUpSuite(c) s.Fixture = fixtures.Basic().One() - storer, err := filesystem.NewStorage(s.Fixture.DotGit()) - c.Assert(err, IsNil) + storer := filesystem.NewStorage(s.Fixture.DotGit(), cache.NewObjectLRUDefault()) s.Storer = storer } diff --git a/plumbing/object/patch_test.go b/plumbing/object/patch_test.go index 8eb65ec30..47057fba7 100644 --- a/plumbing/object/patch_test.go +++ b/plumbing/object/patch_test.go @@ -4,6 +4,7 @@ import ( . "gopkg.in/check.v1" fixtures "gopkg.in/src-d/go-git-fixtures.v3" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/storage/filesystem" ) @@ -14,8 +15,8 @@ type PatchSuite struct { var _ = Suite(&PatchSuite{}) func (s *PatchSuite) TestStatsWithSubmodules(c *C) { - storer, err := filesystem.NewStorage( - fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One().DotGit()) + storer := filesystem.NewStorage( + fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One().DotGit(), cache.NewObjectLRUDefault()) commit, err := GetCommit(storer, plumbing.NewHash("b685400c1f9316f350965a5993d350bc746b0bf4")) diff --git a/plumbing/object/tag_test.go b/plumbing/object/tag_test.go index e7dd06e21..59c28b022 100644 --- a/plumbing/object/tag_test.go +++ b/plumbing/object/tag_test.go @@ -7,6 +7,7 @@ import ( "time" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/storage/filesystem" "gopkg.in/src-d/go-git.v4/storage/memory" @@ -22,9 +23,7 @@ var _ = Suite(&TagSuite{}) func (s *TagSuite) SetUpSuite(c *C) { s.BaseObjectsSuite.SetUpSuite(c) - storer, err := filesystem.NewStorage( - fixtures.ByURL("https://github.com/git-fixtures/tags.git").One().DotGit()) - c.Assert(err, IsNil) + storer := filesystem.NewStorage(fixtures.ByURL("https://github.com/git-fixtures/tags.git").One().DotGit(), cache.NewObjectLRUDefault()) s.Storer = storer } diff --git a/plumbing/object/tree_test.go b/plumbing/object/tree_test.go index 59d5d215f..736642186 100644 --- a/plumbing/object/tree_test.go +++ b/plumbing/object/tree_test.go @@ -5,6 +5,7 @@ import ( "io" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/filemode" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -341,8 +342,7 @@ func (s *TreeSuite) TestTreeWalkerNextNonRecursive(c *C) { func (s *TreeSuite) TestTreeWalkerNextSubmodule(c *C) { dotgit := fixtures.ByURL("https://github.com/git-fixtures/submodule.git").One().DotGit() - st, err := filesystem.NewStorage(dotgit) - c.Assert(err, IsNil) + st := filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()) hash := plumbing.NewHash("b685400c1f9316f350965a5993d350bc746b0bf4") commit, err := GetCommit(st, hash) diff --git a/plumbing/revlist/revlist_test.go b/plumbing/revlist/revlist_test.go index 55d9bca2a..dea1c73d9 100644 --- a/plumbing/revlist/revlist_test.go +++ b/plumbing/revlist/revlist_test.go @@ -4,6 +4,7 @@ import ( "testing" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -51,8 +52,7 @@ const ( func (s *RevListSuite) SetUpTest(c *C) { s.Suite.SetUpSuite(c) - sto, err := filesystem.NewStorage(fixtures.Basic().One().DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fixtures.Basic().One().DotGit(), cache.NewObjectLRUDefault()) s.Storer = sto } @@ -67,8 +67,7 @@ func (s *RevListSuite) TestRevListObjects_Submodules(c *C) { "6ecf0ef2c2dffb796033e5a02219af86ec6584e5": true, } - sto, err := filesystem.NewStorage(fixtures.ByTag("submodule").One().DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fixtures.ByTag("submodule").One().DotGit(), cache.NewObjectLRUDefault()) ref, err := storer.ResolveReference(sto, plumbing.HEAD) c.Assert(err, IsNil) @@ -109,10 +108,9 @@ func (s *RevListSuite) TestRevListObjects(c *C) { } func (s *RevListSuite) TestRevListObjectsTagObject(c *C) { - sto, err := filesystem.NewStorage( + sto := filesystem.NewStorage( fixtures.ByTag("tags"). - ByURL("https://github.com/git-fixtures/tags.git").One().DotGit()) - c.Assert(err, IsNil) + ByURL("https://github.com/git-fixtures/tags.git").One().DotGit(), cache.NewObjectLRUDefault()) expected := map[string]bool{ "70846e9a10ef7b41064b40f07713d5b8b9a8fc73": true, diff --git a/plumbing/transport/server/loader.go b/plumbing/transport/server/loader.go index c83752c25..13b35262d 100644 --- a/plumbing/transport/server/loader.go +++ b/plumbing/transport/server/loader.go @@ -1,6 +1,7 @@ package server import ( + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/plumbing/transport" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -43,7 +44,7 @@ func (l *fsLoader) Load(ep *transport.Endpoint) (storer.Storer, error) { return nil, transport.ErrRepositoryNotFound } - return filesystem.NewStorage(fs) + return filesystem.NewStorage(fs, cache.NewObjectLRUDefault()), nil } // MapLoader is a Loader that uses a lookup map of storer.Storer by diff --git a/plumbing/transport/server/server_test.go b/plumbing/transport/server/server_test.go index 33d74d19f..302ff486e 100644 --- a/plumbing/transport/server/server_test.go +++ b/plumbing/transport/server/server_test.go @@ -3,6 +3,7 @@ package server_test import ( "testing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/transport" "gopkg.in/src-d/go-git.v4/plumbing/transport/client" "gopkg.in/src-d/go-git.v4/plumbing/transport/server" @@ -53,8 +54,7 @@ func (s *BaseSuite) prepareRepositories(c *C) { fs := fixtures.Basic().One().DotGit() s.Endpoint, err = transport.NewEndpoint(fs.Root()) c.Assert(err, IsNil) - s.loader[s.Endpoint.String()], err = filesystem.NewStorage(fs) - c.Assert(err, IsNil) + s.loader[s.Endpoint.String()] = filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) s.EmptyEndpoint, err = transport.NewEndpoint("/empty.git") c.Assert(err, IsNil) diff --git a/prune_test.go b/prune_test.go index 60652ec9f..670cd07bd 100644 --- a/prune_test.go +++ b/prune_test.go @@ -4,6 +4,7 @@ import ( "time" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -22,8 +23,7 @@ func (s *PruneSuite) testPrune(c *C, deleteTime time.Time) { srcFs := fixtures.ByTag("unpacked").One().DotGit() var sto storage.Storer var err error - sto, err = filesystem.NewStorage(srcFs) - c.Assert(err, IsNil) + sto = filesystem.NewStorage(srcFs, cache.NewObjectLRUDefault()) los := sto.(storer.LooseObjectStorer) c.Assert(los, NotNil) diff --git a/remote_test.go b/remote_test.go index dd386b083..175faed36 100644 --- a/remote_test.go +++ b/remote_test.go @@ -9,6 +9,7 @@ import ( "gopkg.in/src-d/go-git.v4/config" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/protocol/packp" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage" @@ -238,7 +239,7 @@ func (s *RemoteSuite) TestFetchWithPackfileWriter(c *C) { defer os.RemoveAll(dir) // clean up - fss, err := filesystem.NewStorage(osfs.New(dir)) + fss := filesystem.NewStorage(osfs.New(dir), cache.NewObjectLRUDefault()) c.Assert(err, IsNil) mock := &mockPackfileWriter{Storer: fss} @@ -375,8 +376,7 @@ func (s *RemoteSuite) TestFetchFastForwardFS(c *C) { defer os.RemoveAll(dir) // clean up - fss, err := filesystem.NewStorage(osfs.New(dir)) - c.Assert(err, IsNil) + fss := filesystem.NewStorage(osfs.New(dir), cache.NewObjectLRUDefault()) // This exercises `storage.filesystem.Storage.CheckAndSetReference()`. s.testFetchFastForward(c, fss) @@ -400,8 +400,7 @@ func (s *RemoteSuite) TestPushToEmptyRepository(c *C) { c.Assert(err, IsNil) srcFs := fixtures.Basic().One().DotGit() - sto, err := filesystem.NewStorage(srcFs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(srcFs, cache.NewObjectLRUDefault()) r := newRemote(sto, &config.RemoteConfig{ Name: DefaultRemoteName, @@ -438,8 +437,7 @@ func (s *RemoteSuite) TestPushContext(c *C) { c.Assert(err, IsNil) fs := fixtures.ByURL("https://github.com/git-fixtures/tags.git").One().DotGit() - sto, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r := newRemote(sto, &config.RemoteConfig{ Name: DefaultRemoteName, @@ -461,8 +459,7 @@ func (s *RemoteSuite) TestPushTags(c *C) { c.Assert(err, IsNil) fs := fixtures.ByURL("https://github.com/git-fixtures/tags.git").One().DotGit() - sto, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r := newRemote(sto, &config.RemoteConfig{ Name: DefaultRemoteName, @@ -485,15 +482,14 @@ func (s *RemoteSuite) TestPushTags(c *C) { func (s *RemoteSuite) TestPushNoErrAlreadyUpToDate(c *C) { fs := fixtures.Basic().One().DotGit() - sto, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r := newRemote(sto, &config.RemoteConfig{ Name: DefaultRemoteName, URLs: []string{fs.Root()}, }) - err = r.Push(&PushOptions{ + err := r.Push(&PushOptions{ RefSpecs: []config.RefSpec{"refs/heads/*:refs/heads/*"}, }) c.Assert(err, Equals, NoErrAlreadyUpToDate) @@ -501,8 +497,7 @@ func (s *RemoteSuite) TestPushNoErrAlreadyUpToDate(c *C) { func (s *RemoteSuite) TestPushDeleteReference(c *C) { fs := fixtures.Basic().One().DotGit() - sto, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r, err := PlainClone(c.MkDir(), true, &CloneOptions{ URL: fs.Root(), @@ -526,8 +521,7 @@ func (s *RemoteSuite) TestPushDeleteReference(c *C) { func (s *RemoteSuite) TestPushRejectNonFastForward(c *C) { fs := fixtures.Basic().One().DotGit() - server, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + server := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r, err := PlainClone(c.MkDir(), true, &CloneOptions{ URL: fs.Root(), @@ -554,12 +548,10 @@ func (s *RemoteSuite) TestPushRejectNonFastForward(c *C) { func (s *RemoteSuite) TestPushForce(c *C) { f := fixtures.Basic().One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) dstFs := f.DotGit() - dstSto, err := filesystem.NewStorage(dstFs) - c.Assert(err, IsNil) + dstSto := filesystem.NewStorage(dstFs, cache.NewObjectLRUDefault()) url := dstFs.Root() r := newRemote(sto, &config.RemoteConfig{ @@ -703,8 +695,7 @@ func (s *RemoteSuite) TestPushWrongRemoteName(c *C) { func (s *RemoteSuite) TestGetHaves(c *C) { f := fixtures.Basic().One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) var localRefs = []*plumbing.Reference{ plumbing.NewReferenceFromStrings( diff --git a/repository.go b/repository.go index f619934a4..bfe06a369 100644 --- a/repository.go +++ b/repository.go @@ -13,6 +13,7 @@ import ( "gopkg.in/src-d/go-git.v4/config" "gopkg.in/src-d/go-git.v4/internal/revision" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/format/packfile" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" @@ -220,10 +221,7 @@ func PlainInit(path string, isBare bool) (*Repository, error) { dot, _ = wt.Chroot(GitDirName) } - s, err := filesystem.NewStorage(dot) - if err != nil { - return nil, err - } + s := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) return Init(s, wt) } @@ -251,10 +249,7 @@ func PlainOpenWithOptions(path string, o *PlainOpenOptions) (*Repository, error) return nil, err } - s, err := filesystem.NewStorage(dot) - if err != nil { - return nil, err - } + s := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) return Open(s, wt) } diff --git a/repository_test.go b/repository_test.go index 261af7a7b..88071cfd5 100644 --- a/repository_test.go +++ b/repository_test.go @@ -15,6 +15,7 @@ import ( "gopkg.in/src-d/go-git.v4/config" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage" @@ -51,8 +52,7 @@ func (s *RepositorySuite) TestInitNonStandardDotGit(c *C) { fs := osfs.New(dir) dot, _ := fs.Chroot("storage") - storage, err := filesystem.NewStorage(dot) - c.Assert(err, IsNil) + storage := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) wt, _ := fs.Chroot("worktree") r, err := Init(storage, wt) @@ -78,8 +78,7 @@ func (s *RepositorySuite) TestInitStandardDotGit(c *C) { fs := osfs.New(dir) dot, _ := fs.Chroot(".git") - storage, err := filesystem.NewStorage(dot) - c.Assert(err, IsNil) + storage := filesystem.NewStorage(dot, cache.NewObjectLRUDefault()) r, err := Init(storage, fs) c.Assert(err, IsNil) @@ -1058,8 +1057,7 @@ func (s *RepositorySuite) TestPushDepth(c *C) { func (s *RepositorySuite) TestPushNonExistentRemote(c *C) { srcFs := fixtures.Basic().One().DotGit() - sto, err := filesystem.NewStorage(srcFs) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(srcFs, cache.NewObjectLRUDefault()) r, err := Open(sto, srcFs) c.Assert(err, IsNil) @@ -1277,8 +1275,7 @@ func (s *RepositorySuite) TestTags(c *C) { func (s *RepositorySuite) TestBranches(c *C) { f := fixtures.ByURL("https://github.com/git-fixtures/root-references.git").One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) r, err := Open(sto, f.DotGit()) c.Assert(err, IsNil) @@ -1495,8 +1492,7 @@ func (s *RepositorySuite) TestWorktreeBare(c *C) { func (s *RepositorySuite) TestResolveRevision(c *C) { f := fixtures.ByURL("https://github.com/git-fixtures/basic.git").One() - sto, err := filesystem.NewStorage(f.DotGit()) - c.Assert(err, IsNil) + sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) r, err := Open(sto, f.DotGit()) c.Assert(err, IsNil) @@ -1548,9 +1544,9 @@ func (s *RepositorySuite) TestResolveRevisionWithErrors(c *C) { c.Assert(err, IsNil) datas := map[string]string{ - "efs/heads/master~": "reference not found", - "HEAD^3": `Revision invalid : "3" found must be 0, 1 or 2 after "^"`, - "HEAD^{/whatever}": `No commit message match regexp : "whatever"`, + "efs/heads/master~": "reference not found", + "HEAD^3": `Revision invalid : "3" found must be 0, 1 or 2 after "^"`, + "HEAD^{/whatever}": `No commit message match regexp : "whatever"`, "4e1243bd22c66e76c2ba9eddc1f91394e57f9f83": "reference not found", "918c48b83bd081e863dbe1b80f8998f058cd8294": `refname "918c48b83bd081e863dbe1b80f8998f058cd8294" is ambiguous`, } @@ -1567,8 +1563,7 @@ func (s *RepositorySuite) testRepackObjects( srcFs := fixtures.ByTag("unpacked").One().DotGit() var sto storage.Storer var err error - sto, err = filesystem.NewStorage(srcFs) - c.Assert(err, IsNil) + sto = filesystem.NewStorage(srcFs, cache.NewObjectLRUDefault()) los := sto.(storer.LooseObjectStorer) c.Assert(los, NotNil) @@ -1733,10 +1728,7 @@ func BenchmarkObjects(b *testing.B) { b.Run(f.URL, func(b *testing.B) { fs := f.DotGit() - storer, err := filesystem.NewStorage(fs) - if err != nil { - b.Fatal(err) - } + storer := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) worktree, err := fs.Chroot(filepath.Dir(fs.Root())) if err != nil { diff --git a/storage/filesystem/dotgit/dotgit.go b/storage/filesystem/dotgit/dotgit.go index df5cd10d7..a58c2482a 100644 --- a/storage/filesystem/dotgit/dotgit.go +++ b/storage/filesystem/dotgit/dotgit.go @@ -92,8 +92,8 @@ func New(fs billy.Filesystem) *DotGit { return NewWithOptions(fs, Options{}) } -// NewWithOptions creates a new DotGit and sets non default configuration -// options. See New for complete help. +// NewWithOptions sets non default configuration options. +// See New for complete help. func NewWithOptions(fs billy.Filesystem, o Options) *DotGit { return &DotGit{ options: o, diff --git a/storage/filesystem/module.go b/storage/filesystem/module.go index 7c8c8d866..927220674 100644 --- a/storage/filesystem/module.go +++ b/storage/filesystem/module.go @@ -1,6 +1,7 @@ package filesystem import ( + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/storage" "gopkg.in/src-d/go-git.v4/storage/filesystem/dotgit" ) @@ -15,5 +16,5 @@ func (s *ModuleStorage) Module(name string) (storage.Storer, error) { return nil, err } - return NewStorage(fs) + return NewStorage(fs, cache.NewObjectLRUDefault()), nil } diff --git a/storage/filesystem/object.go b/storage/filesystem/object.go index 3545e2751..9eb085fd3 100644 --- a/storage/filesystem/object.go +++ b/storage/filesystem/object.go @@ -27,24 +27,18 @@ type ObjectStorage struct { index map[plumbing.Hash]idxfile.Index } -// NewObjectStorage creates a new ObjectStorage with the given .git directory. -func NewObjectStorage(dir *dotgit.DotGit) (ObjectStorage, error) { - return NewObjectStorageWithOptions(dir, Options{}) -} - -// NewObjectStorageWithOptions creates a new ObjectStorage with the given .git -// directory and sets its options. -func NewObjectStorageWithOptions( - dir *dotgit.DotGit, - ops Options, -) (ObjectStorage, error) { - s := ObjectStorage{ +// NewObjectStorage creates a new ObjectStorage with the given .git directory and cache. +func NewObjectStorage(dir *dotgit.DotGit, cache cache.Object) *ObjectStorage { + return NewObjectStorageWithOptions(dir, cache, Options{}) +} + +// NewObjectStorageWithOptions creates a new ObjectStorage with the given .git directory, cache and extra options +func NewObjectStorageWithOptions(dir *dotgit.DotGit, cache cache.Object, ops Options) *ObjectStorage { + return &ObjectStorage{ options: ops, - deltaBaseCache: cache.NewObjectLRUDefault(), + deltaBaseCache: cache, dir: dir, } - - return s, nil } func (s *ObjectStorage) requireIndex() error { @@ -182,10 +176,7 @@ func (s *ObjectStorage) EncodedObject(t plumbing.ObjectType, h plumbing.Hash) (p // Create a new object storage with the DotGit(s) and check for the // required hash object. Skip when not found. for _, dg := range dotgits { - o, oe := NewObjectStorage(dg) - if oe != nil { - continue - } + o := NewObjectStorage(dg, s.deltaBaseCache) enobj, enerr := o.EncodedObject(t, h) if enerr != nil { continue diff --git a/storage/filesystem/object_test.go b/storage/filesystem/object_test.go index 4a921a9e6..bd4a94b40 100644 --- a/storage/filesystem/object_test.go +++ b/storage/filesystem/object_test.go @@ -6,6 +6,7 @@ import ( "testing" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/storage/filesystem/dotgit" . "gopkg.in/check.v1" @@ -27,8 +28,7 @@ var _ = Suite(&FsSuite{}) func (s *FsSuite) TestGetFromObjectFile(c *C) { fs := fixtures.ByTag(".git").ByTag("unpacked").One().DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - c.Assert(err, IsNil) + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) expected := plumbing.NewHash("f3dfe29d268303fc6e1bbce268605fc99573406e") obj, err := o.EncodedObject(plumbing.AnyObject, expected) @@ -39,8 +39,7 @@ func (s *FsSuite) TestGetFromObjectFile(c *C) { func (s *FsSuite) TestGetFromPackfile(c *C) { fixtures.Basic().ByTag(".git").Test(c, func(f *fixtures.Fixture) { fs := f.DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - c.Assert(err, IsNil) + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) expected := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") obj, err := o.EncodedObject(plumbing.AnyObject, expected) @@ -53,8 +52,7 @@ func (s *FsSuite) TestGetFromPackfileKeepDescriptors(c *C) { fixtures.Basic().ByTag(".git").Test(c, func(f *fixtures.Fixture) { fs := f.DotGit() dg := dotgit.NewWithOptions(fs, dotgit.Options{KeepDescriptors: true}) - o, err := NewObjectStorageWithOptions(dg, Options{KeepDescriptors: true}) - c.Assert(err, IsNil) + o := NewObjectStorageWithOptions(dg, cache.NewObjectLRUDefault(), Options{KeepDescriptors: true}) expected := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") obj, err := o.EncodedObject(plumbing.AnyObject, expected) @@ -87,8 +85,7 @@ func (s *FsSuite) TestGetFromPackfileKeepDescriptors(c *C) { func (s *FsSuite) TestGetFromPackfileMultiplePackfiles(c *C) { fs := fixtures.ByTag(".git").ByTag("multi-packfile").One().DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - c.Assert(err, IsNil) + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) expected := plumbing.NewHash("8d45a34641d73851e01d3754320b33bb5be3c4d3") obj, err := o.getFromPackfile(expected, false) @@ -104,8 +101,7 @@ func (s *FsSuite) TestGetFromPackfileMultiplePackfiles(c *C) { func (s *FsSuite) TestIter(c *C) { fixtures.ByTag(".git").ByTag("packfile").Test(c, func(f *fixtures.Fixture) { fs := f.DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - c.Assert(err, IsNil) + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) iter, err := o.IterEncodedObjects(plumbing.AnyObject) c.Assert(err, IsNil) @@ -125,8 +121,7 @@ func (s *FsSuite) TestIterWithType(c *C) { fixtures.ByTag(".git").Test(c, func(f *fixtures.Fixture) { for _, t := range objectTypes { fs := f.DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - c.Assert(err, IsNil) + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) iter, err := o.IterEncodedObjects(t) c.Assert(err, IsNil) @@ -308,11 +303,7 @@ func BenchmarkGetObjectFromPackfile(b *testing.B) { for _, f := range fixtures.Basic() { b.Run(f.URL, func(b *testing.B) { fs := f.DotGit() - o, err := NewObjectStorage(dotgit.New(fs)) - if err != nil { - b.Fatal(err) - } - + o := NewObjectStorage(dotgit.New(fs), cache.NewObjectLRUDefault()) for i := 0; i < b.N; i++ { expected := plumbing.NewHash("6ecf0ef2c2dffb796033e5a02219af86ec6584e5") obj, err := o.EncodedObject(plumbing.AnyObject, expected) diff --git a/storage/filesystem/storage.go b/storage/filesystem/storage.go index 7fae7897c..14a772abe 100644 --- a/storage/filesystem/storage.go +++ b/storage/filesystem/storage.go @@ -2,6 +2,7 @@ package filesystem import ( + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/storage/filesystem/dotgit" "gopkg.in/src-d/go-billy.v4" @@ -32,38 +33,35 @@ type Options struct { KeepDescriptors bool } -// NewStorage returns a new Storage backed by a given `fs.Filesystem` -func NewStorage(fs billy.Filesystem) (*Storage, error) { - return NewStorageWithOptions(fs, Options{}) +// NewStorage returns a new Storage backed by a given `fs.Filesystem` and cache. +func NewStorage(fs billy.Filesystem, cache cache.Object) *Storage { + return NewStorageWithOptions(fs, cache, Options{}) } -// NewStorageWithOptions returns a new Storage backed by a given `fs.Filesystem` -func NewStorageWithOptions( - fs billy.Filesystem, - ops Options, -) (*Storage, error) { +// NewStorageWithOptions returns a new Storage with extra options, +// backed by a given `fs.Filesystem` and cache. +func NewStorageWithOptions(fs billy.Filesystem, cache cache.Object, ops Options) *Storage { dirOps := dotgit.Options{ ExclusiveAccess: ops.ExclusiveAccess, KeepDescriptors: ops.KeepDescriptors, } - dir := dotgit.NewWithOptions(fs, dirOps) - o, err := NewObjectStorageWithOptions(dir, ops) - if err != nil { - return nil, err - } return &Storage{ fs: fs, dir: dir, - ObjectStorage: o, + ObjectStorage: ObjectStorage{ + options: ops, + deltaBaseCache: cache, + dir: dir, + }, ReferenceStorage: ReferenceStorage{dir: dir}, IndexStorage: IndexStorage{dir: dir}, ShallowStorage: ShallowStorage{dir: dir}, ConfigStorage: ConfigStorage{dir: dir}, ModuleStorage: ModuleStorage{dir: dir}, - }, nil + } } // Filesystem returns the underlying filesystem @@ -71,6 +69,7 @@ func (s *Storage) Filesystem() billy.Filesystem { return s.fs } +// Init initializes .git directory func (s *Storage) Init() error { return s.dir.Initialize() } diff --git a/storage/filesystem/storage_test.go b/storage/filesystem/storage_test.go index 7f85ef548..6fa0d908a 100644 --- a/storage/filesystem/storage_test.go +++ b/storage/filesystem/storage_test.go @@ -4,6 +4,7 @@ import ( "io/ioutil" "testing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/test" @@ -23,8 +24,7 @@ var _ = Suite(&StorageSuite{}) func (s *StorageSuite) SetUpTest(c *C) { s.dir = c.MkDir() - storage, err := NewStorage(osfs.New(s.dir)) - c.Assert(err, IsNil) + storage := NewStorage(osfs.New(s.dir), cache.NewObjectLRUDefault()) setUpTest(s, c, storage) } @@ -44,8 +44,7 @@ func setUpTest(s *StorageSuite, c *C, storage *Storage) { func (s *StorageSuite) TestFilesystem(c *C) { fs := memfs.New() - storage, err := NewStorage(fs) - c.Assert(err, IsNil) + storage := NewStorage(fs, cache.NewObjectLRUDefault()) c.Assert(storage.Filesystem(), Equals, fs) } @@ -64,10 +63,10 @@ var _ = Suite(&StorageExclusiveSuite{}) func (s *StorageExclusiveSuite) SetUpTest(c *C) { s.dir = c.MkDir() - storage, err := NewStorageWithOptions( + storage := NewStorageWithOptions( osfs.New(s.dir), + cache.NewObjectLRUDefault(), Options{ExclusiveAccess: true}) - c.Assert(err, IsNil) setUpTest(&s.StorageSuite, c, storage) } diff --git a/worktree_commit_test.go b/worktree_commit_test.go index 6979bd559..62aae8a2f 100644 --- a/worktree_commit_test.go +++ b/worktree_commit_test.go @@ -9,6 +9,7 @@ import ( "time" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" "gopkg.in/src-d/go-git.v4/storage/filesystem" @@ -205,8 +206,7 @@ func (s *WorktreeSuite) TestCommitTreeSort(c *C) { path, err := ioutil.TempDir(os.TempDir(), "test-commit-tree-sort") c.Assert(err, IsNil) fs := osfs.New(path) - st, err := filesystem.NewStorage(fs) - c.Assert(err, IsNil) + st := filesystem.NewStorage(fs, cache.NewObjectLRUDefault()) r, err := Init(st, nil) c.Assert(err, IsNil)