Remove context from git struct (#33793)

Argument is moved from struct init in command run, which lets us remove
context from struct.

Author: TheFox0x7

cmd/hook.go

@@ -316,7 +316,7 @@ func runHookPostReceive(c *cli.Context) error {
 	setup(ctx, c.Bool("debug"))
 
 	// First of all run update-server-info no matter what
-	if _, _, err := git.NewCommand(ctx, "update-server-info").RunStdString(nil); err != nil {
+	if _, _, err := git.NewCommand("update-server-info").RunStdString(ctx, nil); err != nil {
 		return fmt.Errorf("Failed to call 'git update-server-info': %w", err)
 	}
 

models/migrations/v1_12/v128.go

@@ -82,17 +82,17 @@ func FixMergeBase(x *xorm.Engine) error {
 
 			if !pr.HasMerged {
 				var err error
-				pr.MergeBase, _, err = git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(pr.BaseBranch, gitRefName).RunStdString(&git.RunOpts{Dir: repoPath})
+				pr.MergeBase, _, err = git.NewCommand("merge-base").AddDashesAndList(pr.BaseBranch, gitRefName).RunStdString(git.DefaultContext, &git.RunOpts{Dir: repoPath})
 				if err != nil {
 					var err2 error
-					pr.MergeBase, _, err2 = git.NewCommand(git.DefaultContext, "rev-parse").AddDynamicArguments(git.BranchPrefix + pr.BaseBranch).RunStdString(&git.RunOpts{Dir: repoPath})
+					pr.MergeBase, _, err2 = git.NewCommand("rev-parse").AddDynamicArguments(git.BranchPrefix+pr.BaseBranch).RunStdString(git.DefaultContext, &git.RunOpts{Dir: 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(git.DefaultContext, "rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath})
+				parentsString, _, err := git.NewCommand("rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(git.DefaultContext, &git.RunOpts{Dir: 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
@@ -104,9 +104,9 @@ func FixMergeBase(x *xorm.Engine) error {
 
 				refs := append([]string{}, parents[1:]...)
 				refs = append(refs, gitRefName)
-				cmd := git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(refs...)
+				cmd := git.NewCommand("merge-base").AddDashesAndList(refs...)
 
-				pr.MergeBase, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath})
+				pr.MergeBase, _, err = cmd.RunStdString(git.DefaultContext, &git.RunOpts{Dir: 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

models/migrations/v1_12/v134.go

@@ -79,7 +79,7 @@ func RefixMergeBase(x *xorm.Engine) error {
 
 			gitRefName := fmt.Sprintf("refs/pull/%d/head", pr.Index)
 
-			parentsString, _, err := git.NewCommand(git.DefaultContext, "rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(&git.RunOpts{Dir: repoPath})
+			parentsString, _, err := git.NewCommand("rev-list", "--parents", "-n", "1").AddDynamicArguments(pr.MergedCommitID).RunStdString(git.DefaultContext, &git.RunOpts{Dir: 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
@@ -92,9 +92,9 @@ func RefixMergeBase(x *xorm.Engine) error {
 			// we should recalculate
 			refs := append([]string{}, parents[1:]...)
 			refs = append(refs, gitRefName)
-			cmd := git.NewCommand(git.DefaultContext, "merge-base").AddDashesAndList(refs...)
+			cmd := git.NewCommand("merge-base").AddDashesAndList(refs...)
 
-			pr.MergeBase, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath})
+			pr.MergeBase, _, err = cmd.RunStdString(git.DefaultContext, &git.RunOpts{Dir: 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

modules/git/batch_reader.go

@@ -29,8 +29,8 @@ 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 := NewCommand(ctx, "rev-parse").
-		Run(&RunOpts{
+	err := NewCommand("rev-parse").
+		Run(ctx, &RunOpts{
 			Dir:    repoPath,
 			Stderr: &stderr,
 		})
@@ -61,8 +61,8 @@ func catFileBatchCheck(ctx context.Context, repoPath string) (WriteCloserError,
 
 	go func() {
 		stderr := strings.Builder{}
-		err := NewCommand(ctx, "cat-file", "--batch-check").
-			Run(&RunOpts{
+		err := NewCommand("cat-file", "--batch-check").
+			Run(ctx, &RunOpts{
 				Dir:    repoPath,
 				Stdin:  batchStdinReader,
 				Stdout: batchStdoutWriter,
@@ -109,8 +109,8 @@ func catFileBatch(ctx context.Context, repoPath string) (WriteCloserError, *bufi
 
 	go func() {
 		stderr := strings.Builder{}
-		err := NewCommand(ctx, "cat-file", "--batch").
-			Run(&RunOpts{
+		err := NewCommand("cat-file", "--batch").
+			Run(ctx, &RunOpts{
 				Dir:    repoPath,
 				Stdin:  batchStdinReader,
 				Stdout: batchStdoutWriter,

modules/git/blame.go

@@ -135,7 +135,7 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath
 		ignoreRevsFile = tryCreateBlameIgnoreRevsFile(commit)
 	}
 
-	cmd := NewCommandContextNoGlobals(ctx, "blame", "--porcelain")
+	cmd := NewCommandNoGlobals("blame", "--porcelain")
 	if ignoreRevsFile != nil {
 		// Possible improvement: use --ignore-revs-file /dev/stdin on unix
 		// There is no equivalent on Windows. May be implemented if Gitea uses an external git backend.
@@ -155,7 +155,7 @@ func CreateBlameReader(ctx context.Context, objectFormat ObjectFormat, repoPath
 	go func() {
 		stderr := bytes.Buffer{}
 		// TODO: it doesn't work for directories (the directories shouldn't be "blamed"), and the "err" should be returned by "Read" but not by "Close"
-		err := cmd.Run(&RunOpts{
+		err := cmd.Run(ctx, &RunOpts{
 			UseContextTimeout: true,
 			Dir:               repoPath,
 			Stdout:            stdout,

modules/git/command.go

@@ -44,7 +44,6 @@ const DefaultLocale = "C"
 type Command struct {
 	prog             string
 	args             []string
-	parentContext    context.Context
 	globalArgsLength int
 	brokenArgs       []string
 }
@@ -82,7 +81,7 @@ func (c *Command) LogString() string {
 
 // NewCommand creates and returns a new Git Command based on given command and arguments.
 // Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead.
-func NewCommand(ctx context.Context, args ...internal.CmdArg) *Command {
+func NewCommand(args ...internal.CmdArg) *Command {
 	// Make an explicit copy of globalCommandArgs, otherwise append might overwrite it
 	cargs := make([]string, 0, len(globalCommandArgs)+len(args))
 	for _, arg := range globalCommandArgs {
@@ -94,31 +93,23 @@ func NewCommand(ctx context.Context, args ...internal.CmdArg) *Command {
 	return &Command{
 		prog:             GitExecutable,
 		args:             cargs,
-		parentContext:    ctx,
 		globalArgsLength: len(globalCommandArgs),
 	}
 }
 
-// NewCommandContextNoGlobals 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
+// NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specified args and don't use global command args
 // Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead.
-func NewCommandContextNoGlobals(ctx context.Context, args ...internal.CmdArg) *Command {
+func NewCommandNoGlobals(args ...internal.CmdArg) *Command {
 	cargs := make([]string, 0, len(args))
 	for _, arg := range args {
 		cargs = append(cargs, string(arg))
 	}
 	return &Command{
-		prog:          GitExecutable,
-		args:          cargs,
-		parentContext: ctx,
+		prog: GitExecutable,
+		args: cargs,
 	}
 }
 
-// SetParentContext sets the parent context for this command
-func (c *Command) SetParentContext(ctx context.Context) *Command {
-	c.parentContext = ctx
-	return c
-}
-
 // isSafeArgumentValue checks if the argument is safe to be used as a value (not an option)
 func isSafeArgumentValue(s string) bool {
 	return s == "" || s[0] != '-'
@@ -277,11 +268,11 @@ func CommonCmdServEnvs() []string {
 var ErrBrokenCommand = errors.New("git command is broken")
 
 // Run runs the command with the RunOpts
-func (c *Command) Run(opts *RunOpts) error {
-	return c.run(1, opts)
+func (c *Command) Run(ctx context.Context, opts *RunOpts) error {
+	return c.run(ctx, 1, opts)
 }
 
-func (c *Command) run(skip int, opts *RunOpts) error {
+func (c *Command) run(ctx context.Context, skip int, opts *RunOpts) error {
 	if len(c.brokenArgs) != 0 {
 		log.Error("git command is broken: %s, broken args: %s", c.LogString(), strings.Join(c.brokenArgs, " "))
 		return ErrBrokenCommand
@@ -305,19 +296,18 @@ func (c *Command) run(skip int, opts *RunOpts) error {
 	desc := fmt.Sprintf("git.Run(by:%s, repo:%s): %s", callerInfo, logArgSanitize(opts.Dir), cmdLogString)
 	log.Debug("git.Command: %s", desc)
 
-	_, span := gtprof.GetTracer().Start(c.parentContext, gtprof.TraceSpanGitRun)
+	_, span := gtprof.GetTracer().Start(ctx, gtprof.TraceSpanGitRun)
 	defer span.End()
 	span.SetAttributeString(gtprof.TraceAttrFuncCaller, callerInfo)
 	span.SetAttributeString(gtprof.TraceAttrGitCommand, cmdLogString)
 
-	var ctx context.Context
 	var cancel context.CancelFunc
 	var finished context.CancelFunc
 
 	if opts.UseContextTimeout {
-		ctx, cancel, finished = process.GetManager().AddContext(c.parentContext, desc)
+		ctx, cancel, finished = process.GetManager().AddContext(ctx, desc)
 	} else {
-		ctx, cancel, finished = process.GetManager().AddContextTimeout(c.parentContext, timeout, desc)
+		ctx, cancel, finished = process.GetManager().AddContextTimeout(ctx, timeout, desc)
 	}
 	defer finished()
 
@@ -410,8 +400,8 @@ func IsErrorExitCode(err error, code int) bool {
 }
 
 // RunStdString runs the command with options and returns stdout/stderr as string. and store stderr to returned error (err combined with stderr).
-func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr RunStdError) {
-	stdoutBytes, stderrBytes, err := c.runStdBytes(opts)
+func (c *Command) RunStdString(ctx context.Context, opts *RunOpts) (stdout, stderr string, runErr RunStdError) {
+	stdoutBytes, stderrBytes, err := c.runStdBytes(ctx, opts)
 	stdout = util.UnsafeBytesToString(stdoutBytes)
 	stderr = util.UnsafeBytesToString(stderrBytes)
 	if err != nil {
@@ -422,11 +412,11 @@ func (c *Command) RunStdString(opts *RunOpts) (stdout, stderr string, runErr Run
 }
 
 // RunStdBytes runs the command with options and returns stdout/stderr as bytes. and store stderr to returned error (err combined with stderr).
-func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
-	return c.runStdBytes(opts)
+func (c *Command) RunStdBytes(ctx context.Context, opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
+	return c.runStdBytes(ctx, opts)
 }
 
-func (c *Command) runStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
+func (c *Command) runStdBytes(ctx context.Context, opts *RunOpts) (stdout, stderr []byte, runErr RunStdError) {
 	if opts == nil {
 		opts = &RunOpts{}
 	}
@@ -449,7 +439,7 @@ func (c *Command) runStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS
 		PipelineFunc:      opts.PipelineFunc,
 	}
 
-	err := c.run(2, newOpts)
+	err := c.run(ctx, 2, newOpts)
 	stderr = stderrBuf.Bytes()
 	if err != nil {
 		return nil, stderr, &runStdError{err: err, stderr: util.UnsafeBytesToString(stderr)}

modules/git/command_race_test.go

@@ -15,9 +15,9 @@ func TestRunWithContextNoTimeout(t *testing.T) {
 	maxLoops := 10
 
 	// 'git --version' does not block so it must be finished before the timeout triggered.
-	cmd := NewCommand(t.Context(), "--version")
+	cmd := NewCommand("--version")
 	for i := 0; i < maxLoops; i++ {
-		if err := cmd.Run(&RunOpts{}); err != nil {
+		if err := cmd.Run(t.Context(), &RunOpts{}); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -27,9 +27,9 @@ func TestRunWithContextTimeout(t *testing.T) {
 	maxLoops := 10
 
 	// 'git hash-object --stdin' blocks on stdin so we can have the timeout triggered.
-	cmd := NewCommand(t.Context(), "hash-object", "--stdin")
+	cmd := NewCommand("hash-object", "--stdin")
 	for i := 0; i < maxLoops; i++ {
-		if err := cmd.Run(&RunOpts{Timeout: 1 * time.Millisecond}); err != nil {
+		if err := cmd.Run(t.Context(), &RunOpts{Timeout: 1 * time.Millisecond}); err != nil {
 			if err != context.DeadlineExceeded {
 				t.Fatalf("Testing %d/%d: %v", i, maxLoops, err)
 			}

modules/git/command_test.go

@@ -10,32 +10,32 @@ import (
 )
 
 func TestRunWithContextStd(t *testing.T) {
-	cmd := NewCommand(t.Context(), "--version")
-	stdout, stderr, err := cmd.RunStdString(&RunOpts{})
+	cmd := NewCommand("--version")
+	stdout, stderr, err := cmd.RunStdString(t.Context(), &RunOpts{})
 	assert.NoError(t, err)
 	assert.Empty(t, stderr)
 	assert.Contains(t, stdout, "git version")
 
-	cmd = NewCommand(t.Context(), "--no-such-arg")
-	stdout, stderr, err = cmd.RunStdString(&RunOpts{})
+	cmd = NewCommand("--no-such-arg")
+	stdout, stderr, err = cmd.RunStdString(t.Context(), &RunOpts{})
 	if assert.Error(t, err) {
 		assert.Equal(t, stderr, err.Stderr())
 		assert.Contains(t, err.Stderr(), "unknown option:")
 		assert.Contains(t, err.Error(), "exit status 129 - unknown option:")
 		assert.Empty(t, stdout)
 	}
 
-	cmd = NewCommand(t.Context())
+	cmd = NewCommand()
 	cmd.AddDynamicArguments("-test")
-	assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand)
+	assert.ErrorIs(t, cmd.Run(t.Context(), &RunOpts{}), ErrBrokenCommand)
 
-	cmd = NewCommand(t.Context())
+	cmd = NewCommand()
 	cmd.AddDynamicArguments("--test")
-	assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand)
+	assert.ErrorIs(t, cmd.Run(t.Context(), &RunOpts{}), ErrBrokenCommand)
 
 	subCmd := "version"
-	cmd = NewCommand(t.Context()).AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production
-	stdout, stderr, err = cmd.RunStdString(&RunOpts{})
+	cmd = NewCommand().AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production
+	stdout, stderr, err = cmd.RunStdString(t.Context(), &RunOpts{})
 	assert.NoError(t, err)
 	assert.Empty(t, stderr)
 	assert.Contains(t, stdout, "git version")
@@ -53,9 +53,9 @@ func TestGitArgument(t *testing.T) {
 }
 
 func TestCommandString(t *testing.T) {
-	cmd := NewCommandContextNoGlobals(t.Context(), "a", "-m msg", "it's a test", `say "hello"`)
+	cmd := NewCommandNoGlobals("a", "-m msg", "it's a test", `say "hello"`)
 	assert.EqualValues(t, cmd.prog+` a "-m msg" "it's a test" "say \"hello\""`, cmd.LogString())
 
-	cmd = NewCommandContextNoGlobals(t.Context(), "url: https://a:b@c/", "/root/dir-a/dir-b")
+	cmd = NewCommandNoGlobals("url: https://a:b@c/", "/root/dir-a/dir-b")
 	assert.EqualValues(t, cmd.prog+` "url: https://sanitized-credential@c/" .../dir-a/dir-b`, cmd.LogString())
 }