Graceful: Cancel Process on monitor pages & HammerTime

cmd/web.go

@@ -197,6 +197,7 @@ func runWeb(ctx *cli.Context) error {
 	log.Info("HTTP Listener: %s Closed", listenAddr)
 	graceful.Manager.WaitForServers()
 	graceful.Manager.WaitForTerminate()
+	log.Info("PID: %d Gitea Web Finished", os.Getpid())
 	log.Close()
 	return nil
 }

models/pull.go

@@ -18,7 +18,6 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/sync"
@@ -536,16 +535,13 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
 	headFile := pr.GetGitRefName()
 
 	// 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.GitExecutable, "merge-base", "--is-ancestor", headFile, pr.BaseBranch)
-
+	_, err := git.NewCommand("merge-base", "--is-ancestor", headFile, pr.BaseBranch).RunInDirWithEnv(pr.BaseRepo.RepoPath(), []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
 	if err != nil {
 		// Errors are signaled by a non-zero status that is not 1
 		if strings.Contains(err.Error(), "exit status 1") {
 			return nil, nil
 		}
-		return nil, fmt.Errorf("git merge-base --is-ancestor: %v %v", stderr, err)
+		return nil, fmt.Errorf("git merge-base --is-ancestor: %v", err)
 	}
 
 	commitIDBytes, err := ioutil.ReadFile(pr.BaseRepo.RepoPath() + "/" + headFile)
@@ -559,11 +555,9 @@ func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
 	cmd := commitID[:40] + ".." + pr.BaseBranch
 
 	// 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.GitExecutable, "rev-list", "--ancestry-path", "--merges", "--reverse", cmd)
+	mergeCommit, err := git.NewCommand("rev-list", "--ancestry-path", "--merges", "--reverse", cmd).RunInDirWithEnv("", []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
 	if err != nil {
-		return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v %v", stderr, err)
+		return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v", err)
 	} else if len(mergeCommit) < 40 {
 		// PR was fast-forwarded, so just use last commit of PR
 		mergeCommit = commitID[:40]
@@ -621,12 +615,9 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
 	indexTmpPath := filepath.Join(os.TempDir(), "gitea-"+pr.BaseRepo.Name+"-"+strconv.Itoa(time.Now().Nanosecond()))
 	defer os.Remove(indexTmpPath)
 
-	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.GitExecutable, "read-tree", pr.BaseBranch)
+	_, err = git.NewCommand("read-tree", pr.BaseBranch).RunInDirWithEnv("", []string{"GIT_DIR=" + pr.BaseRepo.RepoPath(), "GIT_INDEX_FILE=" + indexTmpPath})
 	if err != nil {
-		return fmt.Errorf("git read-tree --index-output=%s %s: %v - %s", indexTmpPath, pr.BaseBranch, err, stderr)
+		return fmt.Errorf("git read-tree --index-output=%s %s: %v", indexTmpPath, pr.BaseBranch, err)
 	}
 
 	prUnit, err := pr.BaseRepo.getUnit(e, UnitTypePullRequests)
@@ -642,9 +633,15 @@ func (pr *PullRequest) testPatch(e Engine) (err error) {
 	args = append(args, patchPath)
 	pr.ConflictedFiles = []string{}
 
-	_, stderr, err = process.GetManager().ExecDirEnv(-1, "", fmt.Sprintf("testPatch (git apply --check): %d", pr.BaseRepo.ID),
+	stderrBuilder := new(strings.Builder)
+	err = git.NewCommand(args...).RunInDirTimeoutEnvPipeline(
 		[]string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()},
-		git.GitExecutable, args...)
+		-1,
+		"",
+		nil,
+		stderrBuilder)
+	stderr := stderrBuilder.String()
+
 	if err != nil {
 		for i := range patchConflicts {
 			if strings.Contains(stderr, patchConflicts[i]) {

models/repo.go

@@ -30,7 +30,6 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/markup"
 	"code.gitea.io/gitea/modules/options"
-	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	api "code.gitea.io/gitea/modules/structs"
@@ -1202,11 +1201,11 @@ func initRepoCommit(tmpPath string, u *User) (err error) {
 		"GIT_COMMITTER_DATE="+commitTimeStr,
 	)
 
-	var stderr string
-	if _, stderr, err = process.GetManager().ExecDir(-1,
-		tmpPath, fmt.Sprintf("initRepoCommit (git add): %s", tmpPath),
-		git.GitExecutable, "add", "--all"); err != nil {
-		return fmt.Errorf("git add: %s", stderr)
+	if stdout, err := git.NewCommand("add", "--all").
+		SetDescription(fmt.Sprintf("initRepoCommit (git add): %s", tmpPath)).
+		RunInDir(tmpPath); err != nil {
+		log.Error("git add --all failed: Stdout: %s\nError: %v", stdout, err)
+		return fmt.Errorf("git add --all: %v", err)
 	}
 
 	binVersion, err := git.BinVersion()
@@ -1228,18 +1227,20 @@ func initRepoCommit(tmpPath string, u *User) (err error) {
 		}
 	}
 
-	if _, stderr, err = process.GetManager().ExecDirEnv(-1,
-		tmpPath, fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath),
-		env,
-		git.GitExecutable, args...); err != nil {
-		return fmt.Errorf("git commit: %s", stderr)
+	if stdout, err := git.NewCommand(args...).
+		SetDescription(fmt.Sprintf("initRepoCommit (git commit): %s", tmpPath)).
+		RunInDirWithEnv(tmpPath, env); err != nil {
+		log.Error("Failed to commit: %v: Stdout: %s\nError: %v", args, stdout, err)
+		return fmt.Errorf("git commit: %v", err)
 	}
 
-	if _, stderr, err = process.GetManager().ExecDir(-1,
-		tmpPath, fmt.Sprintf("initRepoCommit (git push): %s", tmpPath),
-		git.GitExecutable, "push", "origin", "master"); err != nil {
-		return fmt.Errorf("git push: %s", stderr)
+	if stdout, err := git.NewCommand("push", "origin", "master").
+		SetDescription(fmt.Sprintf("initRepoCommit (git push): %s", tmpPath)).
+		RunInDir(tmpPath); err != nil {
+		log.Error("Failed to push back to master: Stdout: %s\nError: %v", stdout, err)
+		return fmt.Errorf("git push: %v", err)
 	}
+
 	return nil
 }
 
@@ -1297,14 +1298,11 @@ func prepareRepoCommit(e Engine, repo *Repository, tmpDir, repoPath string, opts
 	)
 
 	// Clone to temporary path and do the init commit.
-	_, stderr, err := process.GetManager().ExecDirEnv(
-		-1, "",
-		fmt.Sprintf("initRepository(git clone): %s", repoPath),
-		env,
-		git.GitExecutable, "clone", repoPath, tmpDir,
-	)
-	if err != nil {
-		return fmt.Errorf("git clone: %v - %s", err, stderr)
+	if stdout, err := git.NewCommand("clone", repoPath, tmpDir).
+		SetDescription(fmt.Sprintf("initRepository (git clone): %s to %s", repoPath, tmpDir)).
+		RunInDirWithEnv("", env); err != nil {
+		log.Error("Failed to clone from %v into %s: stdout: %s\nError: %v", repo, tmpDir, stdout, err)
+		return fmt.Errorf("git clone: %v", err)
 	}
 
 	// README
@@ -1584,11 +1582,11 @@ 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.GitExecutable, "update-server-info")
-		if err != nil {
-			return nil, errors.New("CreateRepository(git update-server-info): " + stderr)
+		if stdout, err := git.NewCommand("update-server-info").
+			SetDescription(fmt.Sprintf("CreateRepository(git update-server-info): %s", repoPath)).
+			RunInDir(repoPath); err != nil {
+			log.Error("CreateRepitory(git update-server-info) in %v: Stdout: %s\nError: %v", repo, stdout, err)
+			return nil, fmt.Errorf("CreateRepository(git update-server-info): %v", err)
 		}
 	}
 
@@ -2422,12 +2420,13 @@ func GitGcRepos() error {
 				if err := repo.GetOwner(); err != nil {
 					return err
 				}
-				_, stderr, err := process.GetManager().ExecDir(
-					time.Duration(setting.Git.Timeout.GC)*time.Second,
-					RepoPath(repo.Owner.Name, repo.Name), "Repository garbage collection",
-					git.GitExecutable, args...)
-				if err != nil {
-					return fmt.Errorf("%v: %v", err, stderr)
+				if stdout, err := git.NewCommand(args...).
+					SetDescription(fmt.Sprintf("Repository Garbage Collection: %s", repo.FullName())).
+					RunInDirTimeout(
+						time.Duration(setting.Git.Timeout.GC)*time.Second,
+						RepoPath(repo.Owner.Name, repo.Name)); err != nil {
+					log.Error("Repository garbage collection failed for %v. Stdout: %s\nError: %v", repo, stdout, err)
+					return fmt.Errorf("Repository garbage collection failed: Error: %v", err)
 				}
 				return nil
 			})
@@ -2647,18 +2646,19 @@ func ForkRepository(doer, owner *User, oldRepo *Repository, name, desc string) (
 	}
 
 	repoPath := RepoPath(owner.Name, repo.Name)
-	_, stderr, err := process.GetManager().ExecTimeout(10*time.Minute,
-		fmt.Sprintf("ForkRepository(git clone): %s/%s", owner.Name, repo.Name),
-		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.GitExecutable, "update-server-info")
-	if err != nil {
-		return nil, fmt.Errorf("git update-server-info: %v", stderr)
+	if stdout, err := git.NewCommand(
+		"clone", "--bare", oldRepo.repoPath(sess), repoPath).
+		SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", oldRepo.FullName(), repo.FullName())).
+		RunInDirTimeout(10*time.Minute, ""); err != nil {
+		log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, oldRepo, stdout, err)
+		return nil, fmt.Errorf("git clone: %v", err)
+	}
+
+	if stdout, err := git.NewCommand("update-server-info").
+		SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())).
+		RunInDir(repoPath); err != nil {
+		log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err)
+		return nil, fmt.Errorf("git update-server-info: %v", err)
 	}
 
 	if err = createDelegateHooks(repoPath); err != nil {

models/repo_generate.go

@@ -16,7 +16,6 @@ import (
 
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
-	"code.gitea.io/gitea/modules/process"
 	"code.gitea.io/gitea/modules/util"
 
 	"github.com/gobwas/glob"
@@ -168,14 +167,11 @@ func generateRepoCommit(e Engine, repo, templateRepo, generateRepo *Repository,
 	}
 
 	repoPath := repo.repoPath(e)
-	_, stderr, err := process.GetManager().ExecDirEnv(
-		-1, tmpDir,
-		fmt.Sprintf("generateRepoCommit(git remote add): %s", repoPath),
-		env,
-		git.GitExecutable, "remote", "add", "origin", repoPath,
-	)
-	if err != nil {
-		return fmt.Errorf("git remote add: %v - %s", err, stderr)
+	if stdout, err := git.NewCommand("remote", "add", "origin", repoPath).
+		SetDescription(fmt.Sprintf("generateRepoCommit (git remote add): %s to %s", templateRepoPath, tmpDir)).
+		RunInDirWithEnv(tmpDir, env); err != nil {
+		log.Error("Unable to add %v as remote origin to temporary repo to %s: stdout %s\nError: %v", repo, tmpDir, stdout, err)
+		return fmt.Errorf("git remote add: %v", err)
 	}
 
 	return initRepoCommit(tmpDir, repo.Owner)

modules/git/blame.go

@@ -6,6 +6,7 @@ package git
 
 import (
 	"bufio"
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -28,6 +29,7 @@ type BlameReader struct {
 	output  io.ReadCloser
 	scanner *bufio.Scanner
 	lastSha *string
+	cancel  context.CancelFunc
 }
 
 var shaLineRegex = regexp.MustCompile("^([a-z0-9]{40})")
@@ -76,7 +78,8 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 
 // Close BlameReader - don't run NextPart after invoking that
 func (r *BlameReader) Close() error {
-	process.GetManager().Remove(r.pid)
+	defer process.GetManager().Remove(r.pid)
+	defer r.cancel()
 
 	if err := r.cmd.Wait(); err != nil {
 		return fmt.Errorf("Wait: %v", err)
@@ -97,20 +100,24 @@ func CreateBlameReader(repoPath, commitID, file string) (*BlameReader, error) {
 }
 
 func createBlameReader(dir string, command ...string) (*BlameReader, error) {
-	cmd := exec.Command(command[0], command[1:]...)
+	// FIXME: graceful: This should have a timeout
+	ctx, cancel := context.WithCancel(DefaultContext)
+	cmd := exec.CommandContext(ctx, command[0], command[1:]...)
 	cmd.Dir = dir
 	cmd.Stderr = os.Stderr
 
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
+		defer cancel()
 		return nil, fmt.Errorf("StdoutPipe: %v", err)
 	}
 
 	if err = cmd.Start(); err != nil {
+		defer cancel()
 		return nil, fmt.Errorf("Start: %v", err)
 	}
 
-	pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cmd)
+	pid := process.GetManager().Add(fmt.Sprintf("GetBlame [repo_path: %s]", dir), cancel)
 
 	scanner := bufio.NewScanner(stdout)
 
@@ -120,5 +127,6 @@ func createBlameReader(dir string, command ...string) (*BlameReader, error) {
 		stdout,
 		scanner,
 		nil,
+		cancel,
 	}, nil
 }

modules/git/command.go

@@ -30,8 +30,10 @@ const DefaultLocale = "C"
 
 // Command represents a command with its subcommands or arguments.
 type Command struct {
-	name string
-	args []string
+	name          string
+	args          []string
+	parentContext context.Context
+	desc          string
 }
 
 func (c *Command) String() string {
@@ -47,19 +49,34 @@ func NewCommand(args ...string) *Command {
 	cargs := make([]string, len(GlobalCommandArgs))
 	copy(cargs, GlobalCommandArgs)
 	return &Command{
-		name: GitExecutable,
-		args: append(cargs, args...),
+		name:          GitExecutable,
+		args:          append(cargs, args...),
+		parentContext: DefaultContext,
 	}
 }
 
 // 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,
+		name:          GitExecutable,
+		args:          args,
+		parentContext: DefaultContext,
 	}
 }
 
+// SetParentContext sets the parent context for this command
+func (c *Command) SetParentContext(ctx context.Context) *Command {
+	c.parentContext = ctx
+	return c
+}
+
+// SetDescription sets the description for this command which be returned on
+// c.String()
+func (c *Command) SetDescription(desc string) *Command {
+	c.desc = desc
+	return c
+}
+
 // AddArguments adds new argument(s) to the command.
 func (c *Command) AddArguments(args ...string) *Command {
 	c.args = append(c.args, args...)
@@ -92,7 +109,7 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.
 		log("%s: %v", dir, c)
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
+	ctx, cancel := context.WithTimeout(c.parentContext, timeout)
 	defer cancel()
 
 	cmd := exec.CommandContext(ctx, c.name, c.args...)
@@ -110,7 +127,11 @@ func (c *Command) RunInDirTimeoutEnvFullPipelineFunc(env []string, timeout time.
 		return err
 	}
 
-	pid := process.GetManager().Add(fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir), cmd)
+	desc := c.desc
+	if desc == "" {
+		desc = fmt.Sprintf("%s %s %s [repo_path: %s]", GitExecutable, c.name, strings.Join(c.args, " "), dir)
+	}
+	pid := process.GetManager().Add(desc, cancel)
 	defer process.GetManager().Remove(pid)
 
 	if fn != nil {