Skip to content

Delete old git.NewCommand() and use it as git.NewCommandContext() #18552

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

Merged
merged 11 commits into from
Feb 6, 2022
2 changes: 1 addition & 1 deletion cmd/hook.go
Original file line number Diff line number Diff line change
@@ -309,7 +309,7 @@ func runHookPostReceive(c *cli.Context) error {
defer cancel()

// First of all run update-server-info no matter what
if _, err := git.NewCommandContext(ctx, "update-server-info").Run(); err != nil {
if _, err := git.NewCommand(ctx, "update-server-info").Run(); err != nil {
return fmt.Errorf("Failed to call 'git update-server-info': %v", err)
}

4 changes: 2 additions & 2 deletions integrations/api_repo_git_tags_test.go
Original file line number Diff line number Diff line change
@@ -28,8 +28,8 @@ func TestAPIGitTags(t *testing.T) {
token := getTokenForLoggedInUser(t, session)

// Set up git config for the tagger
git.NewCommand("config", "user.name", user.Name).RunInDir(repo.RepoPath())
git.NewCommand("config", "user.email", user.Email).RunInDir(repo.RepoPath())
git.NewCommand(git.DefaultContext, "config", "user.name", user.Name).RunInDir(repo.RepoPath())
git.NewCommand(git.DefaultContext, "config", "user.email", user.Email).RunInDir(repo.RepoPath())

gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()
12 changes: 6 additions & 6 deletions integrations/git_helper_for_declarative_test.go
Original file line number Diff line number Diff line change
@@ -134,7 +134,7 @@ func doGitInitTestRepository(dstPath string) func(*testing.T) {
// Init repository in dstPath
assert.NoError(t, git.InitRepository(git.DefaultContext, dstPath, false))
// forcibly set default branch to master
_, err := git.NewCommand("symbolic-ref", "HEAD", git.BranchPrefix+"master").RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "symbolic-ref", "HEAD", git.BranchPrefix+"master").RunInDir(dstPath)
assert.NoError(t, err)
assert.NoError(t, os.WriteFile(filepath.Join(dstPath, "README.md"), []byte(fmt.Sprintf("# Testing Repository\n\nOriginally created in: %s", dstPath)), 0o644))
assert.NoError(t, git.AddChanges(dstPath, true))
@@ -153,28 +153,28 @@ func doGitInitTestRepository(dstPath string) func(*testing.T) {

func doGitAddRemote(dstPath, remoteName string, u *url.URL) func(*testing.T) {
return func(t *testing.T) {
_, err := git.NewCommand("remote", "add", remoteName, u.String()).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "remote", "add", remoteName, u.String()).RunInDir(dstPath)
assert.NoError(t, err)
}
}

func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
_, err := git.NewCommand(append([]string{"push", "-u"}, args...)...).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, append([]string{"push", "-u"}, args...)...).RunInDir(dstPath)
assert.NoError(t, err)
}
}

func doGitPushTestRepositoryFail(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
_, err := git.NewCommand(append([]string{"push"}, args...)...).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, append([]string{"push"}, args...)...).RunInDir(dstPath)
assert.Error(t, err)
}
}

func doGitCreateBranch(dstPath, branch string) func(*testing.T) {
return func(t *testing.T) {
_, err := git.NewCommand("checkout", "-b", branch).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "checkout", "-b", branch).RunInDir(dstPath)
assert.NoError(t, err)
}
}
@@ -188,7 +188,7 @@ func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) {

func doGitMerge(dstPath string, args ...string) func(*testing.T) {
return func(t *testing.T) {
_, err := git.NewCommand(append([]string{"merge"}, args...)...).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, append([]string{"merge"}, args...)...).RunInDir(dstPath)
assert.NoError(t, err)
}
}
22 changes: 11 additions & 11 deletions integrations/git_test.go
Original file line number Diff line number Diff line change
@@ -160,9 +160,9 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin
return
}
prefix := "lfs-data-file-"
_, err := git.NewCommand("lfs").AddArguments("install").RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("install").RunInDir(dstPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("track", prefix+"*").RunInDir(dstPath)
_, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("track", prefix+"*").RunInDir(dstPath)
assert.NoError(t, err)
err = git.AddChanges(dstPath, false, ".gitattributes")
assert.NoError(t, err)
@@ -292,20 +292,20 @@ func lockTest(t *testing.T, repoPath string) {
}

func lockFileTest(t *testing.T, filename, repoPath string) {
_, err := git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
_, err := git.NewCommand(git.DefaultContext, "lfs").AddArguments("locks").RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("lock", filename).RunInDir(repoPath)
_, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("lock", filename).RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
_, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("locks").RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("unlock", filename).RunInDir(repoPath)
_, err = git.NewCommand(git.DefaultContext, "lfs").AddArguments("unlock", filename).RunInDir(repoPath)
assert.NoError(t, err)
}

func doCommitAndPush(t *testing.T, size int, repoPath, prefix string) string {
name, err := generateCommitWithNewData(size, repoPath, "[email protected]", "User Two", prefix)
assert.NoError(t, err)
_, err = git.NewCommand("push", "origin", "master").RunInDir(repoPath) // Push
_, err = git.NewCommand(git.DefaultContext, "push", "origin", "master").RunInDir(repoPath) // Push
assert.NoError(t, err)
return name
}
@@ -671,7 +671,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB
})

t.Run("Push", func(t *testing.T) {
_, err := git.NewCommand("push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
@@ -692,7 +692,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB
assert.Contains(t, "Testing commit 1", prMsg.Body)
assert.Equal(t, commit, prMsg.Head.Sha)

_, err = git.NewCommand("push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
_, err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
@@ -745,7 +745,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB
})

t.Run("Push2", func(t *testing.T) {
_, err := git.NewCommand("push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
_, err := git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
@@ -757,7 +757,7 @@ func doCreateAgitFlowPull(dstPath string, ctx *APITestContext, baseBranch, headB
assert.Equal(t, false, prMsg.HasMerged)
assert.Equal(t, commit, prMsg.Head.Sha)

_, err = git.NewCommand("push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
_, err = git.NewCommand(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
12 changes: 6 additions & 6 deletions integrations/pull_merge_test.go
Original file line number Diff line number Diff line change
@@ -269,19 +269,19 @@ func TestCantMergeUnrelated(t *testing.T) {
}).(*repo_model.Repository)
path := repo_model.RepoPath(user1.Name, repo1.Name)

_, err := git.NewCommand("read-tree", "--empty").RunInDir(path)
_, err := git.NewCommand(git.DefaultContext, "read-tree", "--empty").RunInDir(path)
assert.NoError(t, err)

stdin := bytes.NewBufferString("Unrelated File")
var stdout strings.Builder
err = git.NewCommand("hash-object", "-w", "--stdin").RunInDirFullPipeline(path, &stdout, nil, stdin)
err = git.NewCommand(git.DefaultContext, "hash-object", "-w", "--stdin").RunInDirFullPipeline(path, &stdout, nil, stdin)
assert.NoError(t, err)
sha := strings.TrimSpace(stdout.String())

_, err = git.NewCommand("update-index", "--add", "--replace", "--cacheinfo", "100644", sha, "somewher-over-the-rainbow").RunInDir(path)
_, err = git.NewCommand(git.DefaultContext, "update-index", "--add", "--replace", "--cacheinfo", "100644", sha, "somewher-over-the-rainbow").RunInDir(path)
assert.NoError(t, err)

treeSha, err := git.NewCommand("write-tree").RunInDir(path)
treeSha, err := git.NewCommand(git.DefaultContext, "write-tree").RunInDir(path)
assert.NoError(t, err)
treeSha = strings.TrimSpace(treeSha)

@@ -301,11 +301,11 @@ func TestCantMergeUnrelated(t *testing.T) {
_, _ = messageBytes.WriteString("\n")

stdout.Reset()
err = git.NewCommand("commit-tree", treeSha).RunInDirTimeoutEnvFullPipeline(env, -1, path, &stdout, nil, messageBytes)
err = git.NewCommand(git.DefaultContext, "commit-tree", treeSha).RunInDirTimeoutEnvFullPipeline(env, -1, path, &stdout, nil, messageBytes)
assert.NoError(t, err)
commitSha := strings.TrimSpace(stdout.String())

_, err = git.NewCommand("branch", "unrelated", commitSha).RunInDir(path)
_, err = git.NewCommand(git.DefaultContext, "branch", "unrelated", commitSha).RunInDir(path)
assert.NoError(t, err)

testEditFileToNewBranch(t, session, "user1", "repo1", "master", "conflict", "README.md", "Hello, World (Edited Once)\n")
4 changes: 2 additions & 2 deletions integrations/repo_tag_test.go
Original file line number Diff line number Diff line change
@@ -66,10 +66,10 @@ func TestCreateNewTagProtected(t *testing.T) {

doGitClone(dstPath, u)(t)

_, err = git.NewCommand("tag", "v-2").RunInDir(dstPath)
_, err = git.NewCommand(git.DefaultContext, "tag", "v-2").RunInDir(dstPath)
assert.NoError(t, err)

_, err = git.NewCommand("push", "--tags").RunInDir(dstPath)
_, err = git.NewCommand(git.DefaultContext, "push", "--tags").RunInDir(dstPath)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Tag v-2 is protected")
})
8 changes: 4 additions & 4 deletions models/migrations/v128.go
Original file line number Diff line number Diff line change
@@ -83,17 +83,17 @@ func fixMergeBase(x *xorm.Engine) error {

if !pr.HasMerged {
var err error
pr.MergeBase, err = git.NewCommand("merge-base", "--", pr.BaseBranch, gitRefName).RunInDir(repoPath)
pr.MergeBase, err = git.NewCommand(git.DefaultContext, "merge-base", "--", pr.BaseBranch, gitRefName).RunInDir(repoPath)
if err != nil {
var err2 error
pr.MergeBase, err2 = git.NewCommand("rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath)
pr.MergeBase, err2 = git.NewCommand(git.DefaultContext, "rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath)
if err2 != nil {
log.Error("Unable to get merge base for PR ID %d, Index %d in %s/%s. Error: %v & %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err, err2)
continue
}
}
} else {
parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
parentsString, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
if err != nil {
log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
continue
@@ -106,7 +106,7 @@ func fixMergeBase(x *xorm.Engine) error {
args := append([]string{"merge-base", "--"}, parents[1:]...)
args = append(args, gitRefName)

pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath)
pr.MergeBase, err = git.NewCommand(git.DefaultContext, args...).RunInDir(repoPath)
if err != nil {
log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
continue
4 changes: 2 additions & 2 deletions models/migrations/v134.go
Original file line number Diff line number Diff line change
@@ -80,7 +80,7 @@ func refixMergeBase(x *xorm.Engine) error {

gitRefName := fmt.Sprintf("refs/pull/%d/head", pr.Index)

parentsString, err := git.NewCommand("rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
parentsString, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
if err != nil {
log.Error("Unable to get parents for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
continue
@@ -94,7 +94,7 @@ func refixMergeBase(x *xorm.Engine) error {
args := append([]string{"merge-base", "--"}, parents[1:]...)
args = append(args, gitRefName)

pr.MergeBase, err = git.NewCommand(args...).RunInDir(repoPath)
pr.MergeBase, err = git.NewCommand(git.DefaultContext, args...).RunInDir(repoPath)
if err != nil {
log.Error("Unable to get merge base for merged PR ID %d, Index %d in %s/%s. Error: %v", pr.ID, pr.Index, baseRepo.OwnerName, baseRepo.Name, err)
continue
8 changes: 4 additions & 4 deletions modules/doctor/mergebase.go
Original file line number Diff line number Diff line change
@@ -44,17 +44,17 @@ func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) erro

if !pr.HasMerged {
var err error
pr.MergeBase, err = git.NewCommandContext(ctx, "merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunInDir(repoPath)
pr.MergeBase, err = git.NewCommand(ctx, "merge-base", "--", pr.BaseBranch, pr.GetGitRefName()).RunInDir(repoPath)
if err != nil {
var err2 error
pr.MergeBase, err2 = git.NewCommandContext(ctx, "rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath)
pr.MergeBase, err2 = git.NewCommand(ctx, "rev-parse", git.BranchPrefix+pr.BaseBranch).RunInDir(repoPath)
if err2 != nil {
logger.Warn("Unable to get merge base for PR ID %d, #%d onto %s in %s/%s. Error: %v & %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err, err2)
return nil
}
}
} else {
parentsString, err := git.NewCommandContext(ctx, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
parentsString, err := git.NewCommand(ctx, "rev-list", "--parents", "-n", "1", pr.MergedCommitID).RunInDir(repoPath)
if err != nil {
logger.Warn("Unable to get parents for merged PR ID %d, #%d onto %s in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)
return nil
@@ -67,7 +67,7 @@ func checkPRMergeBase(ctx context.Context, logger log.Logger, autofix bool) erro
args := append([]string{"merge-base", "--"}, parents[1:]...)
args = append(args, pr.GetGitRefName())

pr.MergeBase, err = git.NewCommandContext(ctx, args...).RunInDir(repoPath)
pr.MergeBase, err = git.NewCommand(ctx, args...).RunInDir(repoPath)
if err != nil {
logger.Warn("Unable to get merge base for merged PR ID %d, #%d onto %s in %s/%s. Error: %v", pr.ID, pr.Index, pr.BaseBranch, pr.BaseRepo.OwnerName, pr.BaseRepo.Name, err)
return nil
4 changes: 2 additions & 2 deletions modules/doctor/misc.go
Original file line number Diff line number Diff line change
@@ -95,11 +95,11 @@ func checkEnablePushOptions(ctx context.Context, logger log.Logger, autofix bool
defer r.Close()

if autofix {
_, err := git.NewCommandContext(ctx, "config", "receive.advertisePushOptions", "true").RunInDir(r.Path)
_, err := git.NewCommand(ctx, "config", "receive.advertisePushOptions", "true").RunInDir(r.Path)
return err
}

value, err := git.NewCommandContext(ctx, "config", "receive.advertisePushOptions").RunInDir(r.Path)
value, err := git.NewCommand(ctx, "config", "receive.advertisePushOptions").RunInDir(r.Path)
if err != nil {
return err
}
6 changes: 3 additions & 3 deletions modules/git/batch_reader.go
Original file line number Diff line number Diff line change
@@ -32,7 +32,7 @@ type WriteCloserError interface {
// This is needed otherwise the git cat-file will hang for invalid repositories.
func EnsureValidGitRepository(ctx context.Context, repoPath string) error {
stderr := strings.Builder{}
err := NewCommandContext(ctx, "rev-parse").
err := NewCommand(ctx, "rev-parse").
SetDescription(fmt.Sprintf("%s rev-parse [repo_path: %s]", GitExecutable, repoPath)).
RunInDirFullPipeline(repoPath, nil, &stderr, nil)
if err != nil {
@@ -59,7 +59,7 @@ func CatFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,

go func() {
stderr := strings.Builder{}
err := NewCommandContext(ctx, "cat-file", "--batch-check").
err := NewCommand(ctx, "cat-file", "--batch-check").
SetDescription(fmt.Sprintf("%s cat-file --batch-check [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
if err != nil {
@@ -98,7 +98,7 @@ func CatFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi

go func() {
stderr := strings.Builder{}
err := NewCommandContext(ctx, "cat-file", "--batch").
err := NewCommand(ctx, "cat-file", "--batch").
SetDescription(fmt.Sprintf("%s cat-file --batch [repo_path: %s] (%s:%d)", GitExecutable, repoPath, filename, line)).
RunInDirFullPipeline(repoPath, batchStdoutWriter, &stderr, batchStdinReader)
if err != nil {
7 changes: 1 addition & 6 deletions modules/git/command.go
Original file line number Diff line number Diff line change
@@ -46,12 +46,7 @@ func (c *Command) String() string {
}

// NewCommand creates and returns a new Git Command based on given command and arguments.
func NewCommand(args ...string) *Command {
return NewCommandContext(DefaultContext, args...)
}

// NewCommandContext creates and returns a new Git Command based on given command and arguments.
func NewCommandContext(ctx context.Context, args ...string) *Command {
func NewCommand(ctx context.Context, args ...string) *Command {
// Make an explicit copy of globalCommandArgs, otherwise append might overwrite it
cargs := make([]string, len(globalCommandArgs))
copy(cargs, globalCommandArgs)
4 changes: 2 additions & 2 deletions modules/git/command_test.go
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@ func TestRunInDirTimeoutPipelineNoTimeout(t *testing.T) {
maxLoops := 1000

// 'git --version' does not block so it must be finished before the timeout triggered.
cmd := NewCommand("--version")
cmd := NewCommand(context.Background(), "--version")
for i := 0; i < maxLoops; i++ {
if err := cmd.RunInDirTimeoutPipeline(-1, "", nil, nil); err != nil {
t.Fatal(err)
@@ -29,7 +29,7 @@ func TestRunInDirTimeoutPipelineAlwaysTimeout(t *testing.T) {
maxLoops := 1000

// 'git hash-object --stdin' blocks on stdin so we can have the timeout triggered.
cmd := NewCommand("hash-object", "--stdin")
cmd := NewCommand(context.Background(), "hash-object", "--stdin")
for i := 0; i < maxLoops; i++ {
if err := cmd.RunInDirTimeoutPipeline(1*time.Microsecond, "", nil, nil); err != nil {
if err != context.DeadlineExceeded {
Loading