Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use commit graph files for listing pages #7314

Merged
merged 10 commits into from
Jul 2, 2019
38 changes: 27 additions & 11 deletions modules/git/commit_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/emirpasic/gods/trees/binaryheap"
"gopkg.in/src-d/go-git.v4/plumbing"
"gopkg.in/src-d/go-git.v4/plumbing/object"
cgobject "gopkg.in/src-d/go-git.v4/plumbing/object/commitgraph"
)

// GetCommitsInfo gets information of all commits that are corresponding to these entries
Expand All @@ -19,7 +20,12 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache LastCom
entryPaths[i+1] = entry.Name()
}

c, err := commit.repo.gogitRepo.CommitObject(plumbing.Hash(commit.ID))
commitNodeIndex, commitGraphFile := commit.repo.CommitNodeIndex()
if commitGraphFile != nil {
defer commitGraphFile.Close()
}

c, err := commitNodeIndex.Get(plumbing.Hash(commit.ID))
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -69,14 +75,14 @@ func (tes Entries) GetCommitsInfo(commit *Commit, treePath string, cache LastCom
}

type commitAndPaths struct {
commit *object.Commit
commit cgobject.CommitNode
// Paths that are still on the branch represented by commit
paths []string
// Set of hashes for the paths
hashes map[string]plumbing.Hash
}

func getCommitTree(c *object.Commit, treePath string) (*object.Tree, error) {
func getCommitTree(c cgobject.CommitNode, treePath string) (*object.Tree, error) {
tree, err := c.Tree()
if err != nil {
return nil, err
Expand All @@ -93,7 +99,7 @@ func getCommitTree(c *object.Commit, treePath string) (*object.Tree, error) {
return tree, nil
}

func getFileHashes(c *object.Commit, treePath string, paths []string) (map[string]plumbing.Hash, error) {
func getFileHashes(c cgobject.CommitNode, treePath string, paths []string) (map[string]plumbing.Hash, error) {
tree, err := getCommitTree(c, treePath)
if err == object.ErrDirectoryNotFound {
// The whole tree didn't exist, so return empty map
Expand All @@ -118,16 +124,16 @@ func getFileHashes(c *object.Commit, treePath string, paths []string) (map[strin
return hashes, nil
}

func getLastCommitForPaths(c *object.Commit, treePath string, paths []string) (map[string]*object.Commit, error) {
func getLastCommitForPaths(c cgobject.CommitNode, treePath string, paths []string) (map[string]*object.Commit, error) {
// We do a tree traversal with nodes sorted by commit time
heap := binaryheap.NewWith(func(a, b interface{}) int {
if a.(*commitAndPaths).commit.Committer.When.Before(b.(*commitAndPaths).commit.Committer.When) {
if a.(*commitAndPaths).commit.CommitTime().Before(b.(*commitAndPaths).commit.CommitTime()) {
return 1
}
return -1
})

result := make(map[string]*object.Commit)
resultNodes := make(map[string]cgobject.CommitNode)
initialHashes, err := getFileHashes(c, treePath, paths)
if err != nil {
return nil, err
Expand All @@ -145,9 +151,9 @@ func getLastCommitForPaths(c *object.Commit, treePath string, paths []string) (m

// Load the parent commits for the one we are currently examining
numParents := current.commit.NumParents()
var parents []*object.Commit
var parents []cgobject.CommitNode
for i := 0; i < numParents; i++ {
parent, err := current.commit.Parent(i)
parent, err := current.commit.ParentNode(i)
if err != nil {
break
}
Expand All @@ -174,7 +180,7 @@ func getLastCommitForPaths(c *object.Commit, treePath string, paths []string) (m
for i, path := range current.paths {
// The results could already contain some newer change for the same path,
// so don't override that and bail out on the file early.
if result[path] == nil {
if resultNodes[path] == nil {
if pathUnchanged[i] {
// The path existed with the same hash in at least one parent so it could
// not have been changed in this commit directly.
Expand All @@ -188,7 +194,7 @@ func getLastCommitForPaths(c *object.Commit, treePath string, paths []string) (m
// - We are looking at a merge commit and the hash of the file doesn't
// match any of the hashes being merged. This is more common for directories,
// but it can also happen if a file is changed through conflict resolution.
result[path] = current.commit
resultNodes[path] = current.commit
}
}
}
Expand Down Expand Up @@ -222,5 +228,15 @@ func getLastCommitForPaths(c *object.Commit, treePath string, paths []string) (m
}
}

// Post-processing
result := make(map[string]*object.Commit)
for path, commitNode := range resultNodes {
var err error
result[path], err = commitNode.Commit()
if err != nil {
return nil, err
}
}

return result, nil
}
12 changes: 11 additions & 1 deletion modules/git/notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,17 @@ func GetNote(repo *Repository, commitID string, note *Note) error {
return err
}

lastCommits, err := getLastCommitForPaths(commit, "", []string{commitID})
commitNodeIndex, commitGraphFile := repo.CommitNodeIndex()
if commitGraphFile != nil {
defer commitGraphFile.Close()
}

commitNode, err := commitNodeIndex.Get(commit.Hash)
if err != nil {
return nil
}

lastCommits, err := getLastCommitForPaths(commitNode, "", []string{commitID})
if err != nil {
return err
}
Expand Down
24 changes: 24 additions & 0 deletions modules/git/repo_commitgraph.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package git
zeripath marked this conversation as resolved.
Show resolved Hide resolved

import (
"os"
"path"

"gopkg.in/src-d/go-git.v4/plumbing/format/commitgraph"
cgobject "gopkg.in/src-d/go-git.v4/plumbing/object/commitgraph"
)

// CommitNodeIndex returns the index for walking commit graph
func (r *Repository) CommitNodeIndex() (cgobject.CommitNodeIndex, *os.File) {
indexPath := path.Join(r.Path, "objects", "info", "commit-graph")

file, err := os.Open(indexPath)
if err == nil {
index, err := commitgraph.OpenFileIndex(file)
if err == nil {
return cgobject.NewGraphCommitNodeIndex(index, r.gogitRepo.Storer), file
}
}

Copy link
Member

Choose a reason for hiding this comment

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

If err is not file not exist error, we should return it but not hide it.

Copy link
Contributor Author

@filipnavara filipnavara Jun 29, 2019

Choose a reason for hiding this comment

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

I don't think we should return it. This is quite graceful error handling with fallback to the regular git object storage. Worst case is a performance penalty but it will still yield the same results.

Copy link
Member

@lunny lunny Jun 30, 2019

Choose a reason for hiding this comment

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

OK. But at least, we should know commit graph is not using since something wrong. We need an error log or warning log if commitgraph file exist but load failed. That will be helpful when we investigate errors.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lunny is my change to lines 30-33 acceptable?

return cgobject.NewObjectCommitNodeIndex(r.gogitRepo.Storer), nil
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

103 changes: 103 additions & 0 deletions vendor/gopkg.in/src-d/go-git.v4/plumbing/format/commitgraph/doc.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading