Refactor process package and introduce ProcessManager{} with tests (#75)

metalmatze · lunny · commit d1006150fb3a · 2017-01-17T13:58:58.000+08:00
* Add a process.Manager singleton with process.GetManager()

* Use process.GetManager everywhere

* Fix godoc comments for process module

* Increment process counter id after locking the mutex
diff --git a/models/git_diff.go b/models/git_diff.go
@@ -483,8 +483,8 @@ func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxL
 		return nil, fmt.Errorf("Start: %v", err)
 	}
 
-	pid := process.Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cmd)
-	defer process.Remove(pid)
+	pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cmd)
+	defer process.GetManager().Remove(pid)
 
 	diff, err := ParsePatch(maxLines, maxLineCharacters, maxFiles, stdout)
 	if err != nil {
diff --git a/models/pull.go b/models/pull.go
@@ -281,49 +281,49 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository) (err error
 	defer os.RemoveAll(path.Dir(tmpBasePath))
 
 	var stderr string
-	if _, stderr, err = process.ExecTimeout(5*time.Minute,
+	if _, stderr, err = process.GetManager().ExecTimeout(5*time.Minute,
 		fmt.Sprintf("PullRequest.Merge (git clone): %s", tmpBasePath),
 		"git", "clone", baseGitRepo.Path, tmpBasePath); err != nil {
 		return fmt.Errorf("git clone: %s", stderr)
 	}
 
 	// Check out base branch.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
 		"git", "checkout", pr.BaseBranch); err != nil {
 		return fmt.Errorf("git checkout: %s", stderr)
 	}
 
 	// Add head repo remote.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath),
 		"git", "remote", "add", "head_repo", headRepoPath); err != nil {
 		return fmt.Errorf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
 	}
 
 	// Merge commits.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath),
 		"git", "fetch", "head_repo"); err != nil {
 		return fmt.Errorf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
 	}
 
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath),
 		"git", "merge", "--no-ff", "--no-commit", "head_repo/"+pr.HeadBranch); err != nil {
 		return fmt.Errorf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr)
 	}
 
 	sig := doer.NewGitSig()
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
 		"git", "commit", fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email),
 		"-m", fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch)); err != nil {
 		return fmt.Errorf("git commit [%s]: %v - %s", tmpBasePath, err, stderr)
 	}
 
 	// Push back to upstream.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+	if _, stderr, err = process.GetManager().ExecDir(-1, tmpBasePath,
 		fmt.Sprintf("PullRequest.Merge (git push): %s", tmpBasePath),
 		"git", "push", baseGitRepo.Path, pr.BaseBranch); err != nil {
 		return fmt.Errorf("git push: %s", stderr)
@@ -437,14 +437,14 @@ func (pr *PullRequest) testPatch() (err error) {
 	defer os.Remove(indexTmpPath)
 
 	var stderr string
-	_, stderr, err = process.ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git read-tree): %d", pr.BaseRepo.ID),
+	_, 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)
 	if err != nil {
 		return fmt.Errorf("git read-tree --index-output=%s %s: %v - %s", indexTmpPath, pr.BaseBranch, err, stderr)
 	}
 
-	_, stderr, err = process.ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git apply --check): %d", pr.BaseRepo.ID),
+	_, 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", "apply", "--check", "--cached", patchPath)
 	if err != nil {
diff --git a/models/release.go b/models/release.go
@@ -10,14 +10,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-xorm/xorm"
-
 	"code.gitea.io/git"
-
 	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
-
 	api "code.gitea.io/sdk/gitea"
+	"github.com/go-xorm/xorm"
 )
 
 // Release represents a release of repository.
