From 83e914defef47910ef112b1848c706e176439616 Mon Sep 17 00:00:00 2001 From: Antonio Jesus Navarro Perez Date: Wed, 5 Apr 2017 17:13:48 +0200 Subject: object: fix Change.Files() method behavior (fix #317) - If 'from' or 'to' are tree entries that aren't files, Files() method will return nil instead of object not found error. - Added a test checking this using modules fixture. --- plumbing/object/change.go | 8 +++++++ plumbing/object/change_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/plumbing/object/change.go b/plumbing/object/change.go index d29836d..2f702e4 100644 --- a/plumbing/object/change.go +++ b/plumbing/object/change.go @@ -46,6 +46,10 @@ func (c *Change) Files() (from, to *File, err error) { if action == merkletrie.Insert || action == merkletrie.Modify { to, err = c.To.Tree.TreeEntryFile(&c.To.TreeEntry) + if !c.To.TreeEntry.Mode.IsFile() { + return nil, nil, nil + } + if err != nil { return } @@ -53,6 +57,10 @@ func (c *Change) Files() (from, to *File, err error) { if action == merkletrie.Delete || action == merkletrie.Modify { from, err = c.From.Tree.TreeEntryFile(&c.From.TreeEntry) + if !c.From.TreeEntry.Mode.IsFile() { + return nil, nil, nil + } + if err != nil { return } diff --git a/plumbing/object/change_test.go b/plumbing/object/change_test.go index 3bab5d2..005ceb0 100644 --- a/plumbing/object/change_test.go +++ b/plumbing/object/change_test.go @@ -198,6 +198,54 @@ func (s *ChangeSuite) TestEmptyChangeFails(c *C) { c.Assert(str, Equals, "malformed change") } +// test reproducing bug #317 +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) + + iter, err := sto.IterEncodedObjects(plumbing.AnyObject) + c.Assert(err, IsNil) + var commits []*Commit + iter.ForEach(func(o plumbing.EncodedObject) error { + if o.Type() == plumbing.CommitObject { + commit, err := GetCommit(sto, o.Hash()) + c.Assert(err, IsNil) + commits = append(commits, commit) + + } + + return nil + }) + + c.Assert(len(commits), Not(Equals), 0) + + var prev *Commit + for _, commit := range commits { + if prev == nil { + prev = commit + continue + } + tree, err := commit.Tree() + c.Assert(err, IsNil) + prevTree, err := prev.Tree() + c.Assert(err, IsNil) + changes, err := DiffTree(tree, prevTree) + c.Assert(err, IsNil) + for _, change := range changes { + _, _, err := change.Files() + if err != nil { + panic(err) + c.Assert(err, IsNil) + } + } + + prev = commit + } +} + func (s *ChangeSuite) TestErrorsFindingChildsAreDetected(c *C) { // Commit 7beaad711378a4daafccc2c04bc46d36df2a0fd1 of the go-git // fixture modified "examples/latest/latest.go". -- cgit