Fix datarace on git.GlobalCommandArgs on tests (#9162)

* fix datarace on git.GlobalCommandArgs on tests

* fix tests

* fix tests

* fix tests

diff --git a/integrations/git_helper_for_declarative_test.go b/integrations/git_helper_for_declarative_test.go
index 024bf87a3ca07d2acee06b1d901e3ecb85377c6f..1eb1f1dc8b99ba8b2de7c705d6e1f31c85b01ea7 100644 (file)
--- a/integrations/git_helper_for_declarative_test.go
+++ b/integrations/git_helper_for_declarative_test.go
@@ -63,7 +63,6 @@ func createSSHUrl(gitPath string, u *url.URL) *url.URL {
 
 func allowLFSFilters() []string {
        // Now here we should explicitly allow lfs filters to run
-       globalArgs := git.GlobalCommandArgs
        filteredLFSGlobalArgs := make([]string, len(git.GlobalCommandArgs))
        j := 0
        for _, arg := range git.GlobalCommandArgs {
@@ -74,9 +73,7 @@ func allowLFSFilters() []string {
                        j++
                }
        }
-       filteredLFSGlobalArgs = filteredLFSGlobalArgs[:j]
-       git.GlobalCommandArgs = filteredLFSGlobalArgs
-       return globalArgs
+       return filteredLFSGlobalArgs[:j]
 }
 
 func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bool) {
@@ -107,9 +104,7 @@ func onGiteaRun(t *testing.T, callback func(*testing.T, *url.URL), prepare ...bo
 
 func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) {
        return func(t *testing.T) {
-               oldGlobals := allowLFSFilters()
-               assert.NoError(t, git.Clone(u.String(), dstLocalPath, git.CloneRepoOptions{}))
-               git.GlobalCommandArgs = oldGlobals
+               assert.NoError(t, git.CloneWithArgs(u.String(), dstLocalPath, allowLFSFilters(), git.CloneRepoOptions{}))
                assert.True(t, com.IsExist(filepath.Join(dstLocalPath, "README.md")))
        }
 }
@@ -170,9 +165,7 @@ func doGitCreateBranch(dstPath, branch string) func(*testing.T) {
 
 func doGitCheckoutBranch(dstPath string, args ...string) func(*testing.T) {
        return func(t *testing.T) {
-               oldGlobals := allowLFSFilters()
-               _, err := git.NewCommand(append([]string{"checkout"}, args...)...).RunInDir(dstPath)
-               git.GlobalCommandArgs = oldGlobals
+               _, err := git.NewCommandNoGlobals(append(append(allowLFSFilters(), "checkout"), args...)...).RunInDir(dstPath)
                assert.NoError(t, err)
        }
 }
@@ -186,9 +179,7 @@ func doGitMerge(dstPath string, args ...string) func(*testing.T) {
 
 func doGitPull(dstPath string, args ...string) func(*testing.T) {
        return func(t *testing.T) {
-               oldGlobals := allowLFSFilters()
-               _, err := git.NewCommand(append([]string{"pull"}, args...)...).RunInDir(dstPath)
-               git.GlobalCommandArgs = oldGlobals
+               _, err := git.NewCommandNoGlobals(append(append(allowLFSFilters(), "pull"), args...)...).RunInDir(dstPath)
                assert.NoError(t, err)
        }
 }

diff --git a/integrations/git_test.go b/integrations/git_test.go
index f6137a947a6506afac6150f48e3885649f072b47..3ca4cc54c2edf688f80296c211a895242195ff38 100644 (file)
--- a/integrations/git_test.go
+++ b/integrations/git_test.go
@@ -148,8 +148,8 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin
                assert.NoError(t, err)
                err = git.AddChanges(dstPath, false, ".gitattributes")
                assert.NoError(t, err)
-               oldGlobals := allowLFSFilters()
-               err = git.CommitChanges(dstPath, git.CommitChangesOptions{
+
+               err = git.CommitChangesWithArgs(dstPath, allowLFSFilters(), git.CommitChangesOptions{
                        Committer: &git.Signature{
                                Email: "user2@example.com",
                                Name:  "User Two",
@@ -163,7 +163,6 @@ func lfsCommitAndPushTest(t *testing.T, dstPath string) (littleLFS, bigLFS strin
                        Message: fmt.Sprintf("Testing commit @ %v", time.Now()),
                })
                assert.NoError(t, err)
-               git.GlobalCommandArgs = oldGlobals
 
                littleLFS, bigLFS = commitAndPushTest(t, dstPath, prefix)
 
@@ -307,12 +306,12 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin
 
        //Commit
        // Now here we should explicitly allow lfs filters to run
-       oldGlobals := allowLFSFilters()
-       err = git.AddChanges(repoPath, false, filepath.Base(tmpFile.Name()))
+       globalArgs := allowLFSFilters()
+       err = git.AddChangesWithArgs(repoPath, globalArgs, false, filepath.Base(tmpFile.Name()))
        if err != nil {
                return "", err
        }
-       err = git.CommitChanges(repoPath, git.CommitChangesOptions{
+       err = git.CommitChangesWithArgs(repoPath, globalArgs, git.CommitChangesOptions{
                Committer: &git.Signature{
                        Email: email,
                        Name:  fullName,
@@ -325,7 +324,6 @@ func generateCommitWithNewData(size int, repoPath, email, fullName, prefix strin
                },
                Message: fmt.Sprintf("Testing commit @ %v", time.Now()),
        })
-       git.GlobalCommandArgs = oldGlobals
        return filepath.Base(tmpFile.Name()), err
 }
 

diff --git a/modules/git/command.go b/modules/git/command.go
index 7772abd2d58c93657a4950c05a8137ac511f8050..65878edb7de66fd8c5fbda616edef8efe653c399 100644 (file)
--- a/modules/git/command.go
+++ b/modules/git/command.go
@@ -52,6 +52,14 @@ func NewCommand(args ...string) *Command {
        }
 }
 
+// NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args
+func NewCommandNoGlobals(args ...string) *Command {
+       return &Command{
+               name: GitExecutable,
+               args: args,
+       }
+}
+
 // AddArguments adds new argument(s) to the command.
 func (c *Command) AddArguments(args ...string) *Command {
        c.args = append(c.args, args...)

diff --git a/modules/git/commit.go b/modules/git/commit.go
index ce55dd55f633a687426228514ee024e0c857473c..0388d5e9beb69ecdcd4a9fc753008c7c67828c57 100644 (file)
--- a/modules/git/commit.go
+++ b/modules/git/commit.go
@@ -207,7 +207,12 @@ func (c *Commit) GetCommitByPath(relpath string) (*Commit, error) {
 
 // AddChanges marks local changes to be ready for commit.
 func AddChanges(repoPath string, all bool, files ...string) error {
-       cmd := NewCommand("add")
+       return AddChangesWithArgs(repoPath, GlobalCommandArgs, all, files...)
+}
+
+// AddChangesWithArgs marks local changes to be ready for commit.
+func AddChangesWithArgs(repoPath string, gloablArgs []string, all bool, files ...string) error {
+       cmd := NewCommandNoGlobals(append(gloablArgs, "add")...)
        if all {
                cmd.AddArguments("--all")
        }
@@ -226,7 +231,15 @@ type CommitChangesOptions struct {
 // CommitChanges commits local changes with given committer, author and message.
 // If author is nil, it will be the same as committer.
 func CommitChanges(repoPath string, opts CommitChangesOptions) error {
-       cmd := NewCommand()
+       cargs := make([]string, len(GlobalCommandArgs))
+       copy(cargs, GlobalCommandArgs)
+       return CommitChangesWithArgs(repoPath, cargs, opts)
+}
+
+// CommitChangesWithArgs commits local changes with given committer, author and message.
+// If author is nil, it will be the same as committer.
+func CommitChangesWithArgs(repoPath string, args []string, opts CommitChangesOptions) error {
+       cmd := NewCommandNoGlobals(args...)
        if opts.Committer != nil {
                cmd.AddArguments("-c", "user.name="+opts.Committer.Name, "-c", "user.email="+opts.Committer.Email)
        }

diff --git a/modules/git/repo.go b/modules/git/repo.go
index ffca2524be1feea9d7bebd4ff5df5e11cf213597..e277f896bf969b3ada611963d7e8c356148e96b3 100644 (file)
--- a/modules/git/repo.go
+++ b/modules/git/repo.go
@@ -165,12 +165,19 @@ type CloneRepoOptions struct {
 
 // Clone clones original repository to target path.
 func Clone(from, to string, opts CloneRepoOptions) (err error) {
+       cargs := make([]string, len(GlobalCommandArgs))
+       copy(cargs, GlobalCommandArgs)
+       return CloneWithArgs(from, to, cargs, opts)
+}
+
+// CloneWithArgs original repository to target path.
+func CloneWithArgs(from, to string, args []string, opts CloneRepoOptions) (err error) {
        toDir := path.Dir(to)
        if err = os.MkdirAll(toDir, os.ModePerm); err != nil {
                return err
        }
 
-       cmd := NewCommand("clone")
+       cmd := NewCommandNoGlobals(args...).AddArguments("clone")
        if opts.Mirror {
                cmd.AddArguments("--mirror")
        }