Improve listing performance by using go-git (#6478)

* Use go-git for tree reading and commit info lookup.

Signed-off-by: Filip Navara <[email protected]>

* Use TreeEntry.IsRegular() instead of ObjectType that was removed.

Signed-off-by: Filip Navara <[email protected]>

* Use the treePath to optimize commit info search.

Signed-off-by: Filip Navara <[email protected]>

* Extract the latest commit at treePath along with the other commits.

Signed-off-by: Filip Navara <[email protected]>

* Fix listing commit info for a directory that was created in one commit and never modified after.

Signed-off-by: Filip Navara <[email protected]>

* Avoid nearly all external 'git' invocations when doing directory listing (.editorconfig code path is still hit).

Signed-off-by: Filip Navara <[email protected]>

* Use go-git for reading blobs.

Signed-off-by: Filip Navara <[email protected]>

* Make SHA1 type alias for plumbing.Hash in go-git.

Signed-off-by: Filip Navara <[email protected]>

* Make Signature type alias for object.Signature in go-git.

Signed-off-by: Filip Navara <[email protected]>

* Fix GetCommitsInfo for repository with only one commit.

Signed-off-by: Filip Navara <[email protected]>

* Fix PGP signature verification.

Signed-off-by: Filip Navara <[email protected]>

* Fix issues with walking commit graph across merges.

Signed-off-by: Filip Navara <[email protected]>

* Fix typo in condition.

Signed-off-by: Filip Navara <[email protected]>

* Speed up loading branch list by keeping the repository reference (and thus all the loaded packfile indexes).

Signed-off-by: Filip Navara <[email protected]>

* Fix lising submodules.

Signed-off-by: Filip Navara <[email protected]>

* Fix build

Signed-off-by: Filip Navara <[email protected]>

* Add back commit cache because of name-rev

Signed-off-by: Filip Navara <[email protected]>

* Fix tests

Signed-off-by: Filip Navara <[email protected]>

* Fix code style

* Fix spelling

* Address PR feedback

Signed-off-by: Filip Navara <[email protected]>

* Update vendor module list

Signed-off-by: Filip Navara <[email protected]>

* Fix getting trees by commit id

Signed-off-by: Filip Navara <[email protected]>

* Fix remaining unit test failures

* Fix GetTreeBySHA

* Avoid running `git name-rev` if not necessary

Signed-off-by: Filip Navara <[email protected]>

* Move Branch code to git module

* Clean up GPG signature verification and fix it for tagged commits

* Address PR feedback (import formatting, copyright headers)

* Make blob lookup by SHA working

* Update tests to use public API

* Allow getting content from any type of object through the blob interface

* Change test to actually expect the object content that is in the GIT repository

* Change one more test to actually expect the object content that is in the GIT repository

* Add comments

Author: filipnavara

go.mod

@@ -32,7 +32,7 @@ require (
 	github.com/dgrijalva/jwt-go v0.0.0-20161101193935-9ed569b5d1ac
 	github.com/edsrzf/mmap-go v0.0.0-20170320065105-0bce6a688712 // indirect
 	github.com/elazarl/go-bindata-assetfs v0.0.0-20151224045452-57eb5e1fc594 // indirect
-	github.com/emirpasic/gods v1.12.0 // indirect
+	github.com/emirpasic/gods v1.12.0
 	github.com/etcd-io/bbolt v1.3.2 // indirect
 	github.com/ethantkoenig/rupture v0.0.0-20180203182544-0a76f03a811a
 	github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a // indirect
@@ -127,7 +127,7 @@ require (
 	gopkg.in/ldap.v3 v3.0.2
 	gopkg.in/macaron.v1 v1.3.2
 	gopkg.in/redis.v2 v2.3.2 // indirect
-	gopkg.in/src-d/go-billy.v4 v4.3.0 // indirect
+	gopkg.in/src-d/go-billy.v4 v4.3.0
 	gopkg.in/src-d/go-git.v4 v4.10.0
 	gopkg.in/testfixtures.v2 v2.5.0
 	mvdan.cc/xurls/v2 v2.0.0

integrations/api_repo_git_blobs_test.go

@@ -38,7 +38,7 @@ func TestAPIReposGitBlobs(t *testing.T) {
 	var gitBlobResponse api.GitBlobResponse
 	DecodeJSON(t, resp, &gitBlobResponse)
 	assert.NotNil(t, gitBlobResponse)
-	expectedContent := "Y29tbWl0IDY1ZjFiZjI3YmMzYmY3MGY2NDY1NzY1ODYzNWU2NjA5NGVkYmNiNGQKQXV0aG9yOiB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+CkRhdGU6ICAgU3VuIE1hciAxOSAxNjo0Nzo1OSAyMDE3IC0wNDAwCgogICAgSW5pdGlhbCBjb21taXQKCmRpZmYgLS1naXQgYS9SRUFETUUubWQgYi9SRUFETUUubWQKbmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMC4uNGI0ODUxYQotLS0gL2Rldi9udWxsCisrKyBiL1JFQURNRS5tZApAQCAtMCwwICsxLDMgQEAKKyMgcmVwbzEKKworRGVzY3JpcHRpb24gZm9yIHJlcG8xClwgTm8gbmV3bGluZSBhdCBlbmQgb2YgZmlsZQo="
+	expectedContent := "dHJlZSAyYTJmMWQ0NjcwNzI4YTJlMTAwNDllMzQ1YmQ3YTI3NjQ2OGJlYWI2CmF1dGhvciB1c2VyMSA8YWRkcmVzczFAZXhhbXBsZS5jb20+IDE0ODk5NTY0NzkgLTA0MDAKY29tbWl0dGVyIEV0aGFuIEtvZW5pZyA8ZXRoYW50a29lbmlnQGdtYWlsLmNvbT4gMTQ4OTk1NjQ3OSAtMDQwMAoKSW5pdGlhbCBjb21taXQK"
 	assert.Equal(t, expectedContent, gitBlobResponse.Content)
 
 	// Tests a private repo with no token so will fail

models/error.go

@@ -874,21 +874,6 @@ func (err ErrUserDoesNotHaveAccessToRepo) Error() string {
 //  |______  / |__|  (____  /___|  /\___  >___|  /
 //         \/             \/     \/     \/     \/
 
-// ErrBranchNotExist represents a "BranchNotExist" kind of error.
-type ErrBranchNotExist struct {
-	Name string
-}
-
-// IsErrBranchNotExist checks if an error is a ErrBranchNotExist.
-func IsErrBranchNotExist(err error) bool {
-	_, ok := err.(ErrBranchNotExist)
-	return ok
-}
-
-func (err ErrBranchNotExist) Error() string {
-	return fmt.Sprintf("branch does not exist [name: %s]", err.Name)
-}
-
 // ErrBranchAlreadyExists represents an error that branch with such name already exists.
 type ErrBranchAlreadyExists struct {
 	BranchName string

models/pull.go

@@ -1,4 +1,5 @@
 // Copyright 2015 The Gogs Authors. All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
@@ -165,8 +166,8 @@ func (pr *PullRequest) APIFormat() *api.PullRequest {
 
 func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
 	var (
-		baseBranch *Branch
-		headBranch *Branch
+		baseBranch *git.Branch
+		headBranch *git.Branch
 		baseCommit *git.Commit
 		headCommit *git.Commit
 		err        error

models/repo_branch.go

@@ -1,4 +1,5 @@
 // Copyright 2016 The Gogs Authors. All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
@@ -86,53 +87,24 @@ func (repo *Repository) DeleteLocalBranch(branchName string) error {
 	return deleteLocalBranch(repo.LocalCopyPath(), repo.DefaultBranch, branchName)
 }
 
-// Branch holds the branch information
-type Branch struct {
-	Path string
-	Name string
-}
-
-// GetBranchesByPath returns a branch by it's path
-func GetBranchesByPath(path string) ([]*Branch, error) {
-	gitRepo, err := git.OpenRepository(path)
-	if err != nil {
-		return nil, err
-	}
-
-	brs, err := gitRepo.GetBranches()
-	if err != nil {
-		return nil, err
-	}
-
-	branches := make([]*Branch, len(brs))
-	for i := range brs {
-		branches[i] = &Branch{
-			Path: path,
-			Name: brs[i],
-		}
-	}
-	return branches, nil
-}
-
 // CanCreateBranch returns true if repository meets the requirements for creating new branches.
 func (repo *Repository) CanCreateBranch() bool {
 	return !repo.IsMirror
 }
 
-// GetBranch returns a branch by it's name
-func (repo *Repository) GetBranch(branch string) (*Branch, error) {
-	if !git.IsBranchExist(repo.RepoPath(), branch) {
-		return nil, ErrBranchNotExist{branch}
+// GetBranch returns a branch by its name
+func (repo *Repository) GetBranch(branch string) (*git.Branch, error) {
+	gitRepo, err := git.OpenRepository(repo.RepoPath())
+	if err != nil {
+		return nil, err
 	}
-	return &Branch{
-		Path: repo.RepoPath(),
-		Name: branch,
-	}, nil
+
+	return gitRepo.GetBranch(branch)
 }
 
 // GetBranches returns all the branches of a repository
-func (repo *Repository) GetBranches() ([]*Branch, error) {
-	return GetBranchesByPath(repo.RepoPath())
+func (repo *Repository) GetBranches() ([]*git.Branch, error) {
+	return git.GetBranchesByPath(repo.RepoPath())
 }
 
 // CheckBranchName validates branch name with existing repository branches
@@ -257,12 +229,3 @@ func (repo *Repository) CreateNewBranchFromCommit(doer *User, commit, branchName
 
 	return nil
 }
-
-// GetCommit returns all the commits of a branch
-func (branch *Branch) GetCommit() (*git.Commit, error) {
-	gitRepo, err := git.OpenRepository(branch.Path)
-	if err != nil {
-		return nil, err
-	}
-	return gitRepo.GetBranchCommit(branch.Name)
-}

modules/context/repo.go

@@ -157,10 +157,11 @@ func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) {
 	if treeEntry.Blob().Size() >= setting.UI.MaxDisplayFileSize {
 		return nil, git.ErrNotExist{ID: "", RelPath: ".editorconfig"}
 	}
-	reader, err := treeEntry.Blob().Data()
+	reader, err := treeEntry.Blob().DataAsync()
 	if err != nil {
 		return nil, err
 	}
+	defer reader.Close()
 	data, err := ioutil.ReadAll(reader)
 	if err != nil {
 		return nil, err

modules/git/blob.go

@@ -1,76 +1,40 @@
 // Copyright 2015 The Gogs Authors. All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
 package git
 
 import (
-	"bytes"
 	"encoding/base64"
-	"fmt"
 	"io"
 	"io/ioutil"
-	"os"
-	"os/exec"
+
+	"gopkg.in/src-d/go-git.v4/plumbing"
 )
 
 // Blob represents a Git object.
 type Blob struct {
-	repo *Repository
-	*TreeEntry
-}
-
-// Data gets content of blob all at once and wrap it as io.Reader.
-// This can be very slow and memory consuming for huge content.
-func (b *Blob) Data() (io.Reader, error) {
-	stdout := new(bytes.Buffer)
-	stderr := new(bytes.Buffer)
-
-	// Preallocate memory to save ~50% memory usage on big files.
-	stdout.Grow(int(b.Size() + 2048))
-
-	if err := b.DataPipeline(stdout, stderr); err != nil {
-		return nil, concatenateError(err, stderr.String())
-	}
-	return stdout, nil
-}
+	ID SHA1
 
-// DataPipeline gets content of blob and write the result or error to stdout or stderr
-func (b *Blob) DataPipeline(stdout, stderr io.Writer) error {
-	return NewCommand("show", b.ID.String()).RunInDirPipeline(b.repo.Path, stdout, stderr)
-}
-
-type cmdReadCloser struct {
-	cmd    *exec.Cmd
-	stdout io.Reader
-}
-
-func (c cmdReadCloser) Read(p []byte) (int, error) {
-	return c.stdout.Read(p)
-}
-
-func (c cmdReadCloser) Close() error {
-	io.Copy(ioutil.Discard, c.stdout)
-	return c.cmd.Wait()
+	gogitEncodedObj plumbing.EncodedObject
+	name            string
 }
 
 // DataAsync gets a ReadCloser for the contents of a blob without reading it all.
 // Calling the Close function on the result will discard all unread output.
 func (b *Blob) DataAsync() (io.ReadCloser, error) {
-	cmd := exec.Command("git", "show", b.ID.String())
-	cmd.Dir = b.repo.Path
-	cmd.Stderr = os.Stderr
-
-	stdout, err := cmd.StdoutPipe()
-	if err != nil {
-		return nil, fmt.Errorf("StdoutPipe: %v", err)
-	}
+	return b.gogitEncodedObj.Reader()
+}
 
-	if err = cmd.Start(); err != nil {
-		return nil, fmt.Errorf("Start: %v", err)
-	}
+// Size returns the uncompressed size of the blob
+func (b *Blob) Size() int64 {
+	return b.gogitEncodedObj.Size()
+}
 
-	return cmdReadCloser{stdout: stdout, cmd: cmd}, nil
+// Name returns name of the tree entry this blob object was created from (or empty string)
+func (b *Blob) Name() string {
+	return b.name
 }
 
 // GetBlobContentBase64 Reads the content of the blob with a base64 encode and returns the encoded string

modules/git/blob_test.go

@@ -1,32 +1,18 @@
 // Copyright 2015 The Gogs Authors. All rights reserved.
+// Copyright 2019 The Gitea Authors. All rights reserved.
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
 package git
 
 import (
-	"bytes"
 	"io/ioutil"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-var repoSelf = &Repository{
-	Path: "./",
-}
-
-var testBlob = &Blob{
-	repo: repoSelf,
-	TreeEntry: &TreeEntry{
-		ID: MustIDFromString("a8d4b49dd073a4a38a7e58385eeff7cc52568697"),
-		ptree: &Tree{
-			repo: repoSelf,
-		},
-	},
-}
-
 func TestBlob_Data(t *testing.T) {
 	output := `Copyright (c) 2016 The Gitea Authors
 Copyright (c) 2015 The Gogs Authors
@@ -49,32 +35,37 @@ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 THE SOFTWARE.
 `
+	repo, err := OpenRepository("../../.git")
+	assert.NoError(t, err)
+	testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697")
+	assert.NoError(t, err)
 
-	r, err := testBlob.Data()
+	r, err := testBlob.DataAsync()
 	assert.NoError(t, err)
 	require.NotNil(t, r)
+	defer r.Close()
 
 	data, err := ioutil.ReadAll(r)
 	assert.NoError(t, err)
 	assert.Equal(t, output, string(data))
 }
 
 func Benchmark_Blob_Data(b *testing.B) {
-	for i := 0; i < b.N; i++ {
-		r, err := testBlob.Data()
-		if err != nil {
-			b.Fatal(err)
-		}
-		ioutil.ReadAll(r)
+	repo, err := OpenRepository("../../.git")
+	if err != nil {
+		b.Fatal(err)
+	}
+	testBlob, err := repo.GetBlob("a8d4b49dd073a4a38a7e58385eeff7cc52568697")
+	if err != nil {
+		b.Fatal(err)
 	}
-}
 
-func Benchmark_Blob_DataPipeline(b *testing.B) {
-	stdout := new(bytes.Buffer)
 	for i := 0; i < b.N; i++ {
-		stdout.Reset()
-		if err := testBlob.DataPipeline(stdout, nil); err != nil {
+		r, err := testBlob.DataAsync()
+		if err != nil {
 			b.Fatal(err)
 		}
+		defer r.Close()
+		ioutil.ReadAll(r)
 	}
 }