From 8598356df1eb21b6e33ecb9f9268ba36c5488e7c Mon Sep 17 00:00:00 2001 From: zeripath Date: Tue, 7 Mar 2023 20:07:35 +0000 Subject: [PATCH] Refactor and tidy-up the merge/update branch code (#22568) The merge and update branch code was previously a little tangled and had some very long functions. The functions were not very clear in their reasoning and there were deficiencies in their logging and at least one bug in the handling of LFS for update by rebase. This PR substantially refactors this code and splits things out to into separate functions. It also attempts to tidy up the calls by wrapping things in "context"s. There are also attempts to improve logging when there are errors. Signed-off-by: Andrew Thornton --------- Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick Co-authored-by: delvh --- modules/git/pipeline/revlist.go | 5 +- services/pull/lfs.go | 2 +- services/pull/merge.go | 495 +++++--------------------------- services/pull/merge_merge.go | 25 ++ services/pull/merge_prepare.go | 297 +++++++++++++++++++ services/pull/merge_rebase.go | 50 ++++ services/pull/merge_squash.go | 53 ++++ services/pull/patch.go | 17 +- services/pull/pull.go | 14 +- services/pull/temp_repo.go | 194 ++++++------- services/pull/update.go | 111 ++++--- services/pull/update_rebase.go | 107 +++++++ 12 files changed, 760 insertions(+), 610 deletions(-) create mode 100644 services/pull/merge_merge.go create mode 100644 services/pull/merge_prepare.go create mode 100644 services/pull/merge_rebase.go create mode 100644 services/pull/merge_squash.go create mode 100644 services/pull/update_rebase.go diff --git a/modules/git/pipeline/revlist.go b/modules/git/pipeline/revlist.go index 09bb2c8b3..d88ebe78e 100644 --- a/modules/git/pipeline/revlist.go +++ b/modules/git/pipeline/revlist.go @@ -42,7 +42,10 @@ func RevListObjects(ctx context.Context, revListWriter *io.PipeWriter, wg *sync. defer revListWriter.Close() stderr := new(bytes.Buffer) var errbuf strings.Builder - cmd := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(headSHA).AddArguments("--not").AddDynamicArguments(baseSHA) + cmd := git.NewCommand(ctx, "rev-list", "--objects").AddDynamicArguments(headSHA) + if baseSHA != "" { + cmd = cmd.AddArguments("--not").AddDynamicArguments(baseSHA) + } if err := cmd.Run(&git.RunOpts{ Dir: tmpBasePath, Stdout: revListWriter, diff --git a/services/pull/lfs.go b/services/pull/lfs.go index dc4ca006e..394337246 100644 --- a/services/pull/lfs.go +++ b/services/pull/lfs.go @@ -116,7 +116,7 @@ func createLFSMetaObjectsFromCatFileBatch(catFileBatchReader *io.PipeReader, wg } // Then we need to check that this pointer is in the db - if _, err := git_model.GetLFSMetaObjectByOid(db.DefaultContext, pr.HeadRepo.ID, pointer.Oid); err != nil { + if _, err := git_model.GetLFSMetaObjectByOid(db.DefaultContext, pr.HeadRepoID, pointer.Oid); err != nil { if err == git_model.ErrLFSObjectNotExist { log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) continue diff --git a/services/pull/merge.go b/services/pull/merge.go index ad428427c..afa924fc1 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -5,8 +5,6 @@ package pull import ( - "bufio" - "bytes" "context" "fmt" "os" @@ -14,7 +12,6 @@ import ( "regexp" "strconv" "strings" - "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" @@ -34,22 +31,17 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" ) // GetDefaultMergeMessage returns default message used when merging pull request func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issues_model.PullRequest, mergeStyle repo_model.MergeStyle) (message, body string, err error) { - if err := pr.LoadHeadRepo(ctx); err != nil { - return "", "", err - } if err := pr.LoadBaseRepo(ctx); err != nil { return "", "", err } - if pr.BaseRepo == nil { - return "", "", repo_model.ErrRepoNotExist{ID: pr.BaseRepoID} + if err := pr.LoadHeadRepo(ctx); err != nil { + return "", "", err } - if err := pr.LoadIssue(ctx); err != nil { return "", "", err } @@ -144,18 +136,19 @@ func expandDefaultMergeMessage(template string, vars map[string]string) (message // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { - if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - return fmt.Errorf("LoadHeadRepo: %w", err) - } else if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return fmt.Errorf("LoadBaseRepo: %w", err) + if err := pr.LoadBaseRepo(ctx); err != nil { + log.Error("Unable to load base repo: %v", err) + return fmt.Errorf("unable to load base repo: %w", err) + } else if err := pr.LoadHeadRepo(ctx); err != nil { + log.Error("Unable to load head repo: %v", err) + return fmt.Errorf("unable to load head repo: %w", err) } pullWorkingPool.CheckIn(fmt.Sprint(pr.ID)) defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) // Removing an auto merge pull and ignore if not exist + // FIXME: is this the correct point to do this? Shouldn't this be after IsMergeStyleAllowed? if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { return err } @@ -179,7 +172,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U // Run the merge in the hammer context to prevent cancellation hammerCtx := graceful.GetManager().HammerContext() - pr.MergedCommitID, err = rawMerge(hammerCtx, pr, doer, mergeStyle, expectedHeadCommitID, message) + pr.MergedCommitID, err = doMergeAndPush(hammerCtx, pr, doer, mergeStyle, expectedHeadCommitID, message) if err != nil { return err } @@ -189,18 +182,18 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U pr.MergerID = doer.ID if _, err := pr.SetMerged(hammerCtx); err != nil { - log.Error("SetMerged [%d]: %v", pr.ID, err) + log.Error("SetMerged %-v: %v", pr, err) } if err := pr.LoadIssue(hammerCtx); err != nil { - log.Error("LoadIssue [%d]: %v", pr.ID, err) + log.Error("LoadIssue %-v: %v", pr, err) } if err := pr.Issue.LoadRepo(hammerCtx); err != nil { - log.Error("LoadRepo for issue [%d]: %v", pr.ID, err) + log.Error("pr.Issue.LoadRepo %-v: %v", pr, err) } if err := pr.Issue.Repo.LoadOwner(hammerCtx); err != nil { - log.Error("LoadOwner for PR [%d]: %v", pr.ID, err) + log.Error("LoadOwner for %-v: %v", pr, err) } if wasAutoMerged { @@ -239,326 +232,43 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U return nil } -// rawMerge perform the merge operation without changing any pull information in database -func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) { +// doMergeAndPush performs the merge operation without changing any pull information in database and pushes it up to the base repository +func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) { // Clone base repo. - tmpBasePath, err := createTemporaryRepo(ctx, pr) + mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, expectedHeadCommitID) if err != nil { - log.Error("CreateTemporaryPath: %v", err) return "", err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() - - baseBranch := "base" - trackingBranch := "tracking" - stagingBranch := "staging" - - if expectedHeadCommitID != "" { - trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash").AddDynamicArguments(git.BranchPrefix + trackingBranch).RunStdString(&git.RunOpts{Dir: tmpBasePath}) - if err != nil { - log.Error("show-ref[%s] --hash refs/heads/trackingn: %v", tmpBasePath, git.BranchPrefix+trackingBranch, err) - return "", fmt.Errorf("getDiffTree: %w", err) - } - if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID { - return "", models.ErrSHADoesNotMatch{ - GivenSHA: expectedHeadCommitID, - CurrentSHA: trackingCommitID, - } - } - } - - var outbuf, errbuf strings.Builder - - // Enable sparse-checkout - sparseCheckoutList, err := getDiffTree(ctx, tmpBasePath, baseBranch, trackingBranch) - if err != nil { - log.Error("getDiffTree(%s, %s, %s): %v", tmpBasePath, baseBranch, trackingBranch, err) - return "", fmt.Errorf("getDiffTree: %w", err) - } - - infoPath := filepath.Join(tmpBasePath, ".git", "info") - if err := os.MkdirAll(infoPath, 0o700); err != nil { - log.Error("Unable to create .git/info in %s: %v", tmpBasePath, err) - return "", fmt.Errorf("Unable to create .git/info in tmpBasePath: %w", err) - } - - sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") - if err := os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0o600); err != nil { - log.Error("Unable to write .git/info/sparse-checkout file in %s: %v", tmpBasePath, err) - return "", fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) - } - - gitConfigCommand := func() *git.Command { - return git.NewCommand(ctx, "config", "--local") - } - - // Switch off LFS process (set required, clean and smudge here also) - if err := gitConfigCommand().AddArguments("filter.lfs.process", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.process -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.required", "false"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.required -> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.required -> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.clean", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.clean -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.smudge", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [core.sparseCheckout -> true ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [core.sparsecheckout -> true]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Read base branch index - if err := git.NewCommand(ctx, "read-tree", "HEAD"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git read-tree HEAD: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("Unable to read base branch in to the index: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - sig := doer.NewGitSig() - committer := sig - - // Determine if we should sign. If no signKeyID, use --no-gpg-sign to countermand the sign config (from gitconfig) - var signArgs git.TrustedCmdArgs - sign, signKeyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) - if sign { - if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - committer = signer - } - signArgs = git.ToTrustedCmdArgs([]string{"-S" + signKeyID}) - } else { - signArgs = append(signArgs, "--no-gpg-sign") - } - - commitTimeStr := time.Now().Format(time.RFC3339) - - // Because this may call hooks we should pass in the environment - env := append(os.Environ(), - "GIT_AUTHOR_NAME="+sig.Name, - "GIT_AUTHOR_EMAIL="+sig.Email, - "GIT_AUTHOR_DATE="+commitTimeStr, - "GIT_COMMITTER_NAME="+committer.Name, - "GIT_COMMITTER_EMAIL="+committer.Email, - "GIT_COMMITTER_DATE="+commitTimeStr, - ) + defer cancel() // Merge commits. switch mergeStyle { case repo_model.MergeStyleMerge: - cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge tracking into base: %v", err) + if err := doMergeStyleMerge(mergeCtx, message); err != nil { return "", err } - - if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { - log.Error("Unable to make final commit: %v", err) + case repo_model.MergeStyleRebase, repo_model.MergeStyleRebaseMerge: + if err := doMergeStyleRebase(mergeCtx, mergeStyle, message); err != nil { return "", err } - case repo_model.MergeStyleRebase: - fallthrough - case repo_model.MergeStyleRebaseUpdate: - fallthrough - case repo_model.MergeStyleRebaseMerge: - // Checkout head branch - if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Rebase before merging - if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - // Rebase will leave a REBASE_HEAD file in .git if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { - var commitSha string - ok := false - failingCommitPaths := []string{ - filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit"), // Git < 2.26 - filepath.Join(tmpBasePath, ".git", "rebase-merge", "stopped-sha"), // Git >= 2.26 - } - for _, failingCommitPath := range failingCommitPaths { - if _, statErr := os.Stat(failingCommitPath); statErr == nil { - commitShaBytes, readErr := os.ReadFile(failingCommitPath) - if readErr != nil { - // Abandon this attempt to handle the error - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - commitSha = strings.TrimSpace(string(commitShaBytes)) - ok = true - break - } - } - if !ok { - log.Error("Unable to determine failing commit sha for this rebase message. Cannot cast as models.ErrRebaseConflicts.") - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - log.Debug("RebaseConflict at %s [%s:%s -> %s:%s]: %v\n%s\n%s", commitSha, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", models.ErrRebaseConflicts{ - Style: mergeStyle, - CommitSHA: commitSha, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // not need merge, just update by rebase. so skip - if mergeStyle == repo_model.MergeStyleRebaseUpdate { - break - } - - // Checkout base branch again - if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - cmd := git.NewCommand(ctx, "merge") - if mergeStyle == repo_model.MergeStyleRebase { - cmd.AddArguments("--ff-only") - } else { - cmd.AddArguments("--no-ff", "--no-commit") - } - cmd.AddDynamicArguments(stagingBranch) - - // Prepare merge with commit - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge staging into base: %v", err) - return "", err - } - if mergeStyle == repo_model.MergeStyleRebaseMerge { - if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { - log.Error("Unable to make final commit: %v", err) - return "", err - } - } case repo_model.MergeStyleSquash: - // Merge with squash - cmd := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge --squash tracking into base: %v", err) + if err := doMergeStyleSquash(mergeCtx, message); err != nil { return "", err } - - if err = pr.Issue.LoadPoster(ctx); err != nil { - log.Error("LoadPoster: %v", err) - return "", fmt.Errorf("LoadPoster: %w", err) - } - sig := pr.Issue.Poster.NewGitSig() - if setting.Repository.PullRequest.AddCoCommitterTrailers && committer.String() != sig.String() { - // add trailer - message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) - } - if err := git.NewCommand(ctx, "commit"). - AddArguments(signArgs...). - AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). - AddOptionFormat("--message=%s", message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() default: return "", models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} } // OK we should cache our current head and origin/headbranch - mergeHeadSHA, err := git.GetFullCommitID(ctx, tmpBasePath, "HEAD") + mergeHeadSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "HEAD") if err != nil { return "", fmt.Errorf("Failed to get full commit id for HEAD: %w", err) } - mergeBaseSHA, err := git.GetFullCommitID(ctx, tmpBasePath, "original_"+baseBranch) + mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "original_"+baseBranch) if err != nil { return "", fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err) } - mergeCommitID, err := git.GetFullCommitID(ctx, tmpBasePath, baseBranch) + mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, baseBranch) if err != nil { return "", fmt.Errorf("Failed to get full commit id for the new merge: %w", err) } @@ -567,7 +277,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode // I think in the interests of data safety - failures to push to the lfs should prevent // the merge as you can always remerge. if setting.LFS.StartServer { - if err := LFSPush(ctx, tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil { + if err := LFSPush(ctx, mergeCtx.tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil { return "", err } } @@ -576,167 +286,92 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode err = pr.HeadRepo.LoadOwner(ctx) if err != nil { if !user_model.IsErrUserNotExist(err) { - log.Error("Can't find user: %d for head repository - %v", pr.HeadRepo.OwnerID, err) + log.Error("Can't find user: %d for head repository in %-v: %v", pr.HeadRepo.OwnerID, pr, err) return "", err } - log.Error("Can't find user: %d for head repository - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, doer.Name, err) + log.Warn("Can't find user: %d for head repository in %-v - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, pr, doer.Name, err) headUser = doer } else { headUser = pr.HeadRepo.Owner } - var pushCmd *git.Command - if mergeStyle == repo_model.MergeStyleRebaseUpdate { - // force push the rebase result to head branch - env = repo_module.FullPushingEnvironment( - headUser, - doer, - pr.HeadRepo, - pr.HeadRepo.Name, - pr.ID, - ) - pushCmd = git.NewCommand(ctx, "push", "-f", "head_repo").AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch) - } else { - env = repo_module.FullPushingEnvironment( - headUser, - doer, - pr.BaseRepo, - pr.BaseRepo.Name, - pr.ID, - ) - pushCmd = git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) - } + mergeCtx.env = repo_module.FullPushingEnvironment( + headUser, + doer, + pr.BaseRepo, + pr.BaseRepo.Name, + pr.ID, + ) + pushCmd := git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) // Push back to upstream. // TODO: this cause an api call to "/api/internal/hook/post-receive/...", // that prevents us from doint the whole merge in one db transaction - if err := pushCmd.Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - if strings.Contains(errbuf.String(), "non-fast-forward") { + if err := pushCmd.Run(mergeCtx.RunOpts()); err != nil { + if strings.Contains(mergeCtx.errbuf.String(), "non-fast-forward") { return "", &git.ErrPushOutOfDate{ - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), Err: err, } - } else if strings.Contains(errbuf.String(), "! [remote rejected]") { + } else if strings.Contains(mergeCtx.errbuf.String(), "! [remote rejected]") { err := &git.ErrPushRejected{ - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), Err: err, } err.GenerateMessage() return "", err } - return "", fmt.Errorf("git push: %s", errbuf.String()) + return "", fmt.Errorf("git push: %s", mergeCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() return mergeCommitID, nil } -func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message string, signArgs git.TrustedCmdArgs, tmpBasePath string, env []string) error { - var outbuf, errbuf strings.Builder - if err := git.NewCommand(ctx, "commit").AddArguments(signArgs...).AddOptionFormat("--message=%s", message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) +func commitAndSignNoAuthor(ctx *mergeContext, message string) error { + if err := git.NewCommand(ctx, "commit").AddArguments(ctx.signArg...).AddOptionFormat("--message=%s", message). + Run(ctx.RunOpts()); err != nil { + log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git commit %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) } return nil } -func runMergeCommand(pr *issues_model.PullRequest, mergeStyle repo_model.MergeStyle, cmd *git.Command, tmpBasePath string) error { - var outbuf, errbuf strings.Builder - if err := cmd.Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { +func runMergeCommand(ctx *mergeContext, mergeStyle repo_model.MergeStyle, cmd *git.Command) error { + if err := cmd.Run(ctx.RunOpts()); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + log.Debug("MergeConflict %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), Err: err, } - } else if strings.Contains(errbuf.String(), "refusing to merge unrelated histories") { - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } else if strings.Contains(ctx.errbuf.String(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) return models.ErrMergeUnrelatedHistories{ Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), Err: err, } } - log.Error("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + log.Error("git merge %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git merge %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) } + ctx.outbuf.Reset() + ctx.errbuf.Reset() return nil } var escapedSymbols = regexp.MustCompile(`([*[?! \\])`) -func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (string, error) { - getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { - var outbuf, errbuf strings.Builder - // Compute the diff-tree for sparse-checkout - if err := git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). - Run(&git.RunOpts{ - Dir: repoPath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, errbuf.String()) - } - return outbuf.String(), nil - } - - scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { - if atEOF && len(data) == 0 { - return 0, nil, nil - } - if i := bytes.IndexByte(data, '\x00'); i >= 0 { - return i + 1, data[0:i], nil - } - if atEOF { - return len(data), data, nil - } - return 0, nil, nil - } - - list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) - if err != nil { - return "", err - } - - // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. - out := bytes.Buffer{} - scanner := bufio.NewScanner(strings.NewReader(list)) - scanner.Split(scanNullTerminatedStrings) - for scanner.Scan() { - filepath := scanner.Text() - // escape '*', '?', '[', spaces and '!' prefix - filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) - // no necessary to escape the first '#' symbol because the first symbol is '/' - fmt.Fprintf(&out, "/%s\n", filepath) - } - - return out.String(), nil -} - // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p access_model.Permission, user *user_model.User) (bool, error) { if user == nil { diff --git a/services/pull/merge_merge.go b/services/pull/merge_merge.go new file mode 100644 index 000000000..0f7664297 --- /dev/null +++ b/services/pull/merge_merge.go @@ -0,0 +1,25 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" +) + +// doMergeStyleMerge merges the tracking into the current HEAD - which is assumed to tbe staging branch (equal to the pr.BaseBranch) +func doMergeStyleMerge(ctx *mergeContext, message string) error { + cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) + if err := runMergeCommand(ctx, repo_model.MergeStyleMerge, cmd); err != nil { + log.Error("%-v Unable to merge tracking into base: %v", ctx.pr, err) + return err + } + + if err := commitAndSignNoAuthor(ctx, message); err != nil { + log.Error("%-v Unable to make final commit: %v", ctx.pr, err) + return err + } + return nil +} diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go new file mode 100644 index 000000000..2ba821961 --- /dev/null +++ b/services/pull/merge_prepare.go @@ -0,0 +1,297 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "bufio" + "bytes" + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "code.gitea.io/gitea/models" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + asymkey_service "code.gitea.io/gitea/services/asymkey" +) + +type mergeContext struct { + *prContext + doer *user_model.User + sig *git.Signature + committer *git.Signature + signArg git.TrustedCmdArgs + env []string +} + +func (ctx *mergeContext) RunOpts() *git.RunOpts { + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return &git.RunOpts{ + Env: ctx.env, + Dir: ctx.tmpBasePath, + Stdout: ctx.outbuf, + Stderr: ctx.errbuf, + } +} + +func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, expectedHeadCommitID string) (mergeCtx *mergeContext, cancel context.CancelFunc, err error) { + // Clone base repo. + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) + if err != nil { + log.Error("createTemporaryRepoForPR: %v", err) + return nil, cancel, err + } + + mergeCtx = &mergeContext{ + prContext: prCtx, + doer: doer, + } + + if expectedHeadCommitID != "" { + trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash").AddDynamicArguments(git.BranchPrefix + trackingBranch).RunStdString(&git.RunOpts{Dir: mergeCtx.tmpBasePath}) + if err != nil { + defer cancel() + log.Error("failed to get sha of head branch in %-v: show-ref[%s] --hash refs/heads/tracking: %v", mergeCtx.pr, mergeCtx.tmpBasePath, err) + return nil, nil, fmt.Errorf("unable to get sha of head branch in %v %w", pr, err) + } + if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID { + defer cancel() + return nil, nil, models.ErrSHADoesNotMatch{ + GivenSHA: expectedHeadCommitID, + CurrentSHA: trackingCommitID, + } + } + } + + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() + if err := prepareTemporaryRepoForMerge(mergeCtx); err != nil { + defer cancel() + return nil, nil, err + } + + mergeCtx.sig = doer.NewGitSig() + mergeCtx.committer = mergeCtx.sig + + // Determine if we should sign + sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, mergeCtx.pr, mergeCtx.doer, mergeCtx.tmpBasePath, "HEAD", trackingBranch) + if sign { + mergeCtx.signArg = git.ToTrustedCmdArgs([]string{"-S" + keyID}) + if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + mergeCtx.committer = signer + } + } else { + mergeCtx.signArg = git.ToTrustedCmdArgs([]string{"--no-gpg-sign"}) + } + + commitTimeStr := time.Now().Format(time.RFC3339) + + // Because this may call hooks we should pass in the environment + mergeCtx.env = append(os.Environ(), + "GIT_AUTHOR_NAME="+mergeCtx.sig.Name, + "GIT_AUTHOR_EMAIL="+mergeCtx.sig.Email, + "GIT_AUTHOR_DATE="+commitTimeStr, + "GIT_COMMITTER_NAME="+mergeCtx.committer.Name, + "GIT_COMMITTER_EMAIL="+mergeCtx.committer.Email, + "GIT_COMMITTER_DATE="+commitTimeStr, + ) + + return mergeCtx, cancel, nil +} + +// prepareTemporaryRepoForMerge takes a repository that has been created using createTemporaryRepo +// it then sets up the sparse-checkout and other things +func prepareTemporaryRepoForMerge(ctx *mergeContext) error { + infoPath := filepath.Join(ctx.tmpBasePath, ".git", "info") + if err := os.MkdirAll(infoPath, 0o700); err != nil { + log.Error("%-v Unable to create .git/info in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to create .git/info in tmpBasePath: %w", err) + } + + // Enable sparse-checkout + // Here we use the .git/info/sparse-checkout file as described in the git documentation + sparseCheckoutListFile, err := os.OpenFile(filepath.Join(infoPath, "sparse-checkout"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) + if err != nil { + log.Error("%-v Unable to write .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) + } + defer sparseCheckoutListFile.Close() // we will close it earlier but we need to ensure it is closed if there is an error + + if err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, sparseCheckoutListFile); err != nil { + log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, baseBranch, trackingBranch, err) + return fmt.Errorf("getDiffTree: %w", err) + } + + if err := sparseCheckoutListFile.Close(); err != nil { + log.Error("%-v Unable to close .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to close .git/info/sparse-checkout file in tmpBasePath: %w", err) + } + + gitConfigCommand := func() *git.Command { + return git.NewCommand(ctx, "config", "--local") + } + + setConfig := func(key, value string) error { + if err := gitConfigCommand().AddArguments(git.ToTrustedCmdArgs([]string{key, value})...). + Run(ctx.RunOpts()); err != nil { + if value == "" { + value = "<>" + } + log.Error("git config [%s -> %s ]: %v\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git config [%s -> %s ]: %w\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + return nil + } + + // Switch off LFS process (set required, clean and smudge here also) + if err := setConfig("filter.lfs.process", ""); err != nil { + return err + } + + if err := setConfig("filter.lfs.required", "false"); err != nil { + return err + } + + if err := setConfig("filter.lfs.clean", ""); err != nil { + return err + } + + if err := setConfig("filter.lfs.smudge", ""); err != nil { + return err + } + + if err := setConfig("core.sparseCheckout", "true"); err != nil { + return err + } + + // Read base branch index + if err := git.NewCommand(ctx, "read-tree", "HEAD"). + Run(ctx.RunOpts()); err != nil { + log.Error("git read-tree HEAD: %v\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("Unable to read base branch in to the index: %w\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + return nil +} + +// getDiffTree returns a string containing all the files that were changed between headBranch and baseBranch +// the filenames are escaped so as to fit the format required for .git/info/sparse-checkout +func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out io.Writer) error { + diffOutReader, diffOutWriter, err := os.Pipe() + if err != nil { + log.Error("Unable to create os.Pipe for %s", repoPath) + return err + } + defer func() { + _ = diffOutReader.Close() + _ = diffOutWriter.Close() + }() + + scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\x00'); i >= 0 { + return i + 1, data[0:i], nil + } + if atEOF { + return len(data), data, nil + } + return 0, nil, nil + } + + err = git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). + Run(&git.RunOpts{ + Dir: repoPath, + Stdout: diffOutWriter, + PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { + // Close the writer end of the pipe to begin processing + _ = diffOutWriter.Close() + defer func() { + // Close the reader on return to terminate the git command if necessary + _ = diffOutReader.Close() + }() + + // Now scan the output from the command + scanner := bufio.NewScanner(diffOutReader) + scanner.Split(scanNullTerminatedStrings) + for scanner.Scan() { + filepath := scanner.Text() + // escape '*', '?', '[', spaces and '!' prefix + filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) + // no necessary to escape the first '#' symbol because the first symbol is '/' + fmt.Fprintf(out, "/%s\n", filepath) + } + return scanner.Err() + }, + }) + return err +} + +// rebaseTrackingOnToBase checks out the tracking branch as staging and rebases it on to the base branch +// if there is a conflict it will return a models.ErrRebaseConflicts +func rebaseTrackingOnToBase(ctx *mergeContext, mergeStyle repo_model.MergeStyle) error { + // Checkout head branch + if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). + Run(ctx.RunOpts()); err != nil { + return fmt.Errorf("unable to git checkout tracking as staging in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + // Rebase before merging + if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). + Run(ctx.RunOpts()); err != nil { + // Rebase will leave a REBASE_HEAD file in .git if there is a conflict + if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { + var commitSha string + ok := false + failingCommitPaths := []string{ + filepath.Join(ctx.tmpBasePath, ".git", "rebase-apply", "original-commit"), // Git < 2.26 + filepath.Join(ctx.tmpBasePath, ".git", "rebase-merge", "stopped-sha"), // Git >= 2.26 + } + for _, failingCommitPath := range failingCommitPaths { + if _, statErr := os.Stat(failingCommitPath); statErr == nil { + commitShaBytes, readErr := os.ReadFile(failingCommitPath) + if readErr != nil { + // Abandon this attempt to handle the error + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + commitSha = strings.TrimSpace(string(commitShaBytes)) + ok = true + break + } + } + if !ok { + log.Error("Unable to determine failing commit sha for failing rebase in temp repo for %-v. Cannot cast as models.ErrRebaseConflicts.", ctx.pr) + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + log.Debug("Conflict when rebasing staging on to base in %-v at %s: %v\n%s\n%s", ctx.pr, commitSha, err, ctx.outbuf.String(), ctx.errbuf.String()) + return models.ErrRebaseConflicts{ + CommitSHA: commitSha, + Style: mergeStyle, + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), + Err: err, + } + } + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return nil +} diff --git a/services/pull/merge_rebase.go b/services/pull/merge_rebase.go new file mode 100644 index 000000000..d3bb86d4a --- /dev/null +++ b/services/pull/merge_rebase.go @@ -0,0 +1,50 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "fmt" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" +) + +// doMergeStyleRebase rebaases the tracking branch on the base branch as the current HEAD with or with a merge commit to the original pr branch +func doMergeStyleRebase(ctx *mergeContext, mergeStyle repo_model.MergeStyle, message string) error { + if err := rebaseTrackingOnToBase(ctx, mergeStyle); err != nil { + return err + } + + // Checkout base branch again + if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). + Run(ctx.RunOpts()); err != nil { + log.Error("git checkout base prior to merge post staging rebase %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git checkout base prior to merge post staging rebase %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + cmd := git.NewCommand(ctx, "merge") + if mergeStyle == repo_model.MergeStyleRebase { + cmd.AddArguments("--ff-only") + } else { + cmd.AddArguments("--no-ff", "--no-commit") + } + cmd.AddDynamicArguments(stagingBranch) + + // Prepare merge with commit + if err := runMergeCommand(ctx, mergeStyle, cmd); err != nil { + log.Error("Unable to merge staging into base: %v", err) + return err + } + if mergeStyle == repo_model.MergeStyleRebaseMerge { + if err := commitAndSignNoAuthor(ctx, message); err != nil { + log.Error("Unable to make final commit: %v", err) + return err + } + } + + return nil +} diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go new file mode 100644 index 000000000..0a8cc0167 --- /dev/null +++ b/services/pull/merge_squash.go @@ -0,0 +1,53 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "fmt" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +// doMergeStyleSquash squashes the tracking branch on the current HEAD (=base) +func doMergeStyleSquash(ctx *mergeContext, message string) error { + cmd := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) + if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmd); err != nil { + log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err) + return err + } + + if err := ctx.pr.Issue.LoadPoster(ctx); err != nil { + log.Error("%-v Issue[%d].LoadPoster: %v", ctx.pr, ctx.pr.Issue.ID, err) + return fmt.Errorf("LoadPoster: %w", err) + } + sig := ctx.pr.Issue.Poster.NewGitSig() + if len(ctx.signArg) == 0 { + if err := git.NewCommand(ctx, "commit"). + AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). + AddOptionFormat("--message=%s", message). + Run(ctx.RunOpts()); err != nil { + log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", ctx.pr.HeadRepo.FullName(), ctx.pr.HeadBranch, ctx.pr.BaseRepo.FullName(), ctx.pr.BaseBranch, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + } else { + if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { + // add trailer + message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) + } + if err := git.NewCommand(ctx, "commit"). + AddArguments(ctx.signArg...). + AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). + AddOptionFormat("--message=%s", message). + Run(ctx.RunOpts()); err != nil { + log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", ctx.pr.HeadRepo.FullName(), ctx.pr.HeadBranch, ctx.pr.BaseRepo.FullName(), ctx.pr.BaseBranch, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return nil +} diff --git a/services/pull/patch.go b/services/pull/patch.go index c2ccc75bd..927735572 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -22,7 +22,6 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" - repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -64,25 +63,21 @@ func TestPatch(pr *issues_model.PullRequest) error { defer finished() // Clone base repo. - tmpBasePath, err := createTemporaryRepo(ctx, pr) + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { - log.Error("CreateTemporaryPath: %v", err) + log.Error("createTemporaryRepoForPR %-v: %v", pr, err) return err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - gitRepo, err := git.OpenRepository(ctx, tmpBasePath) + gitRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) if err != nil { return fmt.Errorf("OpenRepository: %w", err) } defer gitRepo.Close() // 1. update merge base - pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base", "--", "base", "tracking").RunStdString(&git.RunOpts{Dir: tmpBasePath}) + pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base", "--", "base", "tracking").RunStdString(&git.RunOpts{Dir: prCtx.tmpBasePath}) if err != nil { var err2 error pr.MergeBase, err2 = gitRepo.GetRefCommitID(git.BranchPrefix + "base") @@ -101,7 +96,7 @@ func TestPatch(pr *issues_model.PullRequest) error { } // 2. Check for conflicts - if conflicts, err := checkConflicts(ctx, pr, gitRepo, tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty { + if conflicts, err := checkConflicts(ctx, pr, gitRepo, prCtx.tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty { return err } diff --git a/services/pull/pull.go b/services/pull/pull.go index a19e88b33..d8923d0d5 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -349,18 +349,14 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, // checkIfPRContentChanged checks if diff to target branch has changed by push // A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) { - tmpBasePath, err := createTemporaryRepo(ctx, pr) + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { - log.Error("CreateTemporaryRepo: %v", err) + log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) return false, err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("checkIfPRContentChanged: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - tmpRepo, err := git.OpenRepository(ctx, tmpBasePath) + tmpRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) if err != nil { return false, fmt.Errorf("OpenRepository: %w", err) } @@ -379,7 +375,7 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, } if err := cmd.Run(&git.RunOpts{ - Dir: tmpBasePath, + Dir: prCtx.tmpBasePath, Stdout: stdoutWriter, PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index 2bef67155..146470780 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -19,49 +19,85 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" ) -// createTemporaryRepo creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch +// Temporary repos created here use standard branch names to help simplify +// merging code +const ( + baseBranch = "base" // equivalent to pr.BaseBranch + trackingBranch = "tracking" // equivalent to pr.HeadBranch + stagingBranch = "staging" // this is used for a working branch +) + +type prContext struct { + context.Context + tmpBasePath string + pr *issues_model.PullRequest + outbuf *strings.Builder // we keep these around to help reduce needless buffer recreation, + errbuf *strings.Builder // any use should be preceded by a Reset and preferably after use +} + +func (ctx *prContext) RunOpts() *git.RunOpts { + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return &git.RunOpts{ + Dir: ctx.tmpBasePath, + Stdout: ctx.outbuf, + Stderr: ctx.errbuf, + } +} + +// createTemporaryRepoForPR creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch // it also create a second base branch called "original_base" -func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (string, error) { +func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prContext, cancel context.CancelFunc, err error) { if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - return "", fmt.Errorf("LoadHeadRepo: %w", err) + log.Error("%-v LoadHeadRepo: %v", pr, err) + return nil, nil, fmt.Errorf("%v LoadHeadRepo: %w", pr, err) } else if pr.HeadRepo == nil { - log.Error("Pr %d HeadRepo %d does not exist", pr.ID, pr.HeadRepoID) - return "", &repo_model.ErrRepoNotExist{ + log.Error("%-v HeadRepo %d does not exist", pr, pr.HeadRepoID) + return nil, nil, &repo_model.ErrRepoNotExist{ ID: pr.HeadRepoID, } } else if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return "", fmt.Errorf("LoadBaseRepo: %w", err) + log.Error("%-v LoadBaseRepo: %v", pr, err) + return nil, nil, fmt.Errorf("%v LoadBaseRepo: %w", pr, err) } else if pr.BaseRepo == nil { - log.Error("Pr %d BaseRepo %d does not exist", pr.ID, pr.BaseRepoID) - return "", &repo_model.ErrRepoNotExist{ + log.Error("%-v BaseRepo %d does not exist", pr, pr.BaseRepoID) + return nil, nil, &repo_model.ErrRepoNotExist{ ID: pr.BaseRepoID, } } else if err := pr.HeadRepo.LoadOwner(ctx); err != nil { - log.Error("HeadRepo.LoadOwner: %v", err) - return "", fmt.Errorf("HeadRepo.LoadOwner: %w", err) + log.Error("%-v HeadRepo.LoadOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v HeadRepo.LoadOwner: %w", pr, err) } else if err := pr.BaseRepo.LoadOwner(ctx); err != nil { - log.Error("BaseRepo.LoadOwner: %v", err) - return "", fmt.Errorf("BaseRepo.LoadOwner: %w", err) + log.Error("%-v BaseRepo.LoadOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v BaseRepo.LoadOwner: %w", pr, err) } // Clone base repo. tmpBasePath, err := repo_module.CreateTemporaryPath("pull") if err != nil { - log.Error("CreateTemporaryPath: %v", err) - return "", err + log.Error("CreateTemporaryPath[%-v]: %v", pr, err) + return nil, nil, err + } + prCtx = &prContext{ + Context: ctx, + tmpBasePath: tmpBasePath, + pr: pr, + outbuf: &strings.Builder{}, + errbuf: &strings.Builder{}, + } + cancel = func() { + if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { + log.Error("Error whilst removing removing temporary repo for %-v: %v", pr, err) + } } baseRepoPath := pr.BaseRepo.RepoPath() headRepoPath := pr.HeadRepo.RepoPath() if err := git.InitRepository(ctx, tmpBasePath, false); err != nil { - log.Error("git init tmpBasePath: %v", err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", err + log.Error("Unable to init tmpBasePath for %-v: %v", pr, err) + cancel() + return nil, nil, err } remoteRepoName := "head_repo" @@ -73,99 +109,63 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str fetchArgs = append(fetchArgs, "--no-write-commit-graph") } - // Add head repo remote. - addCacheRepo := func(staging, cache string) error { - p := filepath.Join(staging, ".git", "objects", "info", "alternates") + // addCacheRepo adds git alternatives for the cacheRepoPath in the repoPath + addCacheRepo := func(repoPath, cacheRepoPath string) error { + p := filepath.Join(repoPath, ".git", "objects", "info", "alternates") f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) if err != nil { - log.Error("Could not create .git/objects/info/alternates file in %s: %v", staging, err) + log.Error("Could not create .git/objects/info/alternates file in %s: %v", repoPath, err) return err } defer f.Close() - data := filepath.Join(cache, "objects") + data := filepath.Join(cacheRepoPath, "objects") if _, err := fmt.Fprintln(f, data); err != nil { - log.Error("Could not write to .git/objects/info/alternates file in %s: %v", staging, err) + log.Error("Could not write to .git/objects/info/alternates file in %s: %v", repoPath, err) return err } return nil } + // Add head repo remote. if err := addCacheRepo(tmpBasePath, baseRepoPath); err != nil { - log.Error("Unable to add base repository to temporary repo [%s -> %s]: %v", pr.BaseRepo.FullName(), tmpBasePath, err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err) + log.Error("%-v Unable to add base repository to temporary repo [%s -> %s]: %v", pr, pr.BaseRepo.FullName(), tmpBasePath, err) + cancel() + return nil, nil, fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err) } - var outbuf, errbuf strings.Builder if err := git.NewCommand(ctx, "remote", "add", "-t").AddDynamicArguments(pr.BaseBranch).AddArguments("-m").AddDynamicArguments(pr.BaseBranch).AddDynamicArguments("origin", baseRepoPath). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr.BaseRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add base repository as origin [%s -> tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr, pr.BaseRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to add base repository as origin [%s -> tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() if err := git.NewCommand(ctx, "fetch", "origin").AddArguments(fetchArgs...).AddDashesAndList(pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr, pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() if err := git.NewCommand(ctx, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to set HEAD as base branch [%s]: %v\n%s\n%s", tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to set HEAD as base branch [tmpBasePath]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to set HEAD as base branch in [%s]: %v\n%s\n%s", pr, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to set HEAD as base branch in tmpBasePath: %w\n%s\n%s", err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { - log.Error("Unable to add head repository to temporary repo [%s -> %s]: %v", pr.HeadRepo.FullName(), tmpBasePath, err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err) + log.Error("%-v Unable to add head repository to temporary repo [%s -> %s]: %v", pr, pr.HeadRepo.FullName(), tmpBasePath, err) + cancel() + return nil, nil, fmt.Errorf("Unable to add head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err) } if err := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteRepoName, headRepoPath). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr.HeadRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr, pr.HeadRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() trackingBranch := "tracking" // Fetch head branch @@ -178,24 +178,18 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str headBranch = pr.GetGitRefName() } if err := git.NewCommand(ctx, "fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+trackingBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } + Run(prCtx.RunOpts()); err != nil { + cancel() if !git.IsBranchExist(ctx, pr.HeadRepo.RepoPath(), pr.HeadBranch) { - return "", models.ErrBranchDoesNotExist{ + return nil, nil, models.ErrBranchDoesNotExist{ BranchName: pr.HeadBranch, } } - log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, outbuf.String(), errbuf.String()) + log.Error("%-v Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr, pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + return nil, nil, fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() + prCtx.outbuf.Reset() + prCtx.errbuf.Reset() - return tmpBasePath, nil + return prCtx, cancel, nil } diff --git a/services/pull/update.go b/services/pull/update.go index b9525cf0c..b977dbdba 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -16,61 +16,67 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - repo_module "code.gitea.io/gitea/modules/repository" ) // Update updates pull request with base branch. -func Update(ctx context.Context, pull *issues_model.PullRequest, doer *user_model.User, message string, rebase bool) error { - var ( - pr *issues_model.PullRequest - style repo_model.MergeStyle - ) - - pullWorkingPool.CheckIn(fmt.Sprint(pull.ID)) - defer pullWorkingPool.CheckOut(fmt.Sprint(pull.ID)) - - if rebase { - pr = pull - style = repo_model.MergeStyleRebaseUpdate - } else { - // use merge functions but switch repo's and branch's - pr = &issues_model.PullRequest{ - HeadRepoID: pull.BaseRepoID, - BaseRepoID: pull.HeadRepoID, - HeadBranch: pull.BaseBranch, - BaseBranch: pull.HeadBranch, - } - style = repo_model.MergeStyleMerge +func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, message string, rebase bool) error { + if pr.Flow == issues_model.PullRequestFlowAGit { + // TODO: update of agit flow pull request's head branch is unsupported + return fmt.Errorf("update of agit flow pull request's head branch is unsupported") } - if pull.Flow == issues_model.PullRequestFlowAGit { - // TODO: Not support update agit flow pull request's head branch - return fmt.Errorf("Not support update agit flow pull request's head branch") - } + pullWorkingPool.CheckIn(fmt.Sprint(pr.ID)) + defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) - if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - return fmt.Errorf("LoadHeadRepo: %w", err) - } else if err = pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return fmt.Errorf("LoadBaseRepo: %w", err) - } - - diffCount, err := GetDiverging(ctx, pull) + diffCount, err := GetDiverging(ctx, pr) if err != nil { return err } else if diffCount.Behind == 0 { - return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index) + return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index) } - _, err = rawMerge(ctx, pr, doer, style, "", message) + if rebase { + defer func() { + go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "") + }() + + return updateHeadByRebaseOnToBase(ctx, pr, doer, message) + } + + if err := pr.LoadBaseRepo(ctx); err != nil { + log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err) + return fmt.Errorf("unable to load BaseRepo for PR[%d] during update-by-merge: %w", pr.ID, err) + } + if err := pr.LoadHeadRepo(ctx); err != nil { + log.Error("unable to load HeadRepo for PR %-v during update-by-merge: %v", pr, err) + return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err) + } + if pr.HeadRepo == nil { + // LoadHeadRepo will swallow ErrRepoNotExist so if pr.HeadRepo is still nil recreate the error + err := repo_model.ErrRepoNotExist{ + ID: pr.HeadRepoID, + } + log.Error("unable to load HeadRepo for PR %-v during update-by-merge: %v", pr, err) + return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err) + } + + // use merge functions but switch repos and branches + reversePR := &issues_model.PullRequest{ + ID: pr.ID, + + HeadRepoID: pr.BaseRepoID, + HeadRepo: pr.BaseRepo, + HeadBranch: pr.BaseBranch, + + BaseRepoID: pr.HeadRepoID, + BaseRepo: pr.HeadRepo, + BaseBranch: pr.HeadBranch, + } + + _, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message) defer func() { - if rebase { - go AddTestPullRequestTask(doer, pr.BaseRepo.ID, pr.BaseBranch, false, "", "") - return - } - go AddTestPullRequestTask(doer, pr.HeadRepo.ID, pr.HeadBranch, false, "", "") + go AddTestPullRequestTask(doer, reversePR.HeadRepo.ID, reversePR.HeadBranch, false, "", "") }() return err @@ -159,27 +165,16 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, // GetDiverging determines how many commits a PR is ahead or behind the PR base branch func GetDiverging(ctx context.Context, pr *issues_model.PullRequest) (*git.DivergeObject, error) { - log.Trace("GetDiverging[%d]: compare commits", pr.ID) - if err := pr.LoadBaseRepo(ctx); err != nil { - return nil, err - } - if err := pr.LoadHeadRepo(ctx); err != nil { - return nil, err - } - - tmpRepo, err := createTemporaryRepo(ctx, pr) + log.Trace("GetDiverging[%-v]: compare commits", pr) + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { if !models.IsErrBranchDoesNotExist(err) { - log.Error("CreateTemporaryRepo: %v", err) + log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) } return nil, err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpRepo); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - diff, err := git.GetDivergingCommits(ctx, tmpRepo, "base", "tracking") + diff, err := git.GetDivergingCommits(ctx, prCtx.tmpBasePath, baseBranch, trackingBranch) return &diff, err } diff --git a/services/pull/update_rebase.go b/services/pull/update_rebase.go new file mode 100644 index 000000000..8e7bfa0ff --- /dev/null +++ b/services/pull/update_rebase.go @@ -0,0 +1,107 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + "fmt" + "strings" + + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" +) + +// updateHeadByRebaseOnToBase handles updating a PR's head branch by rebasing it on the PR current base branch +func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, message string) error { + // "Clone" base repo and add the cache headers for the head repo and branch + mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, "") + if err != nil { + return err + } + defer cancel() + + // Determine the old merge-base before the rebase - we use this for LFS push later on + oldMergeBase, _, _ := git.NewCommand(ctx, "merge-base").AddDashesAndList(baseBranch, trackingBranch).RunStdString(&git.RunOpts{Dir: mergeCtx.tmpBasePath}) + oldMergeBase = strings.TrimSpace(oldMergeBase) + + // Rebase the tracking branch on to the base as the staging branch + if err := rebaseTrackingOnToBase(mergeCtx, repo_model.MergeStyleRebaseUpdate); err != nil { + return err + } + + if setting.LFS.StartServer { + // Now we need to ensure that the head repository contains any LFS objects between the new base and the old mergebase + // It's questionable about where this should go - either after or before the push + // I think in the interests of data safety - failures to push to the lfs should prevent + // the push as you can always re-rebase. + if err := LFSPush(ctx, mergeCtx.tmpBasePath, baseBranch, oldMergeBase, &issues_model.PullRequest{ + HeadRepoID: pr.BaseRepoID, + BaseRepoID: pr.HeadRepoID, + }); err != nil { + log.Error("Unable to push lfs objects between %s and %s up to head branch in %-v: %v", baseBranch, oldMergeBase, pr, err) + return err + } + } + + // Now determine who the pushing author should be + var headUser *user_model.User + if err := pr.HeadRepo.LoadOwner(ctx); err != nil { + if !user_model.IsErrUserNotExist(err) { + log.Error("Can't find user: %d for head repository in %-v - %v", pr.HeadRepo.OwnerID, pr, err) + return err + } + log.Error("Can't find user: %d for head repository in %-v - defaulting to doer: %-v - %v", pr.HeadRepo.OwnerID, pr, doer, err) + headUser = doer + } else { + headUser = pr.HeadRepo.Owner + } + + pushCmd := git.NewCommand(ctx, "push", "-f", "head_repo"). + AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch) + + // Push back to the head repository. + // TODO: this cause an api call to "/api/internal/hook/post-receive/...", + // that prevents us from doint the whole merge in one db transaction + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() + + if err := pushCmd.Run(&git.RunOpts{ + Env: repo_module.FullPushingEnvironment( + headUser, + doer, + pr.HeadRepo, + pr.HeadRepo.Name, + pr.ID, + ), + Dir: mergeCtx.tmpBasePath, + Stdout: mergeCtx.outbuf, + Stderr: mergeCtx.errbuf, + }); err != nil { + if strings.Contains(mergeCtx.errbuf.String(), "non-fast-forward") { + return &git.ErrPushOutOfDate{ + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), + Err: err, + } + } else if strings.Contains(mergeCtx.errbuf.String(), "! [remote rejected]") { + err := &git.ErrPushRejected{ + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), + Err: err, + } + err.GenerateMessage() + return err + } + return fmt.Errorf("git push: %s", mergeCtx.errbuf.String()) + } + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() + + return nil +}