From be8d19438ada078a8598e366ab74aa09e4c521cd Mon Sep 17 00:00:00 2001 From: Antonio Jesus Navarro Perez Date: Mon, 10 Apr 2017 16:48:40 +0200 Subject: Add Repository.Log() method (fix #298) - CommitIter is now an interface - The old CommitIter implementation is now called StorerCommitIter - CommitWalker and CommitWalkerPost are now iterators (CommitPreIterator and CommitPostIterator). - Remove Commit.History() method. There are so many ways to iterate a commit history, depending of the use case. Now, instead of use the History() method, you must use CommitPreIterator or CommitPostIterator. - Move commitSorterer to references.go because is the only place that it is used, and it must not be used into another place. - Make References method private, it must only be used into blame logic. - Added a TODO into references method, where the sortCommits is used to remove it in a near future. --- plumbing/object/commit.go | 65 ++++--------- plumbing/object/commit_test.go | 10 +- plumbing/object/commit_walker.go | 169 +++++++++++++++++++++++----------- plumbing/object/commit_walker_test.go | 10 +- plumbing/revlist/revlist.go | 23 ++--- 5 files changed, 150 insertions(+), 127 deletions(-) (limited to 'plumbing') diff --git a/plumbing/object/commit.go b/plumbing/object/commit.go index 7507bc7..ffbb9f9 100644 --- a/plumbing/object/commit.go +++ b/plumbing/object/commit.go @@ -5,7 +5,6 @@ import ( "bytes" "fmt" "io" - "sort" "strings" "gopkg.in/src-d/go-git.v4/plumbing" @@ -64,7 +63,7 @@ func (c *Commit) Tree() (*Tree, error) { } // Parents return a CommitIter to the parent Commits. -func (c *Commit) Parents() *CommitIter { +func (c *Commit) Parents() CommitIter { return NewCommitIter(c.s, storer.NewEncodedObjectLookupIter(c.s, plumbing.CommitObject, c.parents), ) @@ -163,19 +162,6 @@ func (c *Commit) Decode(o plumbing.EncodedObject) (err error) { } } -// History returns a slice with the previous commits in the history of this -// commit, sorted in reverse chronological order. -func (c *Commit) History() ([]*Commit, error) { - var commits []*Commit - err := WalkCommitHistory(c, func(commit *Commit) error { - commits = append(commits, commit) - return nil - }) - - ReverseSortCommits(commits) - return commits, err -} - // Encode transforms a Commit into a plumbing.EncodedObject. func (b *Commit) Encode(o plumbing.EncodedObject) error { o.SetType(plumbing.CommitObject) @@ -231,24 +217,31 @@ func indent(t string) string { return strings.Join(output, "\n") } -// CommitIter provides an iterator for a set of commits. -type CommitIter struct { +// CommitIter is a generic closable interface for iterating over commits. +type CommitIter interface { + Next() (*Commit, error) + ForEach(func(*Commit) error) error + Close() +} + +// storerCommitIter provides an iterator from commits in an EncodedObjectStorer. +type storerCommitIter struct { storer.EncodedObjectIter s storer.EncodedObjectStorer } // NewCommitIter takes a storer.EncodedObjectStorer and a -// storer.EncodedObjectIter and returns a *CommitIter that iterates over all +// storer.EncodedObjectIter and returns a CommitIter that iterates over all // commits contained in the storer.EncodedObjectIter. // // Any non-commit object returned by the storer.EncodedObjectIter is skipped. -func NewCommitIter(s storer.EncodedObjectStorer, iter storer.EncodedObjectIter) *CommitIter { - return &CommitIter{iter, s} +func NewCommitIter(s storer.EncodedObjectStorer, iter storer.EncodedObjectIter) CommitIter { + return &storerCommitIter{iter, s} } // Next moves the iterator to the next commit and returns a pointer to it. If // there are no more commits, it returns io.EOF. -func (iter *CommitIter) Next() (*Commit, error) { +func (iter *storerCommitIter) Next() (*Commit, error) { obj, err := iter.EncodedObjectIter.Next() if err != nil { return nil, err @@ -260,7 +253,7 @@ func (iter *CommitIter) Next() (*Commit, error) { // ForEach call the cb function for each commit contained on this iter until // an error appends or the end of the iter is reached. If ErrStop is sent // the iteration is stop but no error is returned. The iterator is closed. -func (iter *CommitIter) ForEach(cb func(*Commit) error) error { +func (iter *storerCommitIter) ForEach(cb func(*Commit) error) error { return iter.EncodedObjectIter.ForEach(func(obj plumbing.EncodedObject) error { c, err := DecodeCommit(iter.s, obj) if err != nil { @@ -271,30 +264,6 @@ func (iter *CommitIter) ForEach(cb func(*Commit) error) error { }) } -type commitSorterer struct { - l []*Commit -} - -func (s commitSorterer) Len() int { - return len(s.l) -} - -func (s commitSorterer) Less(i, j int) bool { - return s.l[i].Committer.When.Before(s.l[j].Committer.When) -} - -func (s commitSorterer) Swap(i, j int) { - s.l[i], s.l[j] = s.l[j], s.l[i] -} - -// SortCommits sorts a commit list by commit date, from older to newer. -func SortCommits(l []*Commit) { - s := &commitSorterer{l} - sort.Sort(s) -} - -// ReverseSortCommits sorts a commit list by commit date, from newer to older. -func ReverseSortCommits(l []*Commit) { - s := &commitSorterer{l} - sort.Sort(sort.Reverse(s)) +func (iter *storerCommitIter) Close() { + iter.EncodedObjectIter.Close() } diff --git a/plumbing/object/commit_test.go b/plumbing/object/commit_test.go index 8b4ee2a..c1f49db 100644 --- a/plumbing/object/commit_test.go +++ b/plumbing/object/commit_test.go @@ -62,6 +62,8 @@ func (s *SuiteCommit) TestParents(c *C) { c.Assert(err, IsNil) c.Assert(output, DeepEquals, expected) + + i.Close() } func (s *SuiteCommit) TestCommitEncodeDecodeIdempotent(c *C) { @@ -110,14 +112,6 @@ func (s *SuiteCommit) TestNumParents(c *C) { c.Assert(s.Commit.NumParents(), Equals, 2) } -func (s *SuiteCommit) TestHistory(c *C) { - commits, err := s.Commit.History() - c.Assert(err, IsNil) - c.Assert(commits, HasLen, 5) - c.Assert(commits[0].Hash.String(), Equals, s.Commit.Hash.String()) - c.Assert(commits[len(commits)-1].Hash.String(), Equals, "b029517f6300c2da0f4b651b8642506cd6aaf45d") -} - func (s *SuiteCommit) TestString(c *C) { c.Assert(s.Commit.String(), Equals, ""+ "commit 1669dce138d9b841a518c64b10914d88f5e488ea\n"+ diff --git a/plumbing/object/commit_walker.go b/plumbing/object/commit_walker.go index 681ea5e..3316fab 100644 --- a/plumbing/object/commit_walker.go +++ b/plumbing/object/commit_walker.go @@ -4,93 +4,150 @@ import ( "io" "gopkg.in/src-d/go-git.v4/plumbing" + "gopkg.in/src-d/go-git.v4/plumbing/storer" ) -type commitWalker struct { +type commitPreIterator struct { seen map[plumbing.Hash]bool - stack []*CommitIter + stack []CommitIter start *Commit - cb func(*Commit) error } -// WalkCommitHistory walks the commit history, starting at the given commit and -// visiting its parents in pre-order. The given callback will be called for each -// visited commit. Each commit will be visited only once. If the callback returns -// an error, walking will stop and will return the error. Other errors might be -// returned if the history cannot be traversed (e.g. missing objects). -func WalkCommitHistory(c *Commit, cb func(*Commit) error) error { - w := &commitWalker{ +// NewCommitPreIterator returns a CommitIter that walks the commit history, +// starting at the given commit and visiting its parents in pre-order. +// The given callback will be called for each visited commit. Each commit will +// be visited only once. If the callback returns an error, walking will stop +// and will return the error. Other errors might be returned if the history +// cannot be traversed (e.g. missing objects). +func NewCommitPreIterator(c *Commit) CommitIter { + return &commitPreIterator{ seen: make(map[plumbing.Hash]bool), - stack: make([]*CommitIter, 0), + stack: make([]CommitIter, 0), start: c, - cb: cb, } - - return w.walk() } -func (w *commitWalker) walk() error { - var commit *Commit +func (w *commitPreIterator) Next() (*Commit, error) { + var c *Commit + for { + if w.start != nil { + c = w.start + w.start = nil + } else { + current := len(w.stack) - 1 + if current < 0 { + return nil, io.EOF + } - if w.start != nil { - commit = w.start - w.start = nil - } else { - current := len(w.stack) - 1 - if current < 0 { - return nil + var err error + c, err = w.stack[current].Next() + if err == io.EOF { + w.stack = w.stack[:current] + continue + } + + if err != nil { + return nil, err + } + } + + // check and update seen + if w.seen[c.Hash] { + continue } - var err error - commit, err = w.stack[current].Next() + w.seen[c.Hash] = true + if c.NumParents() > 0 { + w.stack = append(w.stack, c.Parents()) + } + + return c, nil + } +} + +func (w *commitPreIterator) ForEach(cb func(*Commit) error) error { + for { + c, err := w.Next() if err == io.EOF { - w.stack = w.stack[:current] - return w.walk() + break + } + if err != nil { + return err } + err = cb(c) + if err == storer.ErrStop { + break + } if err != nil { return err } } - // check and update seen - if w.seen[commit.Hash] { - return w.walk() - } + return nil +} - w.seen[commit.Hash] = true - if commit.NumParents() > 0 { - w.stack = append(w.stack, commit.Parents()) - } +func (w *commitPreIterator) Close() {} - if err := w.cb(commit); err != nil { - return err - } +type commitPostIterator struct { + stack []*Commit + seen map[plumbing.Hash]bool +} - return w.walk() +// NewCommitPostIterator returns a CommitIter that walks the commit +// history like WalkCommitHistory but in post-order. This means that after +// walking a merge commit, the merged commit will be walked before the base +// it was merged on. This can be useful if you wish to see the history in +// chronological order. +func NewCommitPostIterator(c *Commit) CommitIter { + return &commitPostIterator{ + stack: []*Commit{c}, + seen: make(map[plumbing.Hash]bool), + } } -// WalkCommitHistoryPost walks the commit history like WalkCommitHistory -// but in post-order. This means that after walking a merge commit, the -// merged commit will be walked before the base it was merged on. This -// can be useful if you wish to see the history in chronological order. -func WalkCommitHistoryPost(c *Commit, cb func(*Commit) error) error { - stack := []*Commit{c} - seen := make(map[plumbing.Hash]bool) - for len(stack) > 0 { - c := stack[len(stack)-1] - stack = stack[:len(stack)-1] - if seen[c.Hash] { - continue +func (w *commitPostIterator) Next() (*Commit, error) { + for { + if len(w.stack) == 0 { + return nil, io.EOF } - seen[c.Hash] = true - if err := cb(c); err != nil { - return err + + c := w.stack[len(w.stack)-1] + w.stack = w.stack[:len(w.stack)-1] + if w.seen[c.Hash] { + continue } - c.Parents().ForEach(func(pcm *Commit) error { - stack = append(stack, pcm) + w.seen[c.Hash] = true + + err := c.Parents().ForEach(func(pcm *Commit) error { + w.stack = append(w.stack, pcm) return nil }) + + return c, err + } +} + +func (w *commitPostIterator) ForEach(cb func(*Commit) error) error { + for { + c, err := w.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + + err = cb(c) + if err == storer.ErrStop { + break + } + if err != nil { + return err + } } + return nil } + +func (w *commitPostIterator) Close() {} diff --git a/plumbing/object/commit_walker_test.go b/plumbing/object/commit_walker_test.go index aa54de0..7d04bce 100644 --- a/plumbing/object/commit_walker_test.go +++ b/plumbing/object/commit_walker_test.go @@ -8,11 +8,12 @@ type CommitWalkerSuite struct { var _ = Suite(&CommitWalkerSuite{}) -func (s *CommitWalkerSuite) TestWalkHistory(c *C) { +func (s *CommitWalkerSuite) TestCommitPreIterator(c *C) { commit := s.commit(c, s.Fixture.Head) var commits []*Commit - WalkCommitHistory(commit, func(c *Commit) error { + wIter := NewCommitPreIterator(commit) + wIter.ForEach(func(c *Commit) error { commits = append(commits, c) return nil }) @@ -34,11 +35,12 @@ func (s *CommitWalkerSuite) TestWalkHistory(c *C) { } } -func (s *CommitWalkerSuite) TestWalkHistoryPost(c *C) { +func (s *CommitWalkerSuite) TestCommitPostIterator(c *C) { commit := s.commit(c, s.Fixture.Head) var commits []*Commit - WalkCommitHistoryPost(commit, func(c *Commit) error { + wIter := NewCommitPostIterator(commit) + wIter.ForEach(func(c *Commit) error { commits = append(commits, c) return nil }) diff --git a/plumbing/revlist/revlist.go b/plumbing/revlist/revlist.go index eb9afaf..fbd1bd9 100644 --- a/plumbing/revlist/revlist.go +++ b/plumbing/revlist/revlist.go @@ -82,20 +82,21 @@ func reachableObjects( commit *object.Commit, seen map[plumbing.Hash]bool, cb func(h plumbing.Hash)) error { - return object.WalkCommitHistory(commit, func(commit *object.Commit) error { - if seen[commit.Hash] { - return nil - } + return object.NewCommitPreIterator(commit). + ForEach(func(commit *object.Commit) error { + if seen[commit.Hash] { + return nil + } - cb(commit.Hash) + cb(commit.Hash) - tree, err := commit.Tree() - if err != nil { - return err - } + tree, err := commit.Tree() + if err != nil { + return err + } - return iterateCommitTrees(seen, tree, cb) - }) + return iterateCommitTrees(seen, tree, cb) + }) } // iterateCommitTrees iterate all reachable trees from the given commit -- cgit