Monitor all git commands; move blame to git package and replace git a…

…s a variable (#6864) * monitor all git commands; move blame to git package and replace git as a variable * use git command but not other commands * fix build * move exec.Command to git.NewCommand * fix fmt * remove unrelated changes * remove unrelated changes * refactor IsEmpty and add tests * fix tests * fix tests * fix tests * fix tests * remove gitLogger * fix fmt * fix isEmpty * fix lint * fix tests
go-gitea · Jun 26, 2019 · edc94c7 · edc94c7
1 parent 161e12e
commit edc94c7
Show file tree

Hide file tree

Showing 34 changed files with 750 additions and 74 deletions.
diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go
@@ -20,20 +20,20 @@ import (
 	"strconv"
 	"time"
 
+	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/markup/external"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/routers"
 	"code.gitea.io/gitea/routers/routes"
+
 	"github.com/Unknwon/com"
 	"github.com/go-xorm/xorm"
 	context2 "github.com/gorilla/context"
 	"gopkg.in/src-d/go-git.v4"
 	"gopkg.in/src-d/go-git.v4/config"
 	"gopkg.in/src-d/go-git.v4/plumbing"
 	"gopkg.in/testfixtures.v2"
-
-	"code.gitea.io/gitea/models"
-	"code.gitea.io/gitea/modules/setting"
 )
 
 var codeFilePath = "contrib/pr/checkout.go"

diff --git a/models/git_diff.go b/models/git_diff.go
@@ -675,7 +675,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
 
 	var cmd *exec.Cmd
 	if len(beforeCommitID) == 0 && commit.ParentCount() == 0 {
-		cmd = exec.Command("git", "show", afterCommitID)
+		cmd = exec.Command(git.GitExecutable, "show", afterCommitID)
 	} else {
 		actualBeforeCommitID := beforeCommitID
 		if len(actualBeforeCommitID) == 0 {
@@ -688,7 +688,7 @@ func GetDiffRangeWithWhitespaceBehavior(repoPath, beforeCommitID, afterCommitID
 		}
 		diffArgs = append(diffArgs, actualBeforeCommitID)
 		diffArgs = append(diffArgs, afterCommitID)
-		cmd = exec.Command("git", diffArgs...)
+		cmd = exec.Command(git.GitExecutable, diffArgs...)
 	}
 	cmd.Dir = repoPath
 	cmd.Stderr = os.Stderr
@@ -752,23 +752,23 @@ func GetRawDiffForFile(repoPath, startCommit, endCommit string, diffType RawDiff
 	switch diffType {
 	case RawDiffNormal:
 		if len(startCommit) != 0 {
-			cmd = exec.Command("git", append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"diff", "-M", startCommit, endCommit}, fileArgs...)...)
 		} else if commit.ParentCount() == 0 {
-			cmd = exec.Command("git", append([]string{"show", endCommit}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"show", endCommit}, fileArgs...)...)
 		} else {
 			c, _ := commit.Parent(0)
-			cmd = exec.Command("git", append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"diff", "-M", c.ID.String(), endCommit}, fileArgs...)...)
 		}
 	case RawDiffPatch:
 		if len(startCommit) != 0 {
 			query := fmt.Sprintf("%s...%s", endCommit, startCommit)
-			cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", query}, fileArgs...)...)
 		} else if commit.ParentCount() == 0 {
-			cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", "--root", endCommit}, fileArgs...)...)
 		} else {
 			c, _ := commit.Parent(0)
 			query := fmt.Sprintf("%s...%s", endCommit, c.ID.String())
-			cmd = exec.Command("git", append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...)
+			cmd = exec.Command(git.GitExecutable, append([]string{"format-patch", "--no-signature", "--stdout", query}, fileArgs...)...)
 		}
 	default:
 		return fmt.Errorf("invalid diffType: %s", diffType)

