mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2024-12-20 20:07:03 +01:00
Prevent panic in doctor command when running default checks (#21791)
There was a bug introduced in #21352 due to a change of behaviour caused by #19280. This causes a panic on running the default doctor checks because the panic introduced by #19280 assumes that the only way opts.StdOut and opts.Stderr can be set in RunOpts is deliberately. Unfortunately, when running a git.Command the provided RunOpts can be set, therefore if you share a common set of RunOpts these two values can be set by the previous commands. This PR stops using common RunOpts for the commands in that doctor check but secondly stops RunCommand variants from changing the provided RunOpts. Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
parent
3e3975e0fa
commit
d9ba7f7442
2 changed files with 24 additions and 12 deletions
|
@ -19,11 +19,9 @@ func synchronizeRepoHeads(ctx context.Context, logger log.Logger, autofix bool)
|
|||
numReposUpdated := 0
|
||||
err := iterateRepositories(ctx, func(repo *repo_model.Repository) error {
|
||||
numRepos++
|
||||
runOpts := &git.RunOpts{Dir: repo.RepoPath()}
|
||||
_, _, defaultBranchErr := git.NewCommand(ctx, "rev-parse").AddDashesAndList(repo.DefaultBranch).RunStdString(&git.RunOpts{Dir: repo.RepoPath()})
|
||||
|
||||
_, _, defaultBranchErr := git.NewCommand(ctx, "rev-parse").AddDashesAndList(repo.DefaultBranch).RunStdString(runOpts)
|
||||
|
||||
head, _, headErr := git.NewCommand(ctx, "symbolic-ref", "--short", "HEAD").RunStdString(runOpts)
|
||||
head, _, headErr := git.NewCommand(ctx, "symbolic-ref", "--short", "HEAD").RunStdString(&git.RunOpts{Dir: repo.RepoPath()})
|
||||
|
||||
// what we expect: default branch is valid, and HEAD points to it
|
||||
if headErr == nil && defaultBranchErr == nil && head == repo.DefaultBranch {
|
||||
|
@ -49,7 +47,7 @@ func synchronizeRepoHeads(ctx context.Context, logger log.Logger, autofix bool)
|
|||
}
|
||||
|
||||
// otherwise, let's try fixing HEAD
|
||||
err := git.NewCommand(ctx, "symbolic-ref").AddDashesAndList("HEAD", git.BranchPrefix+repo.DefaultBranch).Run(runOpts)
|
||||
err := git.NewCommand(ctx, "symbolic-ref").AddDashesAndList("HEAD", git.BranchPrefix+repo.DefaultBranch).Run(&git.RunOpts{Dir: repo.RepoPath()})
|
||||
if err != nil {
|
||||
logger.Warn("Failed to fix HEAD for %s/%s: %v", repo.OwnerName, repo.Name, err)
|
||||
return nil
|
||||
|
@ -65,7 +63,7 @@ func synchronizeRepoHeads(ctx context.Context, logger log.Logger, autofix bool)
|
|||
logger.Info("Out of %d repos, HEADs for %d are now fixed and HEADS for %d are still broken", numRepos, numReposUpdated, numDefaultBranchesBroken+numHeadsBroken-numReposUpdated)
|
||||
} else {
|
||||
if numHeadsBroken == 0 && numDefaultBranchesBroken == 0 {
|
||||
logger.Info("All %d repos have their HEADs in the correct state")
|
||||
logger.Info("All %d repos have their HEADs in the correct state", numRepos)
|
||||
} else {
|
||||
if numHeadsBroken == 0 && numDefaultBranchesBroken != 0 {
|
||||
logger.Critical("Default branches are broken for %d/%d repos", numDefaultBranchesBroken, numRepos)
|
||||
|
|
|
@ -202,8 +202,11 @@ func (c *Command) Run(opts *RunOpts) error {
|
|||
if opts == nil {
|
||||
opts = &RunOpts{}
|
||||
}
|
||||
if opts.Timeout <= 0 {
|
||||
opts.Timeout = defaultCommandExecutionTimeout
|
||||
|
||||
// We must not change the provided options
|
||||
timeout := opts.Timeout
|
||||
if timeout <= 0 {
|
||||
timeout = defaultCommandExecutionTimeout
|
||||
}
|
||||
|
||||
if len(opts.Dir) == 0 {
|
||||
|
@ -238,7 +241,7 @@ func (c *Command) Run(opts *RunOpts) error {
|
|||
if opts.UseContextTimeout {
|
||||
ctx, cancel, finished = process.GetManager().AddContext(c.parentContext, desc)
|
||||
} else {
|
||||
ctx, cancel, finished = process.GetManager().AddContextTimeout(c.parentContext, opts.Timeout, desc)
|
||||
ctx, cancel, finished = process.GetManager().AddContextTimeout(c.parentContext, timeout, desc)
|
||||
}
|
||||
defer finished()
|
||||
|
||||
|
@ -339,9 +342,20 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS
|
|||
}
|
||||
stdoutBuf := &bytes.Buffer{}
|
||||
stderrBuf := &bytes.Buffer{}
|
||||
opts.Stdout = stdoutBuf
|
||||
opts.Stderr = stderrBuf
|
||||
err := c.Run(opts)
|
||||
|
||||
// We must not change the provided options as it could break future calls - therefore make a copy.
|
||||
newOpts := &RunOpts{
|
||||
Env: opts.Env,
|
||||
Timeout: opts.Timeout,
|
||||
UseContextTimeout: opts.UseContextTimeout,
|
||||
Dir: opts.Dir,
|
||||
Stdout: stdoutBuf,
|
||||
Stderr: stderrBuf,
|
||||
Stdin: opts.Stdin,
|
||||
PipelineFunc: opts.PipelineFunc,
|
||||
}
|
||||
|
||||
err := c.Run(newOpts)
|
||||
stderr = stderrBuf.Bytes()
|
||||
if err != nil {
|
||||
return nil, stderr, &runStdError{err: err, stderr: bytesToString(stderr)}
|
||||
|
|
Loading…
Reference in a new issue