From 121754637caffe09368008dcea8d2729114ddec2 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 21 Jan 2023 13:09:00 +0000 Subject: [PATCH 1/8] Refactor and tidy-up the merge/update branch code 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 --- models/issues/pull.go | 62 +++- modules/git/pipeline/revlist.go | 5 +- services/pull/lfs.go | 2 +- services/pull/merge.go | 522 +++++--------------------------- services/pull/merge_merge.go | 25 ++ services/pull/merge_prepare.go | 293 ++++++++++++++++++ services/pull/merge_rebase.go | 50 +++ services/pull/merge_squash.go | 51 ++++ services/pull/patch.go | 15 +- services/pull/temp_repo.go | 189 ++++++------ services/pull/update.go | 103 +++---- services/pull/update_rebase.go | 105 +++++++ 12 files changed, 802 insertions(+), 620 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/models/issues/pull.go b/models/issues/pull.go index 93b227f3fd5ce..7efb7b97f7990 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -202,6 +202,59 @@ func DeletePullsByBaseRepoID(ctx context.Context, repoID int64) error { return err } +// ColorFormat writes a colored string to identify this struct +func (pr *PullRequest) ColorFormat(s fmt.State) { + if pr == nil { + log.ColorFprintf(s, "PR[%d][%s:%s...%s:%s]", + log.NewColoredIDValue(0), + log.NewColoredValue("/"), + log.NewColoredValue(""), + log.NewColoredValue("/"), + log.NewColoredValue(""), + ) + return + } + + log.ColorFprintf(s, "PR[%d][", log.NewColoredIDValue(pr.ID)) + if pr.BaseRepo != nil { + log.ColorFprintf(s, "%s:%s...", log.NewColoredValue(pr.BaseRepo.FullName()), log.NewColoredValue(pr.BaseBranch)) + } else { + log.ColorFprintf(s, "Repo[%d]:%s...", log.NewColoredIDValue(pr.BaseRepoID), log.NewColoredValue(pr.BaseBranch)) + } + if pr.HeadRepoID == pr.BaseRepoID { + log.ColorFprintf(s, "%s]", log.NewColoredValue(pr.HeadBranch)) + } else if pr.HeadRepo != nil { + log.ColorFprintf(s, "%s:%s]", log.NewColoredValue(pr.HeadRepo.FullName()), log.NewColoredValue(pr.HeadBranch)) + } else { + log.ColorFprintf(s, "Repo[%d]:%s]", log.NewColoredIDValue(pr.HeadRepoID), log.NewColoredValue(pr.HeadBranch)) + } +} + +// String represents the pr as a simple string +func (pr *PullRequest) String() string { + if pr == nil { + return "PR[0][/:.../:]" + } + + s := new(strings.Builder) + + _, _ = fmt.Fprintf(s, "PR[%d][", pr.ID) + if pr.BaseRepo != nil { + _, _ = fmt.Fprintf(s, "%s:%s...", pr.BaseRepo.FullName(), pr.BaseBranch) + } else { + _, _ = fmt.Fprintf(s, "Repo[%d]:%s...", pr.BaseRepoID, pr.BaseBranch) + } + if pr.HeadRepoID == pr.BaseRepoID { + _, _ = fmt.Fprintf(s, "%s]", pr.HeadBranch) + } else if pr.HeadRepo != nil { + _, _ = fmt.Fprintf(s, "%s:%s]", pr.HeadRepo.FullName(), pr.HeadBranch) + } else { + _, _ = fmt.Fprintf(s, "Repo[%d]:%s]", pr.HeadRepoID, pr.HeadBranch) + } + + return s.String() +} + // MustHeadUserName returns the HeadRepo's username if failed return blank func (pr *PullRequest) MustHeadUserName(ctx context.Context) string { if err := pr.LoadHeadRepo(ctx); err != nil { @@ -234,7 +287,8 @@ func (pr *PullRequest) LoadAttributes(ctx context.Context) (err error) { return nil } -// LoadHeadRepo loads the head repository +// LoadHeadRepo loads the head repository, pr.HeadRepo will remain nil if it does not exist +// and thus ErrRepoNotExist will never be returned func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { if !pr.isHeadRepoLoaded && pr.HeadRepo == nil && pr.HeadRepoID > 0 { if pr.HeadRepoID == pr.BaseRepoID { @@ -249,14 +303,14 @@ func (pr *PullRequest) LoadHeadRepo(ctx context.Context) (err error) { pr.HeadRepo, err = repo_model.GetRepositoryByID(ctx, pr.HeadRepoID) if err != nil && !repo_model.IsErrRepoNotExist(err) { // Head repo maybe deleted, but it should still work - return fmt.Errorf("GetRepositoryByID(head): %w", err) + return fmt.Errorf("pr[%d].LoadHeadRepo[%d]: %w", pr.ID, pr.HeadRepoID, err) } pr.isHeadRepoLoaded = true } return nil } -// LoadBaseRepo loads the target repository +// LoadBaseRepo loads the target repository. ErrRepoNotExist may be returned. func (pr *PullRequest) LoadBaseRepo(ctx context.Context) (err error) { if pr.BaseRepo != nil { return nil @@ -274,7 +328,7 @@ func (pr *PullRequest) LoadBaseRepo(ctx context.Context) (err error) { pr.BaseRepo, err = repo_model.GetRepositoryByID(ctx, pr.BaseRepoID) if err != nil { - return fmt.Errorf("repo_model.GetRepositoryByID(base): %w", err) + return fmt.Errorf("pr[%d].LoadBaseRepo[%d]: %w", pr.ID, pr.BaseRepoID, err) } return nil } diff --git a/modules/git/pipeline/revlist.go b/modules/git/pipeline/revlist.go index 09bb2c8b3c844..d88ebe78ef200 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 dc4ca006e4915..39433724684e1 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 bdd2cb0e8651f..2f350e1fb504e 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 } @@ -141,18 +133,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 } @@ -176,7 +169,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 } @@ -186,18 +179,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.GetOwner(hammerCtx); err != nil { - log.Error("GetOwner for PR [%d]: %v", pr.ID, err) + log.Error("GetOwner for %-v: %v", pr, err) } if wasAutoMerged { @@ -236,339 +229,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 - var signArg git.CmdArg - sign, keyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) - if sign { - signArg = git.CmdArg("-S" + keyID) - if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - committer = signer - } - } else { - signArg = git.CmdArg("--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, signArg, tmpBasePath, env); err != nil { - log.Error("Unable to make final commit: %v", err) - 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) + case repo_model.MergeStyleRebase, repo_model.MergeStyleRebaseMerge: + if err := doMergeStyleRebase(mergeCtx, mergeStyle, message); err != nil { return "", err } - if mergeStyle == repo_model.MergeStyleRebaseMerge { - if err := commitAndSignNoAuthor(ctx, pr, message, signArg, 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 signArg == "" { - if err := git.NewCommand(ctx, "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), "-m").AddDynamicArguments(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()) - } - } else { - 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(signArg). - AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email))). - AddArguments("-m").AddDynamicArguments(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) } @@ -577,7 +274,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 } } @@ -586,180 +283,101 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode err = pr.HeadRepo.GetOwner(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, signArg git.CmdArg, tmpBasePath string, env []string) error { - var outbuf, errbuf strings.Builder - if signArg == "" { +func commitAndSignNoAuthor(ctx *mergeContext, message string) error { + if ctx.signArg == "" { if err := git.NewCommand(ctx, "commit", "-m").AddDynamicArguments(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()) + 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()) } } else { - if err := git.NewCommand(ctx, "commit").AddArguments(signArg).AddArguments("-m").AddDynamicArguments(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()) + if err := git.NewCommand(ctx, "commit").AddArguments(ctx.signArg).AddArguments("-m").AddDynamicArguments(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 0000000000000..1a71320f1f899 --- /dev/null +++ b/services/pull/merge_merge.go @@ -0,0 +1,25 @@ +// Copyright 2022 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("Unable to merge tracking into base: %v", err) + return err + } + + 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_prepare.go b/services/pull/merge_prepare.go new file mode 100644 index 0000000000000..b3694fad64d85 --- /dev/null +++ b/services/pull/merge_prepare.go @@ -0,0 +1,293 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "bufio" + "bytes" + "context" + "fmt" + "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.CmdArg + 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.CmdArg("-S" + keyID) + if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + mergeCtx.committer = signer + } + } else { + mergeCtx.signArg = git.CmdArg("--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 thngs +func prepareTemporaryRepoForMerge(ctx *mergeContext) error { + // Enable sparse-checkout + sparseCheckoutList, err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, ctx.outbuf) + if err != nil { + log.Error("getDiffTree(%s, %s, %s): %v", ctx.tmpBasePath, baseBranch, trackingBranch, err) + return fmt.Errorf("getDiffTree: %w", err) + } + ctx.outbuf.Reset() + + infoPath := filepath.Join(ctx.tmpBasePath, ".git", "info") + if err := os.MkdirAll(infoPath, 0o700); err != nil { + log.Error("Unable to create .git/info in %s: %v", ctx.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", ctx.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") + } + + setConfig := func(key, value git.CmdArg) error { + if err := gitConfigCommand().AddArguments(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 +func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out *strings.Builder) (string, error) { + out.Reset() + defer out.Reset() + + 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 out.String(), 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 0000000000000..296fae1526759 --- /dev/null +++ b/services/pull/merge_rebase.go @@ -0,0 +1,50 @@ +// Copyright 2022 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 [%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() + + 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 0000000000000..cf393930f6e4e --- /dev/null +++ b/services/pull/merge_squash.go @@ -0,0 +1,51 @@ +// Copyright 2022 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("Unable to merge --squash tracking into base: %v", err) + return err + } + + if err := ctx.pr.Issue.LoadPoster(ctx); err != nil { + log.Error("LoadPoster: %v", err) + return fmt.Errorf("LoadPoster: %w", err) + } + sig := ctx.pr.Issue.Poster.NewGitSig() + if ctx.signArg == "" { + if err := git.NewCommand(ctx, "commit", git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email)), "-m").AddDynamicArguments(message). + Run(ctx.RunOpts()); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\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()) + 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). + AddArguments(git.CmdArg(fmt.Sprintf("--author='%s <%s>'", sig.Name, sig.Email))). + AddArguments("-m").AddDynamicArguments(message). + Run(ctx.RunOpts()); err != nil { + log.Error("git commit [%s:%s -> %s:%s]: %v\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()) + 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 26a72a7371bf1..bfe673c34ad04 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) 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/temp_repo.go b/services/pull/temp_repo.go index d49a15cea00ad..1998515ede21b 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -19,147 +19,146 @@ 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.GetOwner(ctx); err != nil { - log.Error("HeadRepo.GetOwner: %v", err) - return "", fmt.Errorf("HeadRepo.GetOwner: %w", err) + log.Error("%-v HeadRepo.GetOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v HeadRepo.GetOwner: %w", pr, err) } else if err := pr.BaseRepo.GetOwner(ctx); err != nil { - log.Error("BaseRepo.GetOwner: %v", err) - return "", fmt.Errorf("BaseRepo.GetOwner: %w", err) + log.Error("%-v BaseRepo.GetOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v BaseRepo.GetOwner: %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, + 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" baseBranch := "base" - // 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) + 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("Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", 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", "--no-tags").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("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, 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("Unable to set HEAD as base branch [%s]: %v\n%s\n%s", tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to set HEAD as base branch [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) + cancel() + return nil, nil, fmt.Errorf("Unable to 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("Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", 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 @@ -172,24 +171,18 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str headBranch = pr.GetGitRefName() } if err := git.NewCommand(ctx, "fetch", "--no-tags").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("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", 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 ede89bcdfff3e..e219114679b71 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 - ) +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") + } - pullWorkingPool.CheckIn(fmt.Sprint(pull.ID)) - defer pullWorkingPool.CheckOut(fmt.Sprint(pull.ID)) + pullWorkingPool.CheckIn(fmt.Sprint(pr.ID)) + defer pullWorkingPool.CheckOut(fmt.Sprint(pr.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 + 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", pr.Index) } - 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") + 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("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) + 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) } - diffCount, err := GetDiverging(ctx, pull) - if err != nil { - return err - } else if diffCount.Behind == 0 { - return fmt.Errorf("HeadBranch of PR %d is up to date", pull.Index) + // 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 = rawMerge(ctx, pr, doer, style, "", message) + _, 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) } 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 0000000000000..3439241b9b314 --- /dev/null +++ b/services/pull/update_rebase.go @@ -0,0 +1,105 @@ +// Copyright 2022 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}) + + // 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 { + return err + } + } + + // Now determine who the pushing author should be + var headUser *user_model.User + if err := pr.HeadRepo.GetOwner(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 +} From 6874c95a87f3c279a8112d08e98a22ac4160eaa1 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 22 Jan 2023 15:57:00 +0000 Subject: [PATCH 2/8] oops Signed-off-by: Andrew Thornton --- services/pull/temp_repo.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index 1998515ede21b..4585425e00903 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -79,10 +79,11 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) return nil, nil, err } prCtx = &prContext{ - Context: ctx, - pr: pr, - outbuf: &strings.Builder{}, - errbuf: &strings.Builder{}, + Context: ctx, + tmpBasePath: tmpBasePath, + pr: pr, + outbuf: &strings.Builder{}, + errbuf: &strings.Builder{}, } cancel = func() { if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { From 8a7b3230128d8f21ae49bdaaa44022dc42626ace Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 22 Jan 2023 17:18:13 +0000 Subject: [PATCH 3/8] trim the output from merge-base Signed-off-by: Andrew Thornton --- services/pull/update_rebase.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/services/pull/update_rebase.go b/services/pull/update_rebase.go index 3439241b9b314..365946e417d56 100644 --- a/services/pull/update_rebase.go +++ b/services/pull/update_rebase.go @@ -28,6 +28,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques // 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 { @@ -43,6 +44,7 @@ func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullReques 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 } } From 01f0aeb2b406425f78552f0189caf0dc3d1b1b94 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 24 Jan 2023 18:44:22 +0000 Subject: [PATCH 4/8] create a common simple way of converting a ColorFormat into a simple string Signed-off-by: Andrew Thornton --- models/issues/pull.go | 22 +--------------------- modules/log/colors.go | 7 +++++++ 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index 7efb7b97f7990..42716755bd42d 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -232,27 +232,7 @@ func (pr *PullRequest) ColorFormat(s fmt.State) { // String represents the pr as a simple string func (pr *PullRequest) String() string { - if pr == nil { - return "PR[0][/:.../:]" - } - - s := new(strings.Builder) - - _, _ = fmt.Fprintf(s, "PR[%d][", pr.ID) - if pr.BaseRepo != nil { - _, _ = fmt.Fprintf(s, "%s:%s...", pr.BaseRepo.FullName(), pr.BaseBranch) - } else { - _, _ = fmt.Fprintf(s, "Repo[%d]:%s...", pr.BaseRepoID, pr.BaseBranch) - } - if pr.HeadRepoID == pr.BaseRepoID { - _, _ = fmt.Fprintf(s, "%s]", pr.HeadBranch) - } else if pr.HeadRepo != nil { - _, _ = fmt.Fprintf(s, "%s:%s]", pr.HeadRepo.FullName(), pr.HeadBranch) - } else { - _, _ = fmt.Fprintf(s, "Repo[%d]:%s]", pr.HeadRepoID, pr.HeadBranch) - } - - return s.String() + return log.ColorFormatAsString(pr) } // MustHeadUserName returns the HeadRepo's username if failed return blank diff --git a/modules/log/colors.go b/modules/log/colors.go index 02781afe84ebd..85e205cb6764d 100644 --- a/modules/log/colors.go +++ b/modules/log/colors.go @@ -383,6 +383,13 @@ func (cv *ColoredValue) Format(s fmt.State, c rune) { s.Write(*cv.resetBytes) } +// ColorFormatAsString returns the result of the ColorFormat without the color +func ColorFormatAsString(colorVal ColorFormatted) string { + s := new(strings.Builder) + _, _ = ColorFprintf(&protectedANSIWriter{w: s, mode: removeColor}, "%-v", colorVal) + return s.String() +} + // SetColorBytes will allow a user to set the colorBytes of a colored value func (cv *ColoredValue) SetColorBytes(colorBytes []byte) { cv.colorBytes = &colorBytes From 71c9b9312d792e27721e6603139da6cd836eb62c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Fri, 27 Jan 2023 21:44:23 +0000 Subject: [PATCH 5/8] adjust pull request colorformat methods Signed-off-by: Andrew Thornton --- models/issues/pull.go | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index 42716755bd42d..bd4e30e068ee8 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "io" + "strconv" "strings" "code.gitea.io/gitea/models/db" @@ -132,6 +133,27 @@ const ( PullRequestStatusAncestor ) +func (status PullRequestStatus) String() string { + switch status { + case PullRequestStatusConflict: + return "CONFLICT" + case PullRequestStatusChecking: + return "CHECKING" + case PullRequestStatusMergeable: + return "MERGEABLE" + case PullRequestStatusManuallyMerged: + return "MANUALLY_MERGED" + case PullRequestStatusError: + return "ERROR" + case PullRequestStatusEmpty: + return "EMPTY" + case PullRequestStatusAncestor: + return "ANCESTOR" + default: + return strconv.Itoa(int(status)) + } +} + // PullRequestFlow the flow of pull request type PullRequestFlow int @@ -205,9 +227,10 @@ func DeletePullsByBaseRepoID(ctx context.Context, repoID int64) error { // ColorFormat writes a colored string to identify this struct func (pr *PullRequest) ColorFormat(s fmt.State) { if pr == nil { - log.ColorFprintf(s, "PR[%d][%s:%s...%s:%s]", + log.ColorFprintf(s, "PR[%d]%s#%d[%s...%s:%s]", log.NewColoredIDValue(0), log.NewColoredValue("/"), + log.NewColoredIDValue(0), log.NewColoredValue(""), log.NewColoredValue("/"), log.NewColoredValue(""), @@ -215,11 +238,13 @@ func (pr *PullRequest) ColorFormat(s fmt.State) { return } - log.ColorFprintf(s, "PR[%d][", log.NewColoredIDValue(pr.ID)) + log.ColorFprintf(s, "PR[%d]", log.NewColoredIDValue(pr.ID)) if pr.BaseRepo != nil { - log.ColorFprintf(s, "%s:%s...", log.NewColoredValue(pr.BaseRepo.FullName()), log.NewColoredValue(pr.BaseBranch)) + log.ColorFprintf(s, "%s#%d[%s...", log.NewColoredValue(pr.BaseRepo.FullName()), + log.NewColoredIDValue(pr.Index), log.NewColoredValue(pr.BaseBranch)) } else { - log.ColorFprintf(s, "Repo[%d]:%s...", log.NewColoredIDValue(pr.BaseRepoID), log.NewColoredValue(pr.BaseBranch)) + log.ColorFprintf(s, "Repo[%d]#%d[%s...", log.NewColoredIDValue(pr.BaseRepoID), + log.NewColoredIDValue(pr.Index), log.NewColoredValue(pr.BaseBranch)) } if pr.HeadRepoID == pr.BaseRepoID { log.ColorFprintf(s, "%s]", log.NewColoredValue(pr.HeadBranch)) From c196577a172308d3af955bd6d474f59ef36d4efc Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 2 Mar 2023 14:11:57 +0000 Subject: [PATCH 6/8] few more minor changes Signed-off-by: Andrew Thornton --- services/pull/merge_prepare.go | 36 +++++++++++++++++++--------------- services/pull/pull.go | 2 +- services/pull/update.go | 2 +- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go index 0121f97388b34..2ba821961a2be 100644 --- a/services/pull/merge_prepare.go +++ b/services/pull/merge_prepare.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "fmt" + "io" "os" "path/filepath" "strings" @@ -110,25 +111,30 @@ func createTemporaryRepoForMerge(ctx context.Context, pr *issues_model.PullReque // 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 { - // Enable sparse-checkout - sparseCheckoutList, err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, ctx.outbuf) - if err != nil { - log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, baseBranch, trackingBranch, err) - return fmt.Errorf("getDiffTree: %w", err) - } - ctx.outbuf.Reset() - 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) } - sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") - if err := os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0o600); err != nil { + // 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") @@ -183,14 +189,12 @@ func prepareTemporaryRepoForMerge(ctx *mergeContext) error { } // getDiffTree returns a string containing all the files that were changed between headBranch and baseBranch -func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out *strings.Builder) (string, error) { - out.Reset() - defer out.Reset() - +// 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 + return err } defer func() { _ = diffOutReader.Close() @@ -235,7 +239,7 @@ func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, o return scanner.Err() }, }) - return out.String(), err + return err } // rebaseTrackingOnToBase checks out the tracking branch as staging and rebases it on to the base branch diff --git a/services/pull/pull.go b/services/pull/pull.go index 4e51712ac703b..cbbe065a906c8 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -351,7 +351,7 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) { 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 cancel() diff --git a/services/pull/update.go b/services/pull/update.go index b67b469025be8..b977dbdba9fe2 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -169,7 +169,7 @@ func GetDiverging(ctx context.Context, pr *issues_model.PullRequest) (*git.Diver 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 } From cfdb33d3f165b56bf226f333c89caa278e590aaf Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 2 Mar 2023 14:32:45 +0000 Subject: [PATCH 7/8] more minor changes Signed-off-by: Andrew Thornton --- services/pull/patch.go | 2 +- services/pull/temp_repo.go | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/services/pull/patch.go b/services/pull/patch.go index 4606203d379a6..9277355720dc5 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -65,7 +65,7 @@ func TestPatch(pr *issues_model.PullRequest) error { // Clone base repo. prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { - log.Error("CreateTemporaryPath: %v", err) + log.Error("createTemporaryRepoForPR %-v: %v", pr, err) return err } defer cancel() diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index 20377592ac769..146470780671e 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -128,41 +128,41 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) // 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) + 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) } if err := git.NewCommand(ctx, "remote", "add", "-t").AddDynamicArguments(pr.BaseBranch).AddArguments("-m").AddDynamicArguments(pr.BaseBranch).AddDynamicArguments("origin", baseRepoPath). Run(prCtx.RunOpts()); err != nil { - log.Error("Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr.BaseRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + 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()) } if err := git.NewCommand(ctx, "fetch", "origin").AddArguments(fetchArgs...).AddDashesAndList(pr.BaseBranch+":"+baseBranch, pr.BaseBranch+":original_"+baseBranch). Run(prCtx.RunOpts()); 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, prCtx.outbuf.String(), prCtx.errbuf.String()) + 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()) } if err := git.NewCommand(ctx, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseBranch). Run(prCtx.RunOpts()); err != nil { - log.Error("Unable to set HEAD as base branch [%s]: %v\n%s\n%s", tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + 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 [tmpBasePath]: %w\n%s\n%s", err, prCtx.outbuf.String(), prCtx.errbuf.String()) + 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()) } 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) + 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 head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err) + 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(prCtx.RunOpts()); err != nil { - log.Error("Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr.HeadRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + 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()) } @@ -185,7 +185,7 @@ func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) 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, prCtx.outbuf.String(), prCtx.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()) } prCtx.outbuf.Reset() From 5196fd2803984bfe587f5a775efa0874fdd7427a Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Tue, 7 Mar 2023 14:31:03 -0500 Subject: [PATCH 8/8] Update services/pull/merge.go Co-authored-by: delvh --- services/pull/merge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/pull/merge.go b/services/pull/merge.go index f304973fa6462..afa924fc109ac 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -193,7 +193,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U 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 {