Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

Implement git log --all #1045

Merged
merged 3 commits into from
Jan 11, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,11 @@ type LogOptions struct {
// Show only those commits in which the specified file was inserted/updated.
// It is equivalent to running `git log -- <file-name>`.
FileName *string

// Pretend as if all the refs in refs/, along with HEAD, are listed on the command line as <commit>.
// It is equivalent to running `git log --all`.
// If set on true, the From option will be ignored.
All bool
}

var (
Expand Down
126 changes: 126 additions & 0 deletions plumbing/object/commit_walker.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package object

import (
"container/list"
"io"

"gopkg.in/src-d/go-git.v4/plumbing"
"gopkg.in/src-d/go-git.v4/plumbing/storer"
"gopkg.in/src-d/go-git.v4/storage"
)

type commitPreIterator struct {
Expand Down Expand Up @@ -181,3 +183,127 @@ func (w *commitPostIterator) ForEach(cb func(*Commit) error) error {
}

func (w *commitPostIterator) Close() {}

// commitAllIterator stands for commit iterator for all refs.
type commitAllIterator struct {
// el points to the current commit.
el *list.Element
}

// NewCommitAllIter returns a new commit iterator for all refs.
// s is a repo Storer used to get commits and references.
// fn is a commit iterator function, used to iterate through ref commits in chosen order
func NewCommitAllIter(s storage.Storer, fn func(*Commit) CommitIter) (CommitIter, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar to revlist.Objects but for commits.
This is just a note that, in the future, we might want to deprecate revlist.Objects in favor of NewObjectAllIter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth to do it now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, but it should go in another PR (IMO)

l := list.New()
m := make(map[plumbing.Hash]*list.Element)

// ...along with the HEAD
head, err := storer.ResolveReference(s, plumbing.HEAD)
if err != nil {
return nil, err
}
headCommit, err := GetCommit(s, head.Hash())
if err != nil {
return nil, err
}
err = fn(headCommit).ForEach(func(c *Commit) error {
el := l.PushBack(c)
m[c.Hash] = el
return nil
})
if err != nil {
return nil, err
}

refIter, err := s.IterReferences()
if err != nil {
return nil, err
}
defer refIter.Close()
err = refIter.ForEach(func(r *plumbing.Reference) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be great if we can split this function in something more readable

if r.Hash() == head.Hash() {
// we already have the HEAD
return nil
}

el, ok := m[r.Hash()]
if ok {
return nil
}

var refCommits []*Commit
c, _ := GetCommit(s, r.Hash())
// if it's not a commit - skip it.
if c == nil {
return nil
}
cit := fn(c)
for c, e := cit.Next(); e == nil; {
el, ok = m[c.Hash]
if ok {
break
}
refCommits = append(refCommits, c)
c, e = cit.Next()
}
cit.Close()

if el == nil {
// push back all commits from this ref.
for _, c := range refCommits {
el = l.PushBack(c)
m[c.Hash] = el
}
} else {
// insert ref's commits into the list
for i := len(refCommits) - 1; i >= 0; i-- {
c := refCommits[i]
el = l.InsertBefore(c, el)
m[c.Hash] = el
}
}
return nil
})
if err != nil {
return nil, err
}

return &commitAllIterator{l.Front()}, nil
}

func (it *commitAllIterator) Next() (*Commit, error) {
if it.el == nil {
return nil, io.EOF
}

c := it.el.Value.(*Commit)
it.el = it.el.Next()

return c, nil
}

func (it *commitAllIterator) ForEach(cb func(*Commit) error) error {
for {
c, err := it.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 (it *commitAllIterator) Close() {
it.el = nil
}
25 changes: 22 additions & 3 deletions plumbing/object/commit_walker_file.go
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
package object

import (
"gopkg.in/src-d/go-git.v4/plumbing/storer"
"io"

"gopkg.in/src-d/go-git.v4/plumbing/storer"
)

type commitFileIter struct {
fileName string
sourceIter CommitIter
currentCommit *Commit
all bool
}

// NewCommitFileIterFromIter returns a commit iterator which performs diffTree between
// successive trees returned from the commit iterator from the argument. The purpose of this is
// to find the commits that explain how the files that match the path came to be.
func NewCommitFileIterFromIter(fileName string, commitIter CommitIter) CommitIter {
func NewCommitFileIterFromIter(fileName string, commitIter CommitIter, all bool) CommitIter {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propagating here the word all maybe is not the best option since is not very meaningful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, actually it should be something like checkParents

iterator := new(commitFileIter)
iterator.sourceIter = commitIter
iterator.fileName = fileName
iterator.all = all
return iterator
}

Expand Down Expand Up @@ -73,8 +76,24 @@ func (c *commitFileIter) getNextFileCommit() (*Commit, error) {

foundChangeForFile := false
for _, change := range changes {
if change.name() == c.fileName {
if change.name() != c.fileName {
continue
}

// filename matches, now check if source iterator contains all commits (from all refs)
if c.all {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This snippet need a refactor

// for `git log --all` also check if the next commit comes from the same parent
for _, h := range c.currentCommit.ParentHashes {
if h == parentCommit.Hash {
foundChangeForFile = true
break
}
}
} else {
foundChangeForFile = true
}

if foundChangeForFile {
break
}
}
Expand Down
71 changes: 47 additions & 24 deletions repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -1027,41 +1027,64 @@ func (r *Repository) PushContext(ctx context.Context, o *PushOptions) error {

// Log returns the commit history from the given LogOptions.
func (r *Repository) Log(o *LogOptions) (object.CommitIter, error) {
h := o.From
if o.From == plumbing.ZeroHash {
head, err := r.Head()
if err != nil {
return nil, err
}

h = head.Hash()
}

commit, err := r.CommitObject(h)
if err != nil {
return nil, err
}

var commitIter object.CommitIter
var (
err error
commitIterFunc func(*object.Commit) object.CommitIter
commitIter object.CommitIter
)
switch o.Order {
case LogOrderDefault:
commitIter = object.NewCommitPreorderIter(commit, nil, nil)
commitIterFunc = func(c *object.Commit) object.CommitIter {
return object.NewCommitPreorderIter(c, nil, nil)
}
case LogOrderDFS:
commitIter = object.NewCommitPreorderIter(commit, nil, nil)
commitIterFunc = func(c *object.Commit) object.CommitIter {
return object.NewCommitPreorderIter(c, nil, nil)
}
case LogOrderDFSPost:
commitIter = object.NewCommitPostorderIter(commit, nil)
commitIterFunc = func(c *object.Commit) object.CommitIter {
return object.NewCommitPostorderIter(c, nil)
}
case LogOrderBSF:
commitIter = object.NewCommitIterBSF(commit, nil, nil)
commitIterFunc = func(c *object.Commit) object.CommitIter {
return object.NewCommitIterBSF(c, nil, nil)
}
case LogOrderCommitterTime:
commitIter = object.NewCommitIterCTime(commit, nil, nil)
commitIterFunc = func(c *object.Commit) object.CommitIter {
return object.NewCommitIterCTime(c, nil, nil)
}
default:
return nil, fmt.Errorf("invalid Order=%v", o.Order)
}

if o.FileName == nil {
return commitIter, nil
if o.All {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this code can be move to a function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly do you suggest to extract?
switch-case? else-branch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know exactly the problem is that I think is very nested and complicated code with a high cyclomatic complexity

commitIter, err = object.NewCommitAllIter(r.Storer, commitIterFunc)
if err != nil {
return nil, err
}
} else {
h := o.From
if o.From == plumbing.ZeroHash {
head, err := r.Head()
if err != nil {
return nil, err
}

h = head.Hash()
}

commit, err := r.CommitObject(h)
if err != nil {
return nil, err
}
commitIter = commitIterFunc(commit)
}
return object.NewCommitFileIterFromIter(*o.FileName, commitIter), nil

if o.FileName != nil {
commitIter = object.NewCommitFileIterFromIter(*o.FileName, commitIter, o.All)
}

return commitIter, nil
}

// Tags returns all the tag References in a repository.
Expand Down
Loading