@@ -159,7 +156,7 @@ func createTag(gitRepo *git.Repository, rel *Release) error {
 
 func addReleaseAttachments(releaseID int64, attachmentUUIDs []string) (err error) {
 	// Check attachments
-	var attachments = make([]*Attachment,0)
+	var attachments = make([]*Attachment, 0)
 	for _, uuid := range attachmentUUIDs {
 		attach, err := getAttachmentByUUID(x, uuid)
 		if err != nil {
@@ -257,9 +254,10 @@ func GetReleasesByRepoIDAndNames(repoID int64, tagNames []string) (rels []*Relea
 }
 
 type releaseMetaSearch struct {
-	ID [] int64
-	Rel [] *Release
+	ID  []int64
+	Rel []*Release
 }
+
 func (s releaseMetaSearch) Len() int {
 	return len(s.ID)
 }
@@ -272,18 +270,18 @@ func (s releaseMetaSearch) Less(i, j int) bool {
 }
 
 // GetReleaseAttachments retrieves the attachments for releases
-func GetReleaseAttachments(rels ... *Release) (err error){
+func GetReleaseAttachments(rels ...*Release) (err error) {
 	if len(rels) == 0 {
 		return
 	}
 
-	// To keep this efficient as possible sort all releases by id, 
+	// To keep this efficient as possible sort all releases by id,
 	//    select attachments by release id,
 	//    then merge join them
 
 	// Sort
 	var sortedRels = releaseMetaSearch{ID: make([]int64, len(rels)), Rel: make([]*Release, len(rels))}
-	var attachments [] *Attachment
+	var attachments []*Attachment
 	for index, element := range rels {
 		element.Attachments = []*Attachment{}
 		sortedRels.ID[index] = element.ID
@@ -375,7 +373,7 @@ func DeleteReleaseByID(id int64, u *User, delTag bool) error {
 	}
 
 	if delTag {
-		_, stderr, err := process.ExecDir(-1, repo.RepoPath(),
+		_, stderr, err := process.GetManager().ExecDir(-1, repo.RepoPath(),
 			fmt.Sprintf("DeleteReleaseByID (git tag -d): %d", rel.ID),
 			"git", "tag", "-d", rel.TagName)
 		if err != nil && !strings.Contains(stderr, "not found") {
diff --git a/models/repo.go b/models/repo.go
@@ -147,10 +147,10 @@ func NewRepoContext() {
 
 	// Git requires setting user.name and user.email in order to commit changes.
 	for configKey, defaultValue := range map[string]string{"user.name": "Gitea", "user.email": "gitea@fake.local"} {
-		if stdout, stderr, err := process.Exec("NewRepoContext(get setting)", "git", "config", "--get", configKey); err != nil || strings.TrimSpace(stdout) == "" {
+		if stdout, stderr, err := process.GetManager().Exec("NewRepoContext(get setting)", "git", "config", "--get", configKey); err != nil || strings.TrimSpace(stdout) == "" {
 			// ExitError indicates this config is not set
 			if _, ok := err.(*exec.ExitError); ok || strings.TrimSpace(stdout) == "" {
-				if _, stderr, gerr := process.Exec("NewRepoContext(set "+configKey+")", "git", "config", "--global", configKey, defaultValue); gerr != nil {
+				if _, stderr, gerr := process.GetManager().Exec("NewRepoContext(set "+configKey+")", "git", "config", "--global", configKey, defaultValue); gerr != nil {
 					log.Fatal(4, "Fail to set git %s(%s): %s", configKey, gerr, stderr)
 				}
 				log.Info("Git config %s set to %s", configKey, defaultValue)
@@ -161,7 +161,7 @@ func NewRepoContext() {
 	}
 
 	// Set git some configurations.
-	if _, stderr, err := process.Exec("NewRepoContext(git config --global core.quotepath false)",
+	if _, stderr, err := process.GetManager().Exec("NewRepoContext(git config --global core.quotepath false)",
 		"git", "config", "--global", "core.quotepath", "false"); err != nil {
 		log.Fatal(4, "Fail to execute 'git config --global core.quotepath false': %s", stderr)
 	}
@@ -797,20 +797,20 @@ func CleanUpMigrateInfo(repo *Repository) (*Repository, error) {
 // initRepoCommit temporarily changes with work directory.
 func initRepoCommit(tmpPath string, sig *git.Signature) (err error) {
 	var stderr string
-	if _, stderr, err = process.ExecDir(-1,
+	if _, stderr, err = process.GetManager().ExecDir(-1,
 		tmpPath, fmt.Sprintf("initRepoCommit (git add): %s", tmpPath),
 		"git", "add", "--all"); err != nil {
 		return fmt.Errorf("git add: %s", stderr)
 	}
 
-	if _, stderr, err = process.ExecDir(-1,
+	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),
 		"-m", "Initial commit"); err != nil {
 		return fmt.Errorf("git commit: %s", stderr)
 	}
 
-	if _, stderr, err = process.ExecDir(-1,
+	if _, stderr, err = process.GetManager().ExecDir(-1,
 		tmpPath, fmt.Sprintf("initRepoCommit (git push): %s", tmpPath),
 		"git", "push", "origin", "master"); err != nil {
 		return fmt.Errorf("git push: %s", stderr)
@@ -856,8 +856,10 @@ func getRepoInitFile(tp, name string) ([]byte, error) {
 
 func prepareRepoCommit(repo *Repository, tmpDir, repoPath string, opts CreateRepoOptions) error {
 	// Clone to temporary path and do the init commit.
-	_, stderr, err := process.Exec(
-		fmt.Sprintf("initRepository(git clone): %s", repoPath), "git", "clone", repoPath, tmpDir)
+	_, stderr, err := process.GetManager().Exec(
+		fmt.Sprintf("initRepository(git clone): %s", repoPath),
+		"git", "clone", repoPath, tmpDir,
+	)
 	if err != nil {
 		return fmt.Errorf("git clone: %v - %s", err, stderr)
 	}
@@ -1066,7 +1068,7 @@ func CreateRepository(u *User, opts CreateRepoOptions) (_ *Repository, err error
 			return nil, fmt.Errorf("initRepository: %v", err)
 		}
 
-		_, stderr, err := process.ExecDir(-1,
+		_, stderr, err := process.GetManager().ExecDir(-1,
 			repoPath, fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath),
 			"git", "update-server-info")
 		if err != nil {
@@ -1839,7 +1841,7 @@ func GitGcRepos() error {
 				if err := repo.GetOwner(); err != nil {
 					return err
 				}
-				_, stderr, err := process.ExecDir(
+				_, stderr, err := process.GetManager().ExecDir(
 					time.Duration(setting.Git.Timeout.GC)*time.Second,
 					RepoPath(repo.Owner.Name, repo.Name), "Repository garbage collection",
 					"git", args...)
@@ -2192,14 +2194,14 @@ func ForkRepository(u *User, oldRepo *Repository, name, desc string) (_ *Reposit
 	}
 
 	repoPath := RepoPath(u.Name, repo.Name)
-	_, stderr, err := process.ExecTimeout(10*time.Minute,
+	_, stderr, err := process.GetManager().ExecTimeout(10*time.Minute,
 		fmt.Sprintf("ForkRepository(git clone): %s/%s", u.Name, repo.Name),
 		"git", "clone", "--bare", oldRepo.RepoPath(), repoPath)
 	if err != nil {
 		return nil, fmt.Errorf("git clone: %v", stderr)
 	}
 
-	_, stderr, err = process.ExecDir(-1,
+	_, stderr, err = process.GetManager().ExecDir(-1,
 		repoPath, fmt.Sprintf("ForkRepository(git update-server-info): %s", repoPath),
 		"git", "update-server-info")
 	if err != nil {
diff --git a/models/repo_editor.go b/models/repo_editor.go
@@ -212,8 +212,8 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *
 		return nil, fmt.Errorf("Start: %v", err)
 	}
 
-	pid := process.Add(fmt.Sprintf("GetDiffPreview [repo_path: %s]", repo.RepoPath()), cmd)
-	defer process.Remove(pid)
+	pid := process.GetManager().Add(fmt.Sprintf("GetDiffPreview [repo_path: %s]", repo.RepoPath()), cmd)
+	defer process.GetManager().Remove(pid)
 
 	diff, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdout)
 	if err != nil {
diff --git a/models/repo_mirror.go b/models/repo_mirror.go
@@ -137,7 +137,7 @@ func (m *Mirror) runSync() bool {
 		gitArgs = append(gitArgs, "--prune")
 	}
 
-	if _, stderr, err := process.ExecDir(
+	if _, stderr, err := process.GetManager().ExecDir(
 		timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath),
 		"git", gitArgs...); err != nil {
 		desc := fmt.Sprintf("Fail to update mirror repository '%s': %s", repoPath, stderr)
@@ -148,7 +148,7 @@ func (m *Mirror) runSync() bool {
 		return false
 	}
 	if m.Repo.HasWiki() {
-		if _, stderr, err := process.ExecDir(
+		if _, stderr, err := process.GetManager().ExecDir(
 			timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath),
 			"git", "remote", "update", "--prune"); err != nil {
 			desc := fmt.Sprintf("Fail to update mirror wiki repository '%s': %s", wikiPath, stderr)
diff --git a/models/ssh_key.go b/models/ssh_key.go
@@ -197,7 +197,7 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) {
 	}
 	defer os.Remove(tmpName)
 
-	stdout, stderr, err := process.Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)
+	stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)
 	if err != nil {
 		return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr)
 	}
@@ -382,7 +382,7 @@ func addKey(e Engine, key *PublicKey) (err error) {
 	if err = ioutil.WriteFile(tmpPath, []byte(key.Content), 0644); err != nil {
 		return err
 	}
-	stdout, stderr, err := process.Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)
+	stdout, stderr, err := process.GetManager().Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)
 	if err != nil {
 		return fmt.Errorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr)
 	} else if len(stdout) < 2 {
diff --git a/modules/process/manager.go b/modules/process/manager.go
diff --git a/modules/process/manager_test.go b/modules/process/manager_test.go
diff --git a/routers/admin/admin.go b/routers/admin/admin.go

Original file line number	Diff line number	Diff line change
`@@ -483,8 +483,8 @@ func GetDiffRange(repoPath, beforeCommitID, afterCommitID string, maxLines, maxL`
`483`	`483`	`return nil, fmt.Errorf("Start: %v", err)`
`484`	`484`	`}`
`485`	`485`
`486`		`- pid := process.Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cmd)`
`487`		`- defer process.Remove(pid)`
	`486`	`+ pid := process.GetManager().Add(fmt.Sprintf("GetDiffRange [repo_path: %s]", repoPath), cmd)`
	`487`	`+ defer process.GetManager().Remove(pid)`
`488`	`488`
`489`	`489`	`diff, err := ParsePatch(maxLines, maxLineCharacters, maxFiles, stdout)`
`490`	`490`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -212,8 +212,8 @@ func (repo Repository) GetDiffPreview(branch, treePath, content string) (diff `
`212`	`212`	`return nil, fmt.Errorf("Start: %v", err)`
`213`	`213`	`}`
`214`	`214`
`215`		`- pid := process.Add(fmt.Sprintf("GetDiffPreview [repo_path: %s]", repo.RepoPath()), cmd)`
`216`		`- defer process.Remove(pid)`
	`215`	`+ pid := process.GetManager().Add(fmt.Sprintf("GetDiffPreview [repo_path: %s]", repo.RepoPath()), cmd)`
	`216`	`+ defer process.GetManager().Remove(pid)`
`217`	`217`
`218`	`218`	`diff, err = ParsePatch(setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdout)`
`219`	`219`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -197,7 +197,7 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) {`
`197`	`197`	`}`
`198`	`198`	`defer os.Remove(tmpName)`
`199`	`199`
`200`		`- stdout, stderr, err := process.Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)`
	`200`	`+ stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName)`
`201`	`201`	`if err != nil {`
`202`	`202`	`return "", 0, fmt.Errorf("fail to parse public key: %s - %s", err, stderr)`
`203`	`203`	`}`
`@@ -382,7 +382,7 @@ func addKey(e Engine, key *PublicKey) (err error) {`
`382`	`382`	`if err = ioutil.WriteFile(tmpPath, []byte(key.Content), 0644); err != nil {`
`383`	`383`	`return err`
`384`	`384`	`}`
`385`		`- stdout, stderr, err := process.Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)`
	`385`	`+ stdout, stderr, err := process.GetManager().Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath)`
`386`	`386`	`if err != nil {`
`387`	`387`	`return fmt.Errorf("'ssh-keygen -lf %s' failed with error '%s': %s", tmpPath, err, stderr)`
`388`	`388`	`} else if len(stdout) < 2 {`