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

Monitor all git commands; move blame to git package and replace git as a variable #6864

Merged
merged 19 commits into from
Jun 26, 2019
Merged
6 changes: 3 additions & 3 deletions contrib/pr/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
16 changes: 8 additions & 8 deletions models/git_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,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 {
Expand All @@ -685,7 +685,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
Expand Down Expand Up @@ -749,23 +749,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)
Expand Down
8 changes: 4 additions & 4 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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]) {
Expand Down
2 changes: 1 addition & 1 deletion models/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
34 changes: 15 additions & 19 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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'") {
Copy link
Member

@sapk sapk May 7, 2019

Choose a reason for hiding this comment

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

This change seems to change how this function works. This will always error without setting the repo.IsEmpty previously to return. Is it intended because it fix an issue ? In place, of starting another cli command inside gitRepo.IsEmpty() couldn't we use a method IsEmptyRepoErr(err) parsing the error of OpenRepository ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I run make unit-test locally successfully but don't know why it failed on drone. I move these codes to git package so that external shouldn't know how it works.

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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions models/repo_mirror.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 2 additions & 4 deletions models/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package models
import (
"container/list"
"fmt"
"os/exec"
"strings"
"time"

Expand Down Expand Up @@ -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)
}

Expand Down
7 changes: 3 additions & 4 deletions models/git_blame.go → modules/git/blame.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -12,7 +12,6 @@ import (
"os/exec"
"regexp"

"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/process"
)

Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion models/git_blame_test.go → modules/git/blame_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions modules/git/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"os/exec"
"strings"
"time"

"code.gitea.io/gitea/modules/process"
)

var (
Expand Down Expand Up @@ -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
}
Expand Down
14 changes: 14 additions & 0 deletions modules/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions modules/git/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package git

import (
"path/filepath"
"testing"
"time"

Expand All @@ -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)
}
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo2_empty/HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ref: refs/heads/master
6 changes: 6 additions & 0 deletions modules/git/tests/repos/repo2_empty/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[core]
repositoryformatversion = 0
filemode = true
bare = true
ignorecase = true
precomposeunicode = true
1 change: 1 addition & 0 deletions modules/git/tests/repos/repo2_empty/description
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Unnamed repository; edit this file 'description' to name the repository.
15 changes: 15 additions & 0 deletions modules/git/tests/repos/repo2_empty/hooks/applypatch-msg.sample
Original file line number Diff line number Diff line change
@@ -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+"$@"}
:
Loading