diff --git a/models/pull.go b/models/pull.go
@@ -463,7 +463,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
 	// Check if a pull request is merged into BaseBranch
 	_, stderr, err := process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("isMerged (git merge-base --is-ancestor): %d", pr.BaseRepo.ID),
 		[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
-		"git", "merge-base", "--is-ancestor", headFile, pr.BaseBranch)
+		git.GitExecutable, "merge-base", "--is-ancestor", headFile, pr.BaseBranch)
 
 	if err != nil {
 		// Errors are signaled by a non-zero status that is not 1
@@ -486,7 +486,7 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
 	// Get the commit from BaseBranch where the pull request got merged
 	mergeCommit, stderr, err := process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("isMerged (git rev-list --ancestry-path --merges --reverse): %d", pr.BaseRepo.ID),
 		[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
-		"git", "rev-list", "--ancestry-path", "--merges", "--reverse", cmd)
+		git.GitExecutable, "rev-list", "--ancestry-path", "--merges", "--reverse", cmd)
 	if err != nil {
 		return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v %v", stderr, err)
 	} else if len(mergeCommit) < 40 {
@@ -548,7 +548,7 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
 	var stderr string
 	_, stderr, err = process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git read-tree): %d", pr.BaseRepo.ID),
 		[]string{"GIT_DIR=" + pr.BaseRepo.RepoPath(), "GIT_INDEX_FILE=" + indexTmpPath},
-		"git", "read-tree", pr.BaseBranch)
+		git.GitExecutable, "read-tree", pr.BaseBranch)
 	if err != nil {
 		return fmt.Errorf("git read-tree --index-output=%s %s: %v - %s", indexTmpPath, pr.BaseBranch, err, stderr)
 	}
@@ -568,7 +568,7 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
 
 	_, stderr, err = process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git apply --check): %d", pr.BaseRepo.ID),
 		[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
-		"git", args...)
+		git.GitExecutable, args...)
 	if err != nil {
 		for i := range patchConflicts {
 			if strings.Contains(stderr, patchConflicts[i]) {

diff --git a/models/release.go b/models/release.go
@@ -434,7 +434,7 @@ func DeleteReleaseByID(id int64, u *User, delTag bool) error {
 	if delTag {
 		_, stderr, err := process.GetManager().ExecDir(-1, repo.RepoPath(),
 			fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID),
-			"git", "tag", "-d", rel.TagName)
+			git.GitExecutable, "tag", "-d", rel.TagName)
 		if err != nil && !strings.Contains(stderr, "not found") {
 			return fmt.Errorf("git tag -d: %v - %s", err, stderr)
 		}

diff --git a/models/repo.go b/models/repo.go
@@ -932,22 +932,18 @@ func MigrateRepository(doer, u *User, opts MigrateRepoOptions) (*Repository, err
 		}
 	}
 
-	// Check if repository is empty.
-	_, stderr, err := com.ExecCmdDir(repoPath, "git", "log", "-1")
+	gitRepo, err := git.OpenRepository(repoPath)
 	if err != nil {
-		if strings.Contains(stderr, "fatal: bad default revision 'HEAD'") {
-			repo.IsEmpty = true
-		} else {
-			return repo, fmt.Errorf("check empty: %v - %s", err, stderr)
-		}
+		return repo, fmt.Errorf("OpenRepository: %v", err)
+	}
+
+	repo.IsEmpty, err = gitRepo.IsEmpty()
+	if err != nil {
+		return repo, fmt.Errorf("git.IsEmpty: %v", err)
 	}
 
 	if !repo.IsEmpty {
 		// Try to get HEAD branch and set it as default branch.
-		gitRepo, err := git.OpenRepository(repoPath)
-		if err != nil {
-			return repo, fmt.Errorf("OpenRepository: %v", err)
-		}
 		headBranch, err := gitRepo.GetHEADBranch()
 		if err != nil {
 			return repo, fmt.Errorf("GetHEADBranch: %v", err)
@@ -1072,20 +1068,20 @@ func initRepoCommit(tmpPath string, sig *git.Signature) (err error) {
 	var stderr string
 	if _, stderr, err = process.GetManager().ExecDir(-1,
 		tmpPath, fmt.Sprintf("initRepoCommit (git add): %s", tmpPath),
-		"git", "add", "--all"); err != nil {
+		git.GitExecutable, "add", "--all"); err != nil {
 		return fmt.Errorf("git add: %s", stderr)
 	}
 
 	if _, stderr, err = process.GetManager().ExecDir(-1,
 		tmpPath, fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath),
-		"git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),
+		git.GitExecutable, "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),
 		"-m", "Initial commit"); err != nil {
 		return fmt.Errorf("git commit: %s", stderr)
 	}
 
 	if _, stderr, err = process.GetManager().ExecDir(-1,
 		tmpPath, fmt.Sprintf("initRepoCommit (git push): %s", tmpPath),
-		"git", "push", "origin", "master"); err != nil {
+		git.GitExecutable, "push", "origin", "master"); err != nil {
 		return fmt.Errorf("git push: %s", stderr)
 	}
 	return nil
@@ -1131,7 +1127,7 @@ func prepareRepoCommit(e Engine, repo *Repository, tmpDir, repoPath string, opts
 	// Clone to temporary path and do the init commit.
 	_, stderr, err := process.GetManager().Exec(
 		fmt.Sprintf("initRepository(git clone): %s", repoPath),
-		"git", "clone", repoPath, tmpDir,
+		git.GitExecutable, "clone", repoPath, tmpDir,
 	)
 	if err != nil {
 		return fmt.Errorf("git clone: %v - %s", err, stderr)
@@ -1390,7 +1386,7 @@ func CreateRepository(doer, u *User, opts CreateRepoOptions) (_ *Repository, err
 
 		_, stderr, err := process.GetManager().ExecDir(-1,
 			repoPath, fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath),
-			"git", "update-server-info")
+			git.GitExecutable, "update-server-info")
 		if err != nil {
 			return nil, errors.New("CreateRepository(git update-server-info): " + stderr)
 		}
@@ -2239,7 +2235,7 @@ func GitGcRepos() error {
 				_, stderr, err := process.GetManager().ExecDir(
 					time.Duration(setting.Git.Timeout.GC)*time.Second,
 					RepoPath(repo.Owner.Name, repo.Name), "Repository garbage collection",
-					"git", args...)
+					git.GitExecutable, args...)
 				if err != nil {
 					return fmt.Errorf("%v: %v", err, stderr)
 				}
@@ -2429,14 +2425,14 @@ func ForkRepository(doer, u *User, oldRepo *Repository, name, desc string) (_ *R
 	repoPath := RepoPath(u.Name, repo.Name)
 	_, stderr, err := process.GetManager().ExecTimeout(10*time.Minute,
 		fmt.Sprintf("ForkRepository(git clone): %s/%s", u.Name, repo.Name),
-		"git", "clone", "--bare", oldRepo.repoPath(sess), repoPath)
+		git.GitExecutable, "clone", "--bare", oldRepo.repoPath(sess), repoPath)
 	if err != nil {
 		return nil, fmt.Errorf("git clone: %v", stderr)
 	}
 
 	_, stderr, err = process.GetManager().ExecDir(-1,
 		repoPath, fmt.Sprintf("ForkRepository(git update-server-info): %s", repoPath),
-		"git", "update-server-info")
+		git.GitExecutable, "update-server-info")
 	if err != nil {
 		return nil, fmt.Errorf("git update-server-info: %v", stderr)
 	}

diff --git a/models/repo_mirror.go b/models/repo_mirror.go
@@ -216,7 +216,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) {
 
 	_, stderr, err := process.GetManager().ExecDir(
 		timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath),
-		"git", gitArgs...)
+		git.GitExecutable, gitArgs...)
 	if err != nil {
 		// sanitize the output, since it may contain the remote address, which may
 		// contain a password
@@ -250,7 +250,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) {
 	if m.Repo.HasWiki() {
 		if _, stderr, err := process.GetManager().ExecDir(
 			timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath),
-			"git", "remote", "update", "--prune"); err != nil {
+			git.GitExecutable, "remote", "update", "--prune"); err != nil {
 			// sanitize the output, since it may contain the remote address, which may
 			// contain a password
 			message, err := sanitizeOutput(stderr, wikiPath)

diff --git a/models/update.go b/models/update.go
@@ -7,7 +7,6 @@ package models
 import (
 	"container/list"
 	"fmt"
-	"os/exec"
 	"strings"
 	"time"
 
@@ -193,9 +192,8 @@ func pushUpdate(opts PushUpdateOptions) (repo *Repository, err error) {
 
 	repoPath := RepoPath(opts.RepoUserName, opts.RepoName)
 
-	gitUpdate := exec.Command("git", "update-server-info")
-	gitUpdate.Dir = repoPath
-	if err = gitUpdate.Run(); err != nil {
+	_, err = git.NewCommand("update-server-info").RunInDir(repoPath)
+	if err != nil {
 		return nil, fmt.Errorf("Failed to call 'git update-server-info': %v", err)
 	}
 

diff --git a/models/git_blame.go → modules/git/blame.go b/models/git_blame.go → modules/git/blame.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
-package models
+package git
 
 import (
 	"bufio"
@@ -12,7 +12,6 @@ import (
 	"os/exec"
 	"regexp"
 
-	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/process"
 )
 
@@ -88,12 +87,12 @@ func (r *BlameReader) Close() error {
 
 // CreateBlameReader creates reader for given repository, commit and file
 func CreateBlameReader(repoPath, commitID, file string) (*BlameReader, error) {
-	_, err := git.OpenRepository(repoPath)
+	_, err := OpenRepository(repoPath)
 	if err != nil {
 		return nil, err
 	}
 
-	return createBlameReader(repoPath, "git", "blame", commitID, "--porcelain", "--", file)
+	return createBlameReader(repoPath, GitExecutable, "blame", commitID, "--porcelain", "--", file)
 }
 
 func createBlameReader(dir string, command ...string) (*BlameReader, error) {

diff --git a/models/git_blame_test.go → modules/git/blame_test.go b/models/git_blame_test.go → modules/git/blame_test.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a MIT-style
 // license that can be found in the LICENSE file.
 
-package models
+package git
 
 import (
 	"io/ioutil"

diff --git a/modules/git/command.go b/modules/git/command.go
@@ -12,6 +12,8 @@ import (
 	"os/exec"
 	"strings"
 	"time"
+
+	"code.gitea.io/gitea/modules/process"
 )
 
 var (
@@ -84,6 +86,9 @@ func (c *Command) RunInDirTimeoutEnvFullPipeline(env []string, timeout time.Dura
 		return err
 	}
 
+	pid := process.GetManager().Add(fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir), cmd)
+	defer process.GetManager().Remove(pid)
+
 	if err := cmd.Wait(); err != nil {
 		return err
 	}

diff --git a/modules/git/repo.go b/modules/git/repo.go
@@ -107,6 +107,20 @@ func OpenRepository(repoPath string) (*Repository, error) {
 	}, nil
 }
 
+// IsEmpty Check if repository is empty.
+func (repo *Repository) IsEmpty() (bool, error) {
+	var errbuf strings.Builder
+	if err := NewCommand("log", "-1").RunInDirPipeline(repo.Path, nil, &errbuf); err != nil {
+		if strings.Contains(errbuf.String(), "fatal: bad default revision 'HEAD'") ||
+			strings.Contains(errbuf.String(), "fatal: your current branch 'master' does not have any commits yet") {
+			return true, nil
+		}
+		return true, fmt.Errorf("check empty: %v - %s", err, errbuf.String())
+	}
+
+	return false, nil
+}
+
 // CloneRepoOptions options when clone a repository
 type CloneRepoOptions struct {
 	Timeout    time.Duration

diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go
@@ -5,6 +5,7 @@
 package git
 
 import (
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -24,3 +25,12 @@ func TestGetLatestCommitTime(t *testing.T) {
 	assert.NoError(t, err)
 	assert.True(t, lct.Unix() > refTime.Unix(), "%d not greater than %d", lct, refTime)
 }
+
+func TestRepoIsEmpty(t *testing.T) {
+	emptyRepo2Path := filepath.Join(testReposDir, "repo2_empty")
+	repo, err := OpenRepository(emptyRepo2Path)
+	assert.NoError(t, err)
+	isEmpty, err := repo.IsEmpty()
+	assert.NoError(t, err)
+	assert.True(t, isEmpty)
+}
diff --git a/modules/git/tests/repos/repo2_empty/HEAD b/modules/git/tests/repos/repo2_empty/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/master
diff --git a/modules/git/tests/repos/repo2_empty/config b/modules/git/tests/repos/repo2_empty/config
@@ -0,0 +1,6 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = true
+	ignorecase = true
+	precomposeunicode = true
diff --git a/modules/git/tests/repos/repo2_empty/description b/modules/git/tests/repos/repo2_empty/description
@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.
diff --git a/modules/git/tests/repos/repo2_empty/hooks/applypatch-msg.sample b/modules/git/tests/repos/repo2_empty/hooks/applypatch-msg.sample
@@ -0,0 +1,15 @@
+#!/bin/sh
+#
+# An example hook script to check the commit log message taken by
+# applypatch from an e-mail message.
+#
+# The hook should exit with non-zero status after issuing an
+# appropriate message if it wants to stop the commit.  The hook is
+# allowed to edit the commit message file.
+#
+# To enable this hook, rename this file to "applypatch-msg".
+
+. git-sh-setup
+commitmsg="$(git rev-parse --git-path hooks/commit-msg)"
+test -x "$commitmsg" && exec "$commitmsg" ${1+"$@"}
+:
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Unnamed repository; edit this file 'description' to name the repository.