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.

Sign up for GitHub

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
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
fe9bf1d
Remove git.NewCommand()
6543 Feb 2, 2022
9e2ab59
rename
6543 Feb 2, 2022
fafc93e
Merge branch 'main' into NO-git.NewCommand
6543 Feb 3, 2022
a2ac5af
Merge branch 'main' into NO-git.NewCommand
zeripath Feb 3, 2022
c138007
Merge branch 'main' into NO-git.NewCommand
6543 Feb 4, 2022
41fca89
Merge branch 'main' into NO-git.NewCommand
6543 Feb 4, 2022
f07c53b
Merge branch 'main' into NO-git.NewCommand
6543 Feb 5, 2022
6bd0ca3
Merge branch 'main' into NO-git.NewCommand
lunny Feb 6, 2022
91da34e
Merge branch 'main' into NO-git.NewCommand
lunny Feb 6, 2022
6824c16
Merge branch 'main' into NO-git.NewCommand
6543 Feb 6, 2022
3943c22
Merge branch 'main' into NO-git.NewCommand
6543 Feb 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions integrations/api_repo_git_tags_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(git.DefaultContext, "config", "user.name", user.Name).RunInDir(repo.RepoPath())
git.NewCommandContext(git.DefaultContext, "config", "user.email", user.Email).RunInDir(repo.RepoPath())

gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()
Expand Down
12 changes: 6 additions & 6 deletions integrations/git_helper_for_declarative_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(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))
Expand All @@ -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.NewCommandContext(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.NewCommandContext(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.NewCommandContext(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.NewCommandContext(git.DefaultContext, "checkout", "-b", branch).RunInDir(dstPath)
assert.NoError(t, err)
}
}
Expand All @@ -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.NewCommandContext(git.DefaultContext, append([]string{"merge"}, args...)...).RunInDir(dstPath)
assert.NoError(t, err)
}
}
Expand Down
22 changes: 11 additions & 11 deletions integrations/git_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(git.DefaultContext, "lfs").AddArguments("install").RunInDir(dstPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("track", prefix+"*").RunInDir(dstPath)
_, err = git.NewCommandContext(git.DefaultContext, "lfs").AddArguments("track", prefix+"*").RunInDir(dstPath)
assert.NoError(t, err)
err = git.AddChanges(dstPath, false, ".gitattributes")
assert.NoError(t, err)
Expand Down Expand Up @@ -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.NewCommandContext(git.DefaultContext, "lfs").AddArguments("locks").RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("lock", filename).RunInDir(repoPath)
_, err = git.NewCommandContext(git.DefaultContext, "lfs").AddArguments("lock", filename).RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("locks").RunInDir(repoPath)
_, err = git.NewCommandContext(git.DefaultContext, "lfs").AddArguments("locks").RunInDir(repoPath)
assert.NoError(t, err)
_, err = git.NewCommand("lfs").AddArguments("unlock", filename).RunInDir(repoPath)
_, err = git.NewCommandContext(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.NewCommandContext(git.DefaultContext, "push", "origin", "master").RunInDir(repoPath) // Push
assert.NoError(t, err)
return name
}
Expand Down Expand Up @@ -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.NewCommandContext(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
Expand All @@ -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.NewCommandContext(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
Expand Down Expand Up @@ -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.NewCommandContext(git.DefaultContext, "push", "origin", "HEAD:refs/for/master", "-o", "topic="+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
Expand All @@ -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.NewCommandContext(git.DefaultContext, "push", "origin", "HEAD:refs/for/master/test/"+headBranch).RunInDir(dstPath)
if !assert.NoError(t, err) {
return
}
Expand Down
12 changes: 6 additions & 6 deletions integrations/pull_merge_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(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.NewCommandContext(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.NewCommandContext(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.NewCommandContext(git.DefaultContext, "write-tree").RunInDir(path)
assert.NoError(t, err)
treeSha = strings.TrimSpace(treeSha)

Expand All @@ -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.NewCommandContext(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.NewCommandContext(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")
Expand Down
4 changes: 2 additions & 2 deletions integrations/repo_tag_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ func TestCreateNewTagProtected(t *testing.T) {

doGitClone(dstPath, u)(t)

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

_, err = git.NewCommand("push", "--tags").RunInDir(dstPath)
_, err = git.NewCommandContext(git.DefaultContext, "push", "--tags").RunInDir(dstPath)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Tag v-2 is protected")
})
Expand Down
8 changes: 4 additions & 4 deletions models/migrations/v128.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(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.NewCommandContext(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.NewCommandContext(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
Expand All @@ -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.NewCommandContext(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
Expand Down
4 changes: 2 additions & 2 deletions models/migrations/v134.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.NewCommandContext(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
Expand All @@ -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.NewCommandContext(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
Expand Down
5 changes: 0 additions & 5 deletions modules/git/command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ func (c *Command) String() string {
return fmt.Sprintf("%s %s", c.name, strings.Join(c.args, " "))
}

// 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 {
// Make an explicit copy of globalCommandArgs, otherwise append might overwrite it
Expand Down
4 changes: 2 additions & 2 deletions modules/git/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 := NewCommandContext(context.Background(), "--version")
for i := 0; i < maxLoops; i++ {
if err := cmd.RunInDirTimeoutPipeline(-1, "", nil, nil); err != nil {
t.Fatal(err)
Expand All @@ -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 := NewCommandContext(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 {
Expand Down
2 changes: 1 addition & 1 deletion modules/git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func LoadGitVersion() error {
return nil
}

stdout, err := NewCommand("version").Run()
stdout, err := NewCommandContext(context.Background(), "version").Run()
if err != nil {
return err
}
Expand Down