diff options
author | Anthony Wang | 2023-03-12 23:17:11 +0000 |
---|---|---|
committer | Anthony Wang | 2023-03-12 23:19:57 +0000 |
commit | dbc3eb7fa071307e623fde961bfdff0894856948 (patch) | |
tree | 8961dc6a935d9ee89f5adf680a142d36900aaebb /services | |
parent | b4640101f3c8938ed689743606a79601201141ca (diff) | |
parent | e72290fd9aeb77a47311483d1d565e428ce40cd9 (diff) |
Merge remote-tracking branch 'origin/main' into forgejo-federation
Diffstat (limited to 'services')
42 files changed, 967 insertions, 812 deletions
diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index 583e588de..6f8e95218 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -43,6 +43,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { return fmt.Errorf("find tasks: %w", err) } + jobs := make([]*actions_model.ActionRunJob, 0, len(tasks)) for _, task := range tasks { if err := db.WithTx(ctx, func(ctx context.Context) error { if err := actions_model.StopTask(ctx, task.ID, actions_model.StatusFailure); err != nil { @@ -51,7 +52,8 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { if err := task.LoadJob(ctx); err != nil { return err } - return CreateCommitStatus(ctx, task.Job) + jobs = append(jobs, task.Job) + return nil }); err != nil { log.Warn("Cannot stop task %v: %v", task.ID, err) // go on @@ -61,6 +63,14 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { remove() } } + + for _, job := range jobs { + if err := CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + } + return nil } @@ -80,14 +90,16 @@ func CancelAbandonedJobs(ctx context.Context) error { job.Status = actions_model.StatusCancelled job.Stopped = now if err := db.WithTx(ctx, func(ctx context.Context) error { - if _, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped"); err != nil { - return err - } - return CreateCommitStatus(ctx, job) + _, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped") + return err }); err != nil { log.Warn("cancel abandoned job %v: %v", job.ID, err) // go on } + if err := CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } } return nil diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index efb5ec6d4..4f3134935 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -30,6 +30,16 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return fmt.Errorf("GetPushEventPayload: %w", err) } + // Since the payload comes from json data, we should check if it's broken, or it will cause panic + switch { + case payload.Repo == nil: + return fmt.Errorf("repo is missing in event payload") + case payload.Pusher == nil: + return fmt.Errorf("pusher is missing in event payload") + case payload.HeadCommit == nil: + return fmt.Errorf("head commit is missing in event payload") + } + creator, err := user_model.GetUserByID(ctx, payload.Pusher.ID) if err != nil { return fmt.Errorf("GetUserByID: %w", err) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index ef63b8cf9..574a37e9a 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -187,7 +187,8 @@ func notify(ctx context.Context, input *notifyInput) error { } else { for _, job := range jobs { if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("CreateCommitStatus: %v", err) + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on } } } diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 7fdacc6aa..3e7df0cee 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -19,14 +19,14 @@ import ( ) // NewAttachment creates a new attachment object, but do not verify. -func NewAttachment(attach *repo_model.Attachment, file io.Reader) (*repo_model.Attachment, error) { +func NewAttachment(attach *repo_model.Attachment, file io.Reader, size int64) (*repo_model.Attachment, error) { if attach.RepoID == 0 { return nil, fmt.Errorf("attachment %s should belong to a repository", attach.Name) } err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { attach.UUID = uuid.New().String() - size, err := storage.Attachments.Save(attach.RelativePath(), file, -1) + size, err := storage.Attachments.Save(attach.RelativePath(), file, size) if err != nil { return fmt.Errorf("Create: %w", err) } @@ -39,7 +39,7 @@ func NewAttachment(attach *repo_model.Attachment, file io.Reader) (*repo_model.A } // UploadAttachment upload new attachment into storage and update database -func UploadAttachment(file io.Reader, allowedTypes string, opts *repo_model.Attachment) (*repo_model.Attachment, error) { +func UploadAttachment(file io.Reader, allowedTypes string, fileSize int64, opts *repo_model.Attachment) (*repo_model.Attachment, error) { buf := make([]byte, 1024) n, _ := util.ReadAtMost(file, buf) buf = buf[:n] @@ -48,5 +48,5 @@ func UploadAttachment(file io.Reader, allowedTypes string, opts *repo_model.Atta return nil, err } - return NewAttachment(opts, io.MultiReader(bytes.NewReader(buf), file)) + return NewAttachment(opts, io.MultiReader(bytes.NewReader(buf), file), fileSize) } diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index 72d1b2ab3..1b9af3442 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -36,7 +36,7 @@ func TestUploadAttachment(t *testing.T) { RepoID: 1, UploaderID: user.ID, Name: filepath.Base(fPath), - }, f) + }, f, -1) assert.NoError(t, err) attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attach.UUID) diff --git a/services/auth/source/oauth2/source_register.go b/services/auth/source/oauth2/source_register.go index 3527d54b6..82a36acaa 100644 --- a/services/auth/source/oauth2/source_register.go +++ b/services/auth/source/oauth2/source_register.go @@ -36,6 +36,10 @@ func (err ErrOpenIDConnectInitialize) Error() string { return fmt.Sprintf("Failed to initialize OpenID Connect Provider with name '%s' with url '%s': %v", err.ProviderName, err.OpenIDConnectAutoDiscoveryURL, err.Cause) } +func (err ErrOpenIDConnectInitialize) Unwrap() error { + return err.Cause +} + // wrapOpenIDConnectInitializeError is used to wrap the error but this cannot be done in modules/auth/oauth2 // inside oauth2: import cycle not allowed models -> modules/auth/oauth2 -> models func wrapOpenIDConnectInitializeError(err error, providerName string, source *Source) error { diff --git a/services/context/user.go b/services/context/user.go index 7642cba4e..9dc84c3ac 100644 --- a/services/context/user.go +++ b/services/context/user.go @@ -8,7 +8,6 @@ import ( "net/http" "strings" - org_model "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" ) @@ -57,14 +56,6 @@ func userAssignment(ctx *context.Context, errCb func(int, string, interface{})) } else { errCb(http.StatusInternalServerError, "GetUserByName", err) } - } else { - if ctx.ContextUser.IsOrganization() { - if ctx.Org == nil { - ctx.Org = &context.Organization{} - } - ctx.Org.Organization = (*org_model.Organization)(ctx.ContextUser) - ctx.Data["Org"] = ctx.Org.Organization - } } } } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index e9645b5ab..3bd073c07 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -622,7 +622,7 @@ type CodeCommentForm struct { Side string `binding:"Required;In(previous,proposed)"` Line int64 TreePath string `form:"path" binding:"Required"` - IsReview bool `form:"is_review"` + SingleReview bool `form:"single_review"` Reply int64 `form:"reply"` LatestCommitID string } diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 4a74c1a89..c50e8137a 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -474,7 +474,7 @@ func ParsePatch(maxLines, maxLineCharacters, maxFiles int, reader io.Reader, ski sb := strings.Builder{} // OK let's set a reasonable buffer size. - // This should be let's say at least the size of maxLineCharacters or 4096 whichever is larger. + // This should be at least the size of maxLineCharacters or 4096 whichever is larger. readerSize := maxLineCharacters if readerSize < 4096 { readerSize = 4096 diff --git a/services/mailer/incoming/incoming_handler.go b/services/mailer/incoming/incoming_handler.go index d89a5eab3..b594e3518 100644 --- a/services/mailer/incoming/incoming_handler.go +++ b/services/mailer/incoming/incoming_handler.go @@ -87,7 +87,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u attachmentIDs := make([]string, 0, len(content.Attachments)) if setting.Attachment.Enabled { for _, attachment := range content.Attachments { - a, err := attachment_service.UploadAttachment(bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, &repo_model.Attachment{ + a, err := attachment_service.UploadAttachment(bytes.NewReader(attachment.Content), setting.Attachment.AllowedTypes, int64(len(attachment.Content)), &repo_model.Attachment{ Name: attachment.Name, UploaderID: doer.ID, RepoID: issue.Repo.ID, @@ -127,7 +127,7 @@ func (h *ReplyHandler) Handle(ctx context.Context, content *MailContent, doer *u comment.Line, content.Content, comment.TreePath, - false, + false, // not pending review but a single review comment.ReviewID, "", ) diff --git a/services/migrations/gitea_uploader.go b/services/migrations/gitea_uploader.go index 20370d99f..ca961524d 100644 --- a/services/migrations/gitea_uploader.go +++ b/services/migrations/gitea_uploader.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "os" - "path" "path/filepath" "strconv" "strings" @@ -21,6 +20,7 @@ import ( 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/label" "code.gitea.io/gitea/modules/log" base "code.gitea.io/gitea/modules/migration" repo_module "code.gitea.io/gitea/modules/repository" @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/uri" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/pull" "github.com/google/uuid" @@ -217,18 +218,20 @@ func (g *GiteaLocalUploader) CreateMilestones(milestones ...*base.Milestone) err // CreateLabels creates labels func (g *GiteaLocalUploader) CreateLabels(labels ...*base.Label) error { lbs := make([]*issues_model.Label, 0, len(labels)) - for _, label := range labels { - // We must validate color here: - if !issues_model.LabelColorPattern.MatchString("#" + label.Color) { - log.Warn("Invalid label color: #%s for label: %s in migration to %s/%s", label.Color, label.Name, g.repoOwner, g.repoName) - label.Color = "ffffff" + for _, l := range labels { + if color, err := label.NormalizeColor(l.Color); err != nil { + log.Warn("Invalid label color: #%s for label: %s in migration to %s/%s", l.Color, l.Name, g.repoOwner, g.repoName) + l.Color = "#ffffff" + } else { + l.Color = color } lbs = append(lbs, &issues_model.Label{ RepoID: g.repo.ID, - Name: label.Name, - Description: label.Description, - Color: "#" + label.Color, + Name: l.Name, + Exclusive: l.Exclusive, + Description: l.Description, + Color: l.Color, }) } @@ -863,7 +866,7 @@ func (g *GiteaLocalUploader) CreateReviews(reviews ...*base.Review) error { } // SECURITY: The TreePath must be cleaned! - comment.TreePath = path.Clean("/" + comment.TreePath)[1:] + comment.TreePath = util.CleanPath(comment.TreePath) var patch string reader, writer := io.Pipe() diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 126d2bf35..60699294c 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -499,6 +499,13 @@ func SyncPullMirror(ctx context.Context, repoID int64) bool { theCommits.Commits = theCommits.Commits[:setting.UI.FeedMaxCommitNum] } + if newCommit, err := gitRepo.GetCommit(newCommitID); err != nil { + log.Error("SyncMirrors [repo: %-v]: unable to get commit %s: %v", m.Repo, newCommitID, err) + continue + } else { + theCommits.HeadCommit = repo_module.CommitToPushCommit(newCommit) + } + theCommits.CompareURL = m.Repo.ComposeCompareURL(oldCommitID, newCommitID) notification.NotifySyncPushCommits(ctx, m.Repo.MustOwner(ctx), m.Repo, &repo_module.PushUpdateOptions{ diff --git a/services/packages/container/blob_uploader.go b/services/packages/container/blob_uploader.go index ba92b0507..860672587 100644 --- a/services/packages/container/blob_uploader.go +++ b/services/packages/container/blob_uploader.go @@ -8,13 +8,13 @@ import ( "errors" "io" "os" - "path" "path/filepath" "strings" packages_model "code.gitea.io/gitea/models/packages" packages_module "code.gitea.io/gitea/modules/packages" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) var ( @@ -33,7 +33,7 @@ type BlobUploader struct { } func buildFilePath(id string) string { - return filepath.Join(setting.Packages.ChunkedUploadPath, path.Clean("/" + strings.ReplaceAll(id, "\\", "/"))[1:]) + return filepath.Join(setting.Packages.ChunkedUploadPath, util.CleanPath(strings.ReplaceAll(id, "\\", "/"))) } // NewBlobUploader creates a new blob uploader for the given id diff --git a/services/pull/comment.go b/services/pull/comment.go index 068aca6cd..933ad09a8 100644 --- a/services/pull/comment.go +++ b/services/pull/comment.go @@ -14,58 +14,6 @@ import ( issue_service "code.gitea.io/gitea/services/issue" ) -type commitBranchCheckItem struct { - Commit *git.Commit - Checked bool -} - -func commitBranchCheck(gitRepo *git.Repository, startCommit *git.Commit, endCommitID, baseBranch string, commitList map[string]*commitBranchCheckItem) error { - if startCommit.ID.String() == endCommitID { - return nil - } - - checkStack := make([]string, 0, 10) - checkStack = append(checkStack, startCommit.ID.String()) - - for len(checkStack) > 0 { - commitID := checkStack[0] - checkStack = checkStack[1:] - - item, ok := commitList[commitID] - if !ok { - continue - } - - if item.Commit.ID.String() == endCommitID { - continue - } - - if err := item.Commit.LoadBranchName(); err != nil { - return err - } - - if item.Commit.Branch == baseBranch { - continue - } - - if item.Checked { - continue - } - - item.Checked = true - - parentNum := item.Commit.ParentCount() - for i := 0; i < parentNum; i++ { - parentCommit, err := item.Commit.Parent(i) - if err != nil { - return err - } - checkStack = append(checkStack, parentCommit.ID.String()) - } - } - return nil -} - // getCommitIDsFromRepo get commit IDs from repo in between oldCommitID and newCommitID // isForcePush will be true if oldCommit isn't on the branch // Commit on baseBranch will skip @@ -82,47 +30,33 @@ func getCommitIDsFromRepo(ctx context.Context, repo *repo_model.Repository, oldC return nil, false, err } - if err = oldCommit.LoadBranchName(); err != nil { - return nil, false, err - } - - if len(oldCommit.Branch) == 0 { - commitIDs = make([]string, 2) - commitIDs[0] = oldCommitID - commitIDs[1] = newCommitID - - return commitIDs, true, err - } - newCommit, err := gitRepo.GetCommit(newCommitID) if err != nil { return nil, false, err } - commits, err := newCommit.CommitsBeforeUntil(oldCommitID) + isForcePush, err = newCommit.IsForcePush(oldCommitID) if err != nil { return nil, false, err } - commitIDs = make([]string, 0, len(commits)) - commitChecks := make(map[string]*commitBranchCheckItem) + if isForcePush { + commitIDs = make([]string, 2) + commitIDs[0] = oldCommitID + commitIDs[1] = newCommitID - for _, commit := range commits { - commitChecks[commit.ID.String()] = &commitBranchCheckItem{ - Commit: commit, - Checked: false, - } + return commitIDs, isForcePush, err } - if err = commitBranchCheck(gitRepo, newCommit, oldCommitID, baseBranch, commitChecks); err != nil { - return + // Find commits between new and old commit exclusing base branch commits + commits, err := gitRepo.CommitsBetweenNotBase(newCommit, oldCommit, baseBranch) + if err != nil { + return nil, false, err } + commitIDs = make([]string, 0, len(commits)) for i := len(commits) - 1; i >= 0; i-- { - commitID := commits[i].ID.String() - if item, ok := commitChecks[commitID]; ok && item.Checked { - commitIDs = append(commitIDs, commitID) - } + commitIDs = append(commitIDs, commits[i].ID.String()) } return commitIDs, isForcePush, err diff --git a/services/pull/lfs.go b/services/pull/lfs.go index dc4ca006e..394337246 100644 --- a/services/pull/lfs.go +++ b/services/pull/lfs.go @@ -116,7 +116,7 @@ func createLFSMetaObjectsFromCatFileBatch(catFileBatchReader *io.PipeReader, wg } // Then we need to check that this pointer is in the db - if _, err := git_model.GetLFSMetaObjectByOid(db.DefaultContext, pr.HeadRepo.ID, pointer.Oid); err != nil { + if _, err := git_model.GetLFSMetaObjectByOid(db.DefaultContext, pr.HeadRepoID, pointer.Oid); err != nil { if err == git_model.ErrLFSObjectNotExist { log.Warn("During merge of: %d in %-v, there is a pointer to LFS Oid: %s which although present in the LFS store is not associated with the head repo %-v", pr.Index, pr.BaseRepo, pointer.Oid, pr.HeadRepo) continue diff --git a/services/pull/merge.go b/services/pull/merge.go index ad428427c..12e01e8ce 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -5,8 +5,6 @@ package pull import ( - "bufio" - "bytes" "context" "fmt" "os" @@ -14,7 +12,6 @@ import ( "regexp" "strconv" "strings" - "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" @@ -34,22 +31,17 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" ) // GetDefaultMergeMessage returns default message used when merging pull request func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issues_model.PullRequest, mergeStyle repo_model.MergeStyle) (message, body string, err error) { - if err := pr.LoadHeadRepo(ctx); err != nil { - return "", "", err - } if err := pr.LoadBaseRepo(ctx); err != nil { return "", "", err } - if pr.BaseRepo == nil { - return "", "", repo_model.ErrRepoNotExist{ID: pr.BaseRepoID} + if err := pr.LoadHeadRepo(ctx); err != nil { + return "", "", err } - if err := pr.LoadIssue(ctx); err != nil { return "", "", err } @@ -144,18 +136,19 @@ func expandDefaultMergeMessage(template string, vars map[string]string) (message // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { - if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - return fmt.Errorf("LoadHeadRepo: %w", err) - } else if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return fmt.Errorf("LoadBaseRepo: %w", err) + if err := pr.LoadBaseRepo(ctx); err != nil { + log.Error("Unable to load base repo: %v", err) + return fmt.Errorf("unable to load base repo: %w", err) + } else if err := pr.LoadHeadRepo(ctx); err != nil { + log.Error("Unable to load head repo: %v", err) + return fmt.Errorf("unable to load head repo: %w", err) } pullWorkingPool.CheckIn(fmt.Sprint(pr.ID)) defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) // Removing an auto merge pull and ignore if not exist + // FIXME: is this the correct point to do this? Shouldn't this be after IsMergeStyleAllowed? if err := pull_model.DeleteScheduledAutoMerge(ctx, pr.ID); err != nil && !db.IsErrNotExist(err) { return err } @@ -179,7 +172,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U // Run the merge in the hammer context to prevent cancellation hammerCtx := graceful.GetManager().HammerContext() - pr.MergedCommitID, err = rawMerge(hammerCtx, pr, doer, mergeStyle, expectedHeadCommitID, message) + pr.MergedCommitID, err = doMergeAndPush(hammerCtx, pr, doer, mergeStyle, expectedHeadCommitID, message) if err != nil { return err } @@ -189,18 +182,18 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U pr.MergerID = doer.ID if _, err := pr.SetMerged(hammerCtx); err != nil { - log.Error("SetMerged [%d]: %v", pr.ID, err) + log.Error("SetMerged %-v: %v", pr, err) } if err := pr.LoadIssue(hammerCtx); err != nil { - log.Error("LoadIssue [%d]: %v", pr.ID, err) + log.Error("LoadIssue %-v: %v", pr, err) } if err := pr.Issue.LoadRepo(hammerCtx); err != nil { - log.Error("LoadRepo for issue [%d]: %v", pr.ID, err) + log.Error("pr.Issue.LoadRepo %-v: %v", pr, err) } if err := pr.Issue.Repo.LoadOwner(hammerCtx); err != nil { - log.Error("LoadOwner for PR [%d]: %v", pr.ID, err) + log.Error("LoadOwner for %-v: %v", pr, err) } if wasAutoMerged { @@ -239,326 +232,43 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U return nil } -// rawMerge perform the merge operation without changing any pull information in database -func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) { +// doMergeAndPush performs the merge operation without changing any pull information in database and pushes it up to the base repository +func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string) (string, error) { // Clone base repo. - tmpBasePath, err := createTemporaryRepo(ctx, pr) + mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, expectedHeadCommitID) if err != nil { - log.Error("CreateTemporaryPath: %v", err) return "", err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() - - baseBranch := "base" - trackingBranch := "tracking" - stagingBranch := "staging" - - if expectedHeadCommitID != "" { - trackingCommitID, _, err := git.NewCommand(ctx, "show-ref", "--hash").AddDynamicArguments(git.BranchPrefix + trackingBranch).RunStdString(&git.RunOpts{Dir: tmpBasePath}) - if err != nil { - log.Error("show-ref[%s] --hash refs/heads/trackingn: %v", tmpBasePath, git.BranchPrefix+trackingBranch, err) - return "", fmt.Errorf("getDiffTree: %w", err) - } - if strings.TrimSpace(trackingCommitID) != expectedHeadCommitID { - return "", models.ErrSHADoesNotMatch{ - GivenSHA: expectedHeadCommitID, - CurrentSHA: trackingCommitID, - } - } - } - - var outbuf, errbuf strings.Builder - - // Enable sparse-checkout - sparseCheckoutList, err := getDiffTree(ctx, tmpBasePath, baseBranch, trackingBranch) - if err != nil { - log.Error("getDiffTree(%s, %s, %s): %v", tmpBasePath, baseBranch, trackingBranch, err) - return "", fmt.Errorf("getDiffTree: %w", err) - } - - infoPath := filepath.Join(tmpBasePath, ".git", "info") - if err := os.MkdirAll(infoPath, 0o700); err != nil { - log.Error("Unable to create .git/info in %s: %v", tmpBasePath, err) - return "", fmt.Errorf("Unable to create .git/info in tmpBasePath: %w", err) - } - - sparseCheckoutListPath := filepath.Join(infoPath, "sparse-checkout") - if err := os.WriteFile(sparseCheckoutListPath, []byte(sparseCheckoutList), 0o600); err != nil { - log.Error("Unable to write .git/info/sparse-checkout file in %s: %v", tmpBasePath, err) - return "", fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) - } - - gitConfigCommand := func() *git.Command { - return git.NewCommand(ctx, "config", "--local") - } - - // Switch off LFS process (set required, clean and smudge here also) - if err := gitConfigCommand().AddArguments("filter.lfs.process", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.process -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.process -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.required", "false"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.required -> <false> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.required -> <false> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.clean", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.clean -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.clean -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("filter.lfs.smudge", ""). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [filter.lfs.smudge -> <> ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [filter.lfs.smudge -> <> ]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - if err := gitConfigCommand().AddArguments("core.sparseCheckout", "true"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git config [core.sparseCheckout -> true ]: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git config [core.sparsecheckout -> true]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Read base branch index - if err := git.NewCommand(ctx, "read-tree", "HEAD"). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git read-tree HEAD: %v\n%s\n%s", err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("Unable to read base branch in to the index: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - sig := doer.NewGitSig() - committer := sig - - // Determine if we should sign. If no signKeyID, use --no-gpg-sign to countermand the sign config (from gitconfig) - var signArgs git.TrustedCmdArgs - sign, signKeyID, signer, _ := asymkey_service.SignMerge(ctx, pr, doer, tmpBasePath, "HEAD", trackingBranch) - if sign { - if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { - committer = signer - } - signArgs = git.ToTrustedCmdArgs([]string{"-S" + signKeyID}) - } else { - signArgs = append(signArgs, "--no-gpg-sign") - } - - commitTimeStr := time.Now().Format(time.RFC3339) - - // Because this may call hooks we should pass in the environment - env := append(os.Environ(), - "GIT_AUTHOR_NAME="+sig.Name, - "GIT_AUTHOR_EMAIL="+sig.Email, - "GIT_AUTHOR_DATE="+commitTimeStr, - "GIT_COMMITTER_NAME="+committer.Name, - "GIT_COMMITTER_EMAIL="+committer.Email, - "GIT_COMMITTER_DATE="+commitTimeStr, - ) + defer cancel() // Merge commits. switch mergeStyle { case repo_model.MergeStyleMerge: - cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge tracking into base: %v", err) + if err := doMergeStyleMerge(mergeCtx, message); err != nil { return "", err } - - if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { - log.Error("Unable to make final commit: %v", err) + case repo_model.MergeStyleRebase, repo_model.MergeStyleRebaseMerge: + if err := doMergeStyleRebase(mergeCtx, mergeStyle, message); err != nil { return "", err } - case repo_model.MergeStyleRebase: - fallthrough - case repo_model.MergeStyleRebaseUpdate: - fallthrough - case repo_model.MergeStyleRebaseMerge: - // Checkout head branch - if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // Rebase before merging - if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - // Rebase will leave a REBASE_HEAD file in .git if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { - var commitSha string - ok := false - failingCommitPaths := []string{ - filepath.Join(tmpBasePath, ".git", "rebase-apply", "original-commit"), // Git < 2.26 - filepath.Join(tmpBasePath, ".git", "rebase-merge", "stopped-sha"), // Git >= 2.26 - } - for _, failingCommitPath := range failingCommitPaths { - if _, statErr := os.Stat(failingCommitPath); statErr == nil { - commitShaBytes, readErr := os.ReadFile(failingCommitPath) - if readErr != nil { - // Abandon this attempt to handle the error - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - commitSha = strings.TrimSpace(string(commitShaBytes)) - ok = true - break - } - } - if !ok { - log.Error("Unable to determine failing commit sha for this rebase message. Cannot cast as models.ErrRebaseConflicts.") - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - log.Debug("RebaseConflict at %s [%s:%s -> %s:%s]: %v\n%s\n%s", commitSha, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", models.ErrRebaseConflicts{ - Style: mergeStyle, - CommitSHA: commitSha, - StdOut: outbuf.String(), - StdErr: errbuf.String(), - Err: err, - } - } - log.Error("git rebase staging on to base [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git rebase staging on to base [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - // not need merge, just update by rebase. so skip - if mergeStyle == repo_model.MergeStyleRebaseUpdate { - break - } - - // Checkout base branch again - if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git checkout base prior to merge post staging rebase [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() - - cmd := git.NewCommand(ctx, "merge") - if mergeStyle == repo_model.MergeStyleRebase { - cmd.AddArguments("--ff-only") - } else { - cmd.AddArguments("--no-ff", "--no-commit") - } - cmd.AddDynamicArguments(stagingBranch) - - // Prepare merge with commit - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge staging into base: %v", err) - return "", err - } - if mergeStyle == repo_model.MergeStyleRebaseMerge { - if err := commitAndSignNoAuthor(ctx, pr, message, signArgs, tmpBasePath, env); err != nil { - log.Error("Unable to make final commit: %v", err) - return "", err - } - } case repo_model.MergeStyleSquash: - // Merge with squash - cmd := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) - if err := runMergeCommand(pr, mergeStyle, cmd, tmpBasePath); err != nil { - log.Error("Unable to merge --squash tracking into base: %v", err) + if err := doMergeStyleSquash(mergeCtx, message); err != nil { return "", err } - - if err = pr.Issue.LoadPoster(ctx); err != nil { - log.Error("LoadPoster: %v", err) - return "", fmt.Errorf("LoadPoster: %w", err) - } - sig := pr.Issue.Poster.NewGitSig() - if setting.Repository.PullRequest.AddCoCommitterTrailers && committer.String() != sig.String() { - // add trailer - message += fmt.Sprintf("\nCo-authored-by: %s\nCo-committed-by: %s\n", sig.String(), sig.String()) - } - if err := git.NewCommand(ctx, "commit"). - AddArguments(signArgs...). - AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). - AddOptionFormat("--message=%s", message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - } - outbuf.Reset() - errbuf.Reset() default: return "", models.ErrInvalidMergeStyle{ID: pr.BaseRepo.ID, Style: mergeStyle} } // OK we should cache our current head and origin/headbranch - mergeHeadSHA, err := git.GetFullCommitID(ctx, tmpBasePath, "HEAD") + mergeHeadSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "HEAD") if err != nil { return "", fmt.Errorf("Failed to get full commit id for HEAD: %w", err) } - mergeBaseSHA, err := git.GetFullCommitID(ctx, tmpBasePath, "original_"+baseBranch) + mergeBaseSHA, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, "original_"+baseBranch) if err != nil { return "", fmt.Errorf("Failed to get full commit id for origin/%s: %w", pr.BaseBranch, err) } - mergeCommitID, err := git.GetFullCommitID(ctx, tmpBasePath, baseBranch) + mergeCommitID, err := git.GetFullCommitID(ctx, mergeCtx.tmpBasePath, baseBranch) if err != nil { return "", fmt.Errorf("Failed to get full commit id for the new merge: %w", err) } @@ -567,7 +277,7 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode // I think in the interests of data safety - failures to push to the lfs should prevent // the merge as you can always remerge. if setting.LFS.StartServer { - if err := LFSPush(ctx, tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil { + if err := LFSPush(ctx, mergeCtx.tmpBasePath, mergeHeadSHA, mergeBaseSHA, pr); err != nil { return "", err } } @@ -576,167 +286,97 @@ func rawMerge(ctx context.Context, pr *issues_model.PullRequest, doer *user_mode err = pr.HeadRepo.LoadOwner(ctx) if err != nil { if !user_model.IsErrUserNotExist(err) { - log.Error("Can't find user: %d for head repository - %v", pr.HeadRepo.OwnerID, err) + log.Error("Can't find user: %d for head repository in %-v: %v", pr.HeadRepo.OwnerID, pr, err) return "", err } - log.Error("Can't find user: %d for head repository - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, doer.Name, err) + log.Warn("Can't find user: %d for head repository in %-v - defaulting to doer: %s - %v", pr.HeadRepo.OwnerID, pr, doer.Name, err) headUser = doer } else { headUser = pr.HeadRepo.Owner } - var pushCmd *git.Command - if mergeStyle == repo_model.MergeStyleRebaseUpdate { - // force push the rebase result to head branch - env = repo_module.FullPushingEnvironment( - headUser, - doer, - pr.HeadRepo, - pr.HeadRepo.Name, - pr.ID, - ) - pushCmd = git.NewCommand(ctx, "push", "-f", "head_repo").AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch) - } else { - env = repo_module.FullPushingEnvironment( - headUser, - doer, - pr.BaseRepo, - pr.BaseRepo.Name, - pr.ID, - ) - pushCmd = git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) - } + mergeCtx.env = repo_module.FullPushingEnvironment( + headUser, + doer, + pr.BaseRepo, + pr.BaseRepo.Name, + pr.ID, + ) + pushCmd := git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch) // Push back to upstream. // TODO: this cause an api call to "/api/internal/hook/post-receive/...", // that prevents us from doint the whole merge in one db transaction - if err := pushCmd.Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - if strings.Contains(errbuf.String(), "non-fast-forward") { + if err := pushCmd.Run(mergeCtx.RunOpts()); err != nil { + if strings.Contains(mergeCtx.errbuf.String(), "non-fast-forward") { return "", &git.ErrPushOutOfDate{ - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), Err: err, } - } else if strings.Contains(errbuf.String(), "! [remote rejected]") { + } else if strings.Contains(mergeCtx.errbuf.String(), "! [remote rejected]") { err := &git.ErrPushRejected{ - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), Err: err, } err.GenerateMessage() return "", err } - return "", fmt.Errorf("git push: %s", errbuf.String()) + return "", fmt.Errorf("git push: %s", mergeCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() return mergeCommitID, nil } -func commitAndSignNoAuthor(ctx context.Context, pr *issues_model.PullRequest, message string, signArgs git.TrustedCmdArgs, tmpBasePath string, env []string) error { - var outbuf, errbuf strings.Builder - if err := git.NewCommand(ctx, "commit").AddArguments(signArgs...).AddOptionFormat("--message=%s", message). - Run(&git.RunOpts{ - Env: env, - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("git commit [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) +func commitAndSignNoAuthor(ctx *mergeContext, message string) error { + cmdCommit := git.NewCommand(ctx, "commit").AddOptionFormat("--message=%s", message) + if ctx.signKeyID == "" { + cmdCommit.AddArguments("--no-gpg-sign") + } else { + cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID) + } + if err := cmdCommit.Run(ctx.RunOpts()); err != nil { + log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git commit %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) } return nil } -func runMergeCommand(pr *issues_model.PullRequest, mergeStyle repo_model.MergeStyle, cmd *git.Command, tmpBasePath string) error { - var outbuf, errbuf strings.Builder - if err := cmd.Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { +func runMergeCommand(ctx *mergeContext, mergeStyle repo_model.MergeStyle, cmd *git.Command) error { + if err := cmd.Run(ctx.RunOpts()); err != nil { // Merge will leave a MERGE_HEAD file in the .git folder if there is a conflict - if _, statErr := os.Stat(filepath.Join(tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { + if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "MERGE_HEAD")); statErr == nil { // We have a merge conflict error - log.Debug("MergeConflict [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + log.Debug("MergeConflict %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) return models.ErrMergeConflicts{ Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), Err: err, } - } else if strings.Contains(errbuf.String(), "refusing to merge unrelated histories") { - log.Debug("MergeUnrelatedHistories [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + } else if strings.Contains(ctx.errbuf.String(), "refusing to merge unrelated histories") { + log.Debug("MergeUnrelatedHistories %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) return models.ErrMergeUnrelatedHistories{ Style: mergeStyle, - StdOut: outbuf.String(), - StdErr: errbuf.String(), + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), Err: err, } } - log.Error("git merge [%s:%s -> %s:%s]: %v\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) - return fmt.Errorf("git merge [%s:%s -> %s:%s]: %w\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), pr.BaseBranch, err, outbuf.String(), errbuf.String()) + log.Error("git merge %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git merge %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) } + ctx.outbuf.Reset() + ctx.errbuf.Reset() return nil } var escapedSymbols = regexp.MustCompile(`([*[?! \\])`) -func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string) (string, error) { - getDiffTreeFromBranch := func(repoPath, baseBranch, headBranch string) (string, error) { - var outbuf, errbuf strings.Builder - // Compute the diff-tree for sparse-checkout - if err := git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). - Run(&git.RunOpts{ - Dir: repoPath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - return "", fmt.Errorf("git diff-tree [%s base:%s head:%s]: %s", repoPath, baseBranch, headBranch, errbuf.String()) - } - return outbuf.String(), nil - } - - scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { - if atEOF && len(data) == 0 { - return 0, nil, nil - } - if i := bytes.IndexByte(data, '\x00'); i >= 0 { - return i + 1, data[0:i], nil - } - if atEOF { - return len(data), data, nil - } - return 0, nil, nil - } - - list, err := getDiffTreeFromBranch(repoPath, baseBranch, headBranch) - if err != nil { - return "", err - } - - // Prefixing '/' for each entry, otherwise all files with the same name in subdirectories would be matched. - out := bytes.Buffer{} - scanner := bufio.NewScanner(strings.NewReader(list)) - scanner.Split(scanNullTerminatedStrings) - for scanner.Scan() { - filepath := scanner.Text() - // escape '*', '?', '[', spaces and '!' prefix - filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) - // no necessary to escape the first '#' symbol because the first symbol is '/' - fmt.Fprintf(&out, "/%s\n", filepath) - } - - return out.String(), nil -} - // IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p access_model.Permission, user *user_model.User) (bool, error) { if user == nil { diff --git a/services/pull/merge_merge.go b/services/pull/merge_merge.go new file mode 100644 index 000000000..0f7664297 --- /dev/null +++ b/services/pull/merge_merge.go @@ -0,0 +1,25 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" +) + +// doMergeStyleMerge merges the tracking into the current HEAD - which is assumed to tbe staging branch (equal to the pr.BaseBranch) +func doMergeStyleMerge(ctx *mergeContext, message string) error { + cmd := git.NewCommand(ctx, "merge", "--no-ff", "--no-commit").AddDynamicArguments(trackingBranch) + if err := runMergeCommand(ctx, repo_model.MergeStyleMerge, cmd); err != nil { + log.Error("%-v Unable to merge tracking into base: %v", ctx.pr, err) + return err + } + + if err := commitAndSignNoAuthor(ctx, message); err != nil { + log.Error("%-v Unable to make final commit: %v", ctx.pr, err) + return err + } + return nil +} diff --git a/services/pull/merge_prepare.go b/services/pull/merge_prepare.go new file mode 100644 index 000000000..88f6c037e --- /dev/null +++ b/services/pull/merge_prepare.go @@ -0,0 +1,288 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "bufio" + "bytes" + "context" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "code.gitea.io/gitea/models" + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + asymkey_service "code.gitea.io/gitea/services/asymkey" +) + +type mergeContext struct { + *prContext + doer *user_model.User + sig *git.Signature + committer *git.Signature + signKeyID string // empty for no-sign, non-empty to sign + 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.signKeyID = keyID + if pr.BaseRepo.GetTrustModel() == repo_model.CommitterTrustModel || pr.BaseRepo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel { + mergeCtx.committer = signer + } + } + + commitTimeStr := time.Now().Format(time.RFC3339) + + // Because this may call hooks we should pass in the environment + mergeCtx.env = append(os.Environ(), + "GIT_AUTHOR_NAME="+mergeCtx.sig.Name, + "GIT_AUTHOR_EMAIL="+mergeCtx.sig.Email, + "GIT_AUTHOR_DATE="+commitTimeStr, + "GIT_COMMITTER_NAME="+mergeCtx.committer.Name, + "GIT_COMMITTER_EMAIL="+mergeCtx.committer.Email, + "GIT_COMMITTER_DATE="+commitTimeStr, + ) + + return mergeCtx, cancel, nil +} + +// prepareTemporaryRepoForMerge takes a repository that has been created using createTemporaryRepo +// it then sets up the sparse-checkout and other things +func prepareTemporaryRepoForMerge(ctx *mergeContext) error { + infoPath := filepath.Join(ctx.tmpBasePath, ".git", "info") + if err := os.MkdirAll(infoPath, 0o700); err != nil { + log.Error("%-v Unable to create .git/info in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to create .git/info in tmpBasePath: %w", err) + } + + // Enable sparse-checkout + // Here we use the .git/info/sparse-checkout file as described in the git documentation + sparseCheckoutListFile, err := os.OpenFile(filepath.Join(infoPath, "sparse-checkout"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) + if err != nil { + log.Error("%-v Unable to write .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to write .git/info/sparse-checkout file in tmpBasePath: %w", err) + } + defer sparseCheckoutListFile.Close() // we will close it earlier but we need to ensure it is closed if there is an error + + if err := getDiffTree(ctx, ctx.tmpBasePath, baseBranch, trackingBranch, sparseCheckoutListFile); err != nil { + log.Error("%-v getDiffTree(%s, %s, %s): %v", ctx.pr, ctx.tmpBasePath, baseBranch, trackingBranch, err) + return fmt.Errorf("getDiffTree: %w", err) + } + + if err := sparseCheckoutListFile.Close(); err != nil { + log.Error("%-v Unable to close .git/info/sparse-checkout file in %s: %v", ctx.pr, ctx.tmpBasePath, err) + return fmt.Errorf("Unable to close .git/info/sparse-checkout file in tmpBasePath: %w", err) + } + + setConfig := func(key, value string) error { + if err := git.NewCommand(ctx, "config", "--local").AddDynamicArguments(key, value). + Run(ctx.RunOpts()); err != nil { + log.Error("git config [%s -> %q]: %v\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git config [%s -> %q]: %w\n%s\n%s", key, value, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + return nil + } + + // Switch off LFS process (set required, clean and smudge here also) + if err := setConfig("filter.lfs.process", ""); err != nil { + return err + } + + if err := setConfig("filter.lfs.required", "false"); err != nil { + return err + } + + if err := setConfig("filter.lfs.clean", ""); err != nil { + return err + } + + if err := setConfig("filter.lfs.smudge", ""); err != nil { + return err + } + + if err := setConfig("core.sparseCheckout", "true"); err != nil { + return err + } + + // Read base branch index + if err := git.NewCommand(ctx, "read-tree", "HEAD"). + Run(ctx.RunOpts()); err != nil { + log.Error("git read-tree HEAD: %v\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("Unable to read base branch in to the index: %w\n%s\n%s", err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + return nil +} + +// getDiffTree returns a string containing all the files that were changed between headBranch and baseBranch +// the filenames are escaped so as to fit the format required for .git/info/sparse-checkout +func getDiffTree(ctx context.Context, repoPath, baseBranch, headBranch string, out io.Writer) error { + diffOutReader, diffOutWriter, err := os.Pipe() + if err != nil { + log.Error("Unable to create os.Pipe for %s", repoPath) + return err + } + defer func() { + _ = diffOutReader.Close() + _ = diffOutWriter.Close() + }() + + scanNullTerminatedStrings := func(data []byte, atEOF bool) (advance int, token []byte, err error) { + if atEOF && len(data) == 0 { + return 0, nil, nil + } + if i := bytes.IndexByte(data, '\x00'); i >= 0 { + return i + 1, data[0:i], nil + } + if atEOF { + return len(data), data, nil + } + return 0, nil, nil + } + + err = git.NewCommand(ctx, "diff-tree", "--no-commit-id", "--name-only", "-r", "-r", "-z", "--root").AddDynamicArguments(baseBranch, headBranch). + Run(&git.RunOpts{ + Dir: repoPath, + Stdout: diffOutWriter, + PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { + // Close the writer end of the pipe to begin processing + _ = diffOutWriter.Close() + defer func() { + // Close the reader on return to terminate the git command if necessary + _ = diffOutReader.Close() + }() + + // Now scan the output from the command + scanner := bufio.NewScanner(diffOutReader) + scanner.Split(scanNullTerminatedStrings) + for scanner.Scan() { + filepath := scanner.Text() + // escape '*', '?', '[', spaces and '!' prefix + filepath = escapedSymbols.ReplaceAllString(filepath, `\$1`) + // no necessary to escape the first '#' symbol because the first symbol is '/' + fmt.Fprintf(out, "/%s\n", filepath) + } + return scanner.Err() + }, + }) + return err +} + +// rebaseTrackingOnToBase checks out the tracking branch as staging and rebases it on to the base branch +// if there is a conflict it will return a models.ErrRebaseConflicts +func rebaseTrackingOnToBase(ctx *mergeContext, mergeStyle repo_model.MergeStyle) error { + // Checkout head branch + if err := git.NewCommand(ctx, "checkout", "-b").AddDynamicArguments(stagingBranch, trackingBranch). + Run(ctx.RunOpts()); err != nil { + return fmt.Errorf("unable to git checkout tracking as staging in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + // Rebase before merging + if err := git.NewCommand(ctx, "rebase").AddDynamicArguments(baseBranch). + Run(ctx.RunOpts()); err != nil { + // Rebase will leave a REBASE_HEAD file in .git if there is a conflict + if _, statErr := os.Stat(filepath.Join(ctx.tmpBasePath, ".git", "REBASE_HEAD")); statErr == nil { + var commitSha string + ok := false + failingCommitPaths := []string{ + filepath.Join(ctx.tmpBasePath, ".git", "rebase-apply", "original-commit"), // Git < 2.26 + filepath.Join(ctx.tmpBasePath, ".git", "rebase-merge", "stopped-sha"), // Git >= 2.26 + } + for _, failingCommitPath := range failingCommitPaths { + if _, statErr := os.Stat(failingCommitPath); statErr == nil { + commitShaBytes, readErr := os.ReadFile(failingCommitPath) + if readErr != nil { + // Abandon this attempt to handle the error + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + commitSha = strings.TrimSpace(string(commitShaBytes)) + ok = true + break + } + } + if !ok { + log.Error("Unable to determine failing commit sha for failing rebase in temp repo for %-v. Cannot cast as models.ErrRebaseConflicts.", ctx.pr) + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + log.Debug("Conflict when rebasing staging on to base in %-v at %s: %v\n%s\n%s", ctx.pr, commitSha, err, ctx.outbuf.String(), ctx.errbuf.String()) + return models.ErrRebaseConflicts{ + CommitSHA: commitSha, + Style: mergeStyle, + StdOut: ctx.outbuf.String(), + StdErr: ctx.errbuf.String(), + Err: err, + } + } + return fmt.Errorf("unable to git rebase staging on to base in temp repo for %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return nil +} diff --git a/services/pull/merge_rebase.go b/services/pull/merge_rebase.go new file mode 100644 index 000000000..d3bb86d4a --- /dev/null +++ b/services/pull/merge_rebase.go @@ -0,0 +1,50 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "fmt" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" +) + +// doMergeStyleRebase rebaases the tracking branch on the base branch as the current HEAD with or with a merge commit to the original pr branch +func doMergeStyleRebase(ctx *mergeContext, mergeStyle repo_model.MergeStyle, message string) error { + if err := rebaseTrackingOnToBase(ctx, mergeStyle); err != nil { + return err + } + + // Checkout base branch again + if err := git.NewCommand(ctx, "checkout").AddDynamicArguments(baseBranch). + Run(ctx.RunOpts()); err != nil { + log.Error("git checkout base prior to merge post staging rebase %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git checkout base prior to merge post staging rebase %v: %w\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + + cmd := git.NewCommand(ctx, "merge") + if mergeStyle == repo_model.MergeStyleRebase { + cmd.AddArguments("--ff-only") + } else { + cmd.AddArguments("--no-ff", "--no-commit") + } + cmd.AddDynamicArguments(stagingBranch) + + // Prepare merge with commit + if err := runMergeCommand(ctx, mergeStyle, cmd); err != nil { + log.Error("Unable to merge staging into base: %v", err) + return err + } + if mergeStyle == repo_model.MergeStyleRebaseMerge { + if err := commitAndSignNoAuthor(ctx, message); err != nil { + log.Error("Unable to make final commit: %v", err) + return err + } + } + + return nil +} diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go new file mode 100644 index 000000000..f52a2301d --- /dev/null +++ b/services/pull/merge_squash.go @@ -0,0 +1,85 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "fmt" + + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" +) + +// doMergeStyleSquash gets a commit author signature for squash commits +func getAuthorSignatureSquash(ctx *mergeContext) (*git.Signature, error) { + if err := ctx.pr.Issue.LoadPoster(ctx); err != nil { + log.Error("%-v Issue[%d].LoadPoster: %v", ctx.pr, ctx.pr.Issue.ID, err) + return nil, err + } + + // Try to get an signature from the same user in one of the commits, as the + // poster email might be private or commits might have a different signature + // than the primary email address of the poster. + gitRepo, closer, err := git.RepositoryFromContextOrOpen(ctx, ctx.tmpBasePath) + if err != nil { + log.Error("%-v Unable to open base repository: %v", ctx.pr, err) + return nil, err + } + defer closer.Close() + + commits, err := gitRepo.CommitsBetweenIDs(trackingBranch, "HEAD") + if err != nil { + log.Error("%-v Unable to get commits between: %s %s: %v", ctx.pr, "HEAD", trackingBranch, err) + return nil, err + } + + uniqueEmails := make(container.Set[string]) + for _, commit := range commits { + if commit.Author != nil && uniqueEmails.Add(commit.Author.Email) { + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser != nil && commitUser.ID == ctx.pr.Issue.Poster.ID { + return commit.Author, nil + } + } + } + + return ctx.pr.Issue.Poster.NewGitSig(), nil +} + +// doMergeStyleSquash squashes the tracking branch on the current HEAD (=base) +func doMergeStyleSquash(ctx *mergeContext, message string) error { + sig, err := getAuthorSignatureSquash(ctx) + if err != nil { + return fmt.Errorf("getAuthorSignatureSquash: %w", err) + } + + cmdMerge := git.NewCommand(ctx, "merge", "--squash").AddDynamicArguments(trackingBranch) + if err := runMergeCommand(ctx, repo_model.MergeStyleSquash, cmdMerge); err != nil { + log.Error("%-v Unable to merge --squash tracking into base: %v", ctx.pr, err) + return err + } + + 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()) + } + cmdCommit := git.NewCommand(ctx, "commit"). + AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). + AddOptionFormat("--message=%s", message) + if ctx.signKeyID == "" { + cmdCommit.AddArguments("--no-gpg-sign") + } else { + cmdCommit.AddOptionFormat("-S%s", ctx.signKeyID) + } + if err := cmdCommit.Run(ctx.RunOpts()); err != nil { + log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), ctx.errbuf.String()) + return fmt.Errorf("git commit [%s:%s -> %s:%s]: %w\n%s\n%s", ctx.pr.HeadRepo.FullName(), ctx.pr.HeadBranch, ctx.pr.BaseRepo.FullName(), ctx.pr.BaseBranch, err, ctx.outbuf.String(), ctx.errbuf.String()) + } + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return nil +} diff --git a/services/pull/patch.go b/services/pull/patch.go index c2ccc75bd..927735572 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -22,7 +22,6 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/process" - repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" @@ -64,25 +63,21 @@ func TestPatch(pr *issues_model.PullRequest) error { defer finished() // Clone base repo. - tmpBasePath, err := createTemporaryRepo(ctx, pr) + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { - log.Error("CreateTemporaryPath: %v", err) + log.Error("createTemporaryRepoForPR %-v: %v", pr, err) return err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - gitRepo, err := git.OpenRepository(ctx, tmpBasePath) + gitRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) if err != nil { return fmt.Errorf("OpenRepository: %w", err) } defer gitRepo.Close() // 1. update merge base - pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base", "--", "base", "tracking").RunStdString(&git.RunOpts{Dir: tmpBasePath}) + pr.MergeBase, _, err = git.NewCommand(ctx, "merge-base", "--", "base", "tracking").RunStdString(&git.RunOpts{Dir: prCtx.tmpBasePath}) if err != nil { var err2 error pr.MergeBase, err2 = gitRepo.GetRefCommitID(git.BranchPrefix + "base") @@ -101,7 +96,7 @@ func TestPatch(pr *issues_model.PullRequest) error { } // 2. Check for conflicts - if conflicts, err := checkConflicts(ctx, pr, gitRepo, tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty { + if conflicts, err := checkConflicts(ctx, pr, gitRepo, prCtx.tmpBasePath); err != nil || conflicts || pr.Status == issues_model.PullRequestStatusEmpty { return err } diff --git a/services/pull/pull.go b/services/pull/pull.go index 0d260c93b..e40e59a2c 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -257,7 +257,7 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, // If you don't let it run all the way then you will lose data // TODO: graceful: AddTestPullRequestTask needs to become a queue! - prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch) + prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, true) if err != nil { log.Error("Find pull requests [head_repo_id: %d, head_branch: %s]: %v", repoID, branch, err) return @@ -349,18 +349,14 @@ func AddTestPullRequestTask(doer *user_model.User, repoID int64, branch string, // checkIfPRContentChanged checks if diff to target branch has changed by push // A commit can be considered to leave the PR untouched if the patch/diff with its merge base is unchanged func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, oldCommitID, newCommitID string) (hasChanged bool, err error) { - tmpBasePath, err := createTemporaryRepo(ctx, pr) + prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr) if err != nil { - log.Error("CreateTemporaryRepo: %v", err) + log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) return false, err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("checkIfPRContentChanged: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - tmpRepo, err := git.OpenRepository(ctx, tmpBasePath) + tmpRepo, err := git.OpenRepository(ctx, prCtx.tmpBasePath) if err != nil { return false, fmt.Errorf("OpenRepository: %w", err) } @@ -379,7 +375,7 @@ func checkIfPRContentChanged(ctx context.Context, pr *issues_model.PullRequest, } if err := cmd.Run(&git.RunOpts{ - Dir: tmpBasePath, + Dir: prCtx.tmpBasePath, Stdout: stdoutWriter, PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() @@ -500,7 +496,7 @@ func (errs errlist) Error() string { // CloseBranchPulls close all the pull requests who's head branch is the branch func CloseBranchPulls(doer *user_model.User, repoID int64, branch string) error { - prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch) + prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repoID, branch, false) if err != nil { return err } @@ -536,7 +532,7 @@ func CloseRepoBranchesPulls(ctx context.Context, doer *user_model.User, repo *re var errs errlist for _, branch := range branches { - prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name) + prs, err := issues_model.GetUnmergedPullRequestsByHeadInfo(repo.ID, branch.Name, false) if err != nil { return err } @@ -673,7 +669,12 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { - authors = append(authors, authorString) + // Compare use account as well to avoid adding the same author multiple times + // times when email addresses are private or multiple emails are used. + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID { + authors = append(authors, authorString) + } } } @@ -694,7 +695,10 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ for _, commit := range commits { authorString := commit.Author.String() if uniqueAuthors.Add(authorString) && authorString != posterSig { - authors = append(authors, authorString) + commitUser, _ := user_model.GetUserByEmail(ctx, commit.Author.Email) + if commitUser == nil || commitUser.ID != pr.Issue.Poster.ID { + authors = append(authors, authorString) + } } } skip += limit diff --git a/services/pull/review.go b/services/pull/review.go index ca386ca6b..ba93b5e2f 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -71,7 +71,7 @@ func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestLis } // CreateCodeComment creates a comment on the code line -func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, isReview bool, replyReviewID int64, latestCommitID string) (*issues_model.Comment, error) { +func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, pendingReview bool, replyReviewID int64, latestCommitID string) (*issues_model.Comment, error) { var ( existsReview bool err error @@ -82,7 +82,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git. // - Comments that are part of a review // - Comments that reply to an existing review - if !isReview && replyReviewID != 0 { + if !pendingReview && replyReviewID != 0 { // It's not part of a review; maybe a reply to a review comment or a single comment. // Check if there are reviews for that line already; if there are, this is a reply if existsReview, err = issues_model.ReviewExists(issue, treePath, line); err != nil { @@ -91,7 +91,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git. } // Comments that are replies don't require a review header to show up in the issue view - if !isReview && existsReview { + if !pendingReview && existsReview { if err = issue.LoadRepo(ctx); err != nil { return nil, err } @@ -149,7 +149,7 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git. return nil, err } - if !isReview && !existsReview { + if !pendingReview && !existsReview { // Submit the review we've just created so the comment shows up in the issue view if _, _, err = SubmitReview(ctx, doer, gitRepo, issue, issues_model.ReviewTypeComment, "", latestCommitID, nil); err != nil { return nil, err diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index e0d6b4a15..146470780 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -19,147 +19,153 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" ) -// createTemporaryRepo creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch +// Temporary repos created here use standard branch names to help simplify +// merging code +const ( + baseBranch = "base" // equivalent to pr.BaseBranch + trackingBranch = "tracking" // equivalent to pr.HeadBranch + stagingBranch = "staging" // this is used for a working branch +) + +type prContext struct { + context.Context + tmpBasePath string + pr *issues_model.PullRequest + outbuf *strings.Builder // we keep these around to help reduce needless buffer recreation, + errbuf *strings.Builder // any use should be preceded by a Reset and preferably after use +} + +func (ctx *prContext) RunOpts() *git.RunOpts { + ctx.outbuf.Reset() + ctx.errbuf.Reset() + return &git.RunOpts{ + Dir: ctx.tmpBasePath, + Stdout: ctx.outbuf, + Stderr: ctx.errbuf, + } +} + +// createTemporaryRepoForPR creates a temporary repo with "base" for pr.BaseBranch and "tracking" for pr.HeadBranch // it also create a second base branch called "original_base" -func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (string, error) { +func createTemporaryRepoForPR(ctx context.Context, pr *issues_model.PullRequest) (prCtx *prContext, cancel context.CancelFunc, err error) { if err := pr.LoadHeadRepo(ctx); err != nil { - log.Error("LoadHeadRepo: %v", err) - return "", fmt.Errorf("LoadHeadRepo: %w", err) + log.Error("%-v LoadHeadRepo: %v", pr, err) + return nil, nil, fmt.Errorf("%v LoadHeadRepo: %w", pr, err) } else if pr.HeadRepo == nil { - log.Error("Pr %d HeadRepo %d does not exist", pr.ID, pr.HeadRepoID) - return "", &repo_model.ErrRepoNotExist{ + log.Error("%-v HeadRepo %d does not exist", pr, pr.HeadRepoID) + return nil, nil, &repo_model.ErrRepoNotExist{ ID: pr.HeadRepoID, } } else if err := pr.LoadBaseRepo(ctx); err != nil { - log.Error("LoadBaseRepo: %v", err) - return "", fmt.Errorf("LoadBaseRepo: %w", err) + log.Error("%-v LoadBaseRepo: %v", pr, err) + return nil, nil, fmt.Errorf("%v LoadBaseRepo: %w", pr, err) } else if pr.BaseRepo == nil { - log.Error("Pr %d BaseRepo %d does not exist", pr.ID, pr.BaseRepoID) - return "", &repo_model.ErrRepoNotExist{ + log.Error("%-v BaseRepo %d does not exist", pr, pr.BaseRepoID) + return nil, nil, &repo_model.ErrRepoNotExist{ ID: pr.BaseRepoID, } } else if err := pr.HeadRepo.LoadOwner(ctx); err != nil { - log.Error("HeadRepo.LoadOwner: %v", err) - return "", fmt.Errorf("HeadRepo.LoadOwner: %w", err) + log.Error("%-v HeadRepo.LoadOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v HeadRepo.LoadOwner: %w", pr, err) } else if err := pr.BaseRepo.LoadOwner(ctx); err != nil { - log.Error("BaseRepo.LoadOwner: %v", err) - return "", fmt.Errorf("BaseRepo.LoadOwner: %w", err) + log.Error("%-v BaseRepo.LoadOwner: %v", pr, err) + return nil, nil, fmt.Errorf("%v BaseRepo.LoadOwner: %w", pr, err) } // Clone base repo. tmpBasePath, err := repo_module.CreateTemporaryPath("pull") if err != nil { - log.Error("CreateTemporaryPath: %v", err) - return "", err + log.Error("CreateTemporaryPath[%-v]: %v", pr, err) + return nil, nil, err + } + prCtx = &prContext{ + Context: ctx, + tmpBasePath: tmpBasePath, + pr: pr, + outbuf: &strings.Builder{}, + errbuf: &strings.Builder{}, + } + cancel = func() { + if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { + log.Error("Error whilst removing removing temporary repo for %-v: %v", pr, err) + } } baseRepoPath := pr.BaseRepo.RepoPath() headRepoPath := pr.HeadRepo.RepoPath() if err := git.InitRepository(ctx, tmpBasePath, false); err != nil { - log.Error("git init tmpBasePath: %v", err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", err + log.Error("Unable to init tmpBasePath for %-v: %v", pr, err) + cancel() + return nil, nil, err } remoteRepoName := "head_repo" baseBranch := "base" - // Add head repo remote. - addCacheRepo := func(staging, cache string) error { - p := filepath.Join(staging, ".git", "objects", "info", "alternates") + fetchArgs := git.TrustedCmdArgs{"--no-tags"} + if git.CheckGitVersionAtLeast("2.25.0") == nil { + // Writing the commit graph can be slow and is not needed here + fetchArgs = append(fetchArgs, "--no-write-commit-graph") + } + + // addCacheRepo adds git alternatives for the cacheRepoPath in the repoPath + addCacheRepo := func(repoPath, cacheRepoPath string) error { + p := filepath.Join(repoPath, ".git", "objects", "info", "alternates") f, err := os.OpenFile(p, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) if err != nil { - log.Error("Could not create .git/objects/info/alternates file in %s: %v", staging, err) + log.Error("Could not create .git/objects/info/alternates file in %s: %v", repoPath, err) return err } defer f.Close() - data := filepath.Join(cache, "objects") + data := filepath.Join(cacheRepoPath, "objects") if _, err := fmt.Fprintln(f, data); err != nil { - log.Error("Could not write to .git/objects/info/alternates file in %s: %v", staging, err) + log.Error("Could not write to .git/objects/info/alternates file in %s: %v", repoPath, err) return err } return nil } + // Add head repo remote. if err := addCacheRepo(tmpBasePath, baseRepoPath); err != nil { - log.Error("Unable to add base repository to temporary repo [%s -> %s]: %v", pr.BaseRepo.FullName(), tmpBasePath, err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err) + log.Error("%-v Unable to add base repository to temporary repo [%s -> %s]: %v", pr, pr.BaseRepo.FullName(), tmpBasePath, err) + cancel() + return nil, nil, fmt.Errorf("Unable to add base repository to temporary repo [%s -> tmpBasePath]: %w", pr.BaseRepo.FullName(), err) } - var outbuf, errbuf strings.Builder if err := git.NewCommand(ctx, "remote", "add", "-t").AddDynamicArguments(pr.BaseBranch).AddArguments("-m").AddDynamicArguments(pr.BaseBranch).AddDynamicArguments("origin", baseRepoPath). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to add base repository as origin [%s -> %s]: %v\n%s\n%s", pr.BaseRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add base repository as origin [%s -> tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), err, outbuf.String(), errbuf.String()) - } - 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("%-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("%-v Unable to fetch origin base branch [%s:%s -> base, original_base in %s]: %v:\n%s\n%s", pr, pr.BaseRepo.FullName(), pr.BaseBranch, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to fetch origin base branch [%s:%s -> base, original_base in tmpBasePath]: %w\n%s\n%s", pr.BaseRepo.FullName(), pr.BaseBranch, err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() if err := git.NewCommand(ctx, "symbolic-ref").AddDynamicArguments("HEAD", git.BranchPrefix+baseBranch). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to set HEAD as base branch [%s]: %v\n%s\n%s", tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to set HEAD as base branch [tmpBasePath]: %w\n%s\n%s", err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to set HEAD as base branch in [%s]: %v\n%s\n%s", pr, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to set HEAD as base branch in tmpBasePath: %w\n%s\n%s", err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() if err := addCacheRepo(tmpBasePath, headRepoPath); err != nil { - log.Error("Unable to add head repository to temporary repo [%s -> %s]: %v", pr.HeadRepo.FullName(), tmpBasePath, err) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err) + log.Error("%-v Unable to add head repository to temporary repo [%s -> %s]: %v", pr, pr.HeadRepo.FullName(), tmpBasePath, err) + cancel() + return nil, nil, fmt.Errorf("Unable to add head base repository to temporary repo [%s -> tmpBasePath]: %w", pr.HeadRepo.FullName(), err) } if err := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteRepoName, headRepoPath). - Run(&git.RunOpts{ - Dir: tmpBasePath, - Stdout: &outbuf, - Stderr: &errbuf, - }); err != nil { - log.Error("Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr.HeadRepo.FullName(), tmpBasePath, err, outbuf.String(), errbuf.String()) - if err := repo_module.RemoveTemporaryPath(tmpBasePath); err != nil { - log.Error("CreateTempRepo: RemoveTemporaryPath: %s", err) - } - return "", fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, outbuf.String(), errbuf.String()) + Run(prCtx.RunOpts()); err != nil { + log.Error("%-v Unable to add head repository as head_repo [%s -> %s]: %v\n%s\n%s", pr, pr.HeadRepo.FullName(), tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + cancel() + return nil, nil, fmt.Errorf("Unable to add head repository as head_repo [%s -> tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() trackingBranch := "tracking" // Fetch head branch @@ -171,25 +177,19 @@ func createTemporaryRepo(ctx context.Context, pr *issues_model.PullRequest) (str } else { 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) - } + if err := git.NewCommand(ctx, "fetch").AddArguments(fetchArgs...).AddDynamicArguments(remoteRepoName, headBranch+":"+trackingBranch). + Run(prCtx.RunOpts()); err != nil { + cancel() if !git.IsBranchExist(ctx, pr.HeadRepo.RepoPath(), pr.HeadBranch) { - return "", models.ErrBranchDoesNotExist{ + return nil, nil, models.ErrBranchDoesNotExist{ BranchName: pr.HeadBranch, } } - log.Error("Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, outbuf.String(), errbuf.String()) - return "", fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, outbuf.String(), errbuf.String()) + log.Error("%-v Unable to fetch head_repo head branch [%s:%s -> tracking in %s]: %v:\n%s\n%s", pr, pr.HeadRepo.FullName(), pr.HeadBranch, tmpBasePath, err, prCtx.outbuf.String(), prCtx.errbuf.String()) + return nil, nil, fmt.Errorf("Unable to fetch head_repo head branch [%s:%s -> tracking in tmpBasePath]: %w\n%s\n%s", pr.HeadRepo.FullName(), headBranch, err, prCtx.outbuf.String(), prCtx.errbuf.String()) } - outbuf.Reset() - errbuf.Reset() + prCtx.outbuf.Reset() + prCtx.errbuf.Reset() - return tmpBasePath, nil + return prCtx, cancel, nil } diff --git a/services/pull/update.go b/services/pull/update.go index b9525cf0c..b977dbdba 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -16,61 +16,67 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - repo_module "code.gitea.io/gitea/modules/repository" ) // Update updates pull request with base branch. -func Update(ctx context.Context, pull *issues_model.PullRequest, doer *user_model.User, message string, rebase bool) error { - var ( - pr *issues_model.PullRequest - style repo_model.MergeStyle - ) +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) + log.Error("CreateTemporaryRepoForPR %-v: %v", pr, err) } return nil, err } - defer func() { - if err := repo_module.RemoveTemporaryPath(tmpRepo); err != nil { - log.Error("Merge: RemoveTemporaryPath: %s", err) - } - }() + defer cancel() - diff, err := git.GetDivergingCommits(ctx, tmpRepo, "base", "tracking") + diff, err := git.GetDivergingCommits(ctx, prCtx.tmpBasePath, baseBranch, trackingBranch) return &diff, err } diff --git a/services/pull/update_rebase.go b/services/pull/update_rebase.go new file mode 100644 index 000000000..8e7bfa0ff --- /dev/null +++ b/services/pull/update_rebase.go @@ -0,0 +1,107 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + "fmt" + "strings" + + issues_model "code.gitea.io/gitea/models/issues" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + repo_module "code.gitea.io/gitea/modules/repository" + "code.gitea.io/gitea/modules/setting" +) + +// updateHeadByRebaseOnToBase handles updating a PR's head branch by rebasing it on the PR current base branch +func updateHeadByRebaseOnToBase(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, message string) error { + // "Clone" base repo and add the cache headers for the head repo and branch + mergeCtx, cancel, err := createTemporaryRepoForMerge(ctx, pr, doer, "") + if err != nil { + return err + } + defer cancel() + + // Determine the old merge-base before the rebase - we use this for LFS push later on + oldMergeBase, _, _ := git.NewCommand(ctx, "merge-base").AddDashesAndList(baseBranch, trackingBranch).RunStdString(&git.RunOpts{Dir: mergeCtx.tmpBasePath}) + oldMergeBase = strings.TrimSpace(oldMergeBase) + + // Rebase the tracking branch on to the base as the staging branch + if err := rebaseTrackingOnToBase(mergeCtx, repo_model.MergeStyleRebaseUpdate); err != nil { + return err + } + + if setting.LFS.StartServer { + // Now we need to ensure that the head repository contains any LFS objects between the new base and the old mergebase + // It's questionable about where this should go - either after or before the push + // I think in the interests of data safety - failures to push to the lfs should prevent + // the push as you can always re-rebase. + if err := LFSPush(ctx, mergeCtx.tmpBasePath, baseBranch, oldMergeBase, &issues_model.PullRequest{ + HeadRepoID: pr.BaseRepoID, + BaseRepoID: pr.HeadRepoID, + }); err != nil { + log.Error("Unable to push lfs objects between %s and %s up to head branch in %-v: %v", baseBranch, oldMergeBase, pr, err) + return err + } + } + + // Now determine who the pushing author should be + var headUser *user_model.User + if err := pr.HeadRepo.LoadOwner(ctx); err != nil { + if !user_model.IsErrUserNotExist(err) { + log.Error("Can't find user: %d for head repository in %-v - %v", pr.HeadRepo.OwnerID, pr, err) + return err + } + log.Error("Can't find user: %d for head repository in %-v - defaulting to doer: %-v - %v", pr.HeadRepo.OwnerID, pr, doer, err) + headUser = doer + } else { + headUser = pr.HeadRepo.Owner + } + + pushCmd := git.NewCommand(ctx, "push", "-f", "head_repo"). + AddDynamicArguments(stagingBranch + ":" + git.BranchPrefix + pr.HeadBranch) + + // Push back to the head repository. + // TODO: this cause an api call to "/api/internal/hook/post-receive/...", + // that prevents us from doint the whole merge in one db transaction + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() + + if err := pushCmd.Run(&git.RunOpts{ + Env: repo_module.FullPushingEnvironment( + headUser, + doer, + pr.HeadRepo, + pr.HeadRepo.Name, + pr.ID, + ), + Dir: mergeCtx.tmpBasePath, + Stdout: mergeCtx.outbuf, + Stderr: mergeCtx.errbuf, + }); err != nil { + if strings.Contains(mergeCtx.errbuf.String(), "non-fast-forward") { + return &git.ErrPushOutOfDate{ + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), + Err: err, + } + } else if strings.Contains(mergeCtx.errbuf.String(), "! [remote rejected]") { + err := &git.ErrPushRejected{ + StdOut: mergeCtx.outbuf.String(), + StdErr: mergeCtx.errbuf.String(), + Err: err, + } + err.GenerateMessage() + return err + } + return fmt.Errorf("git push: %s", mergeCtx.errbuf.String()) + } + mergeCtx.outbuf.Reset() + mergeCtx.errbuf.Reset() + + return nil +} diff --git a/services/release/release_test.go b/services/release/release_test.go index 9b8aaa364..805269413 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -106,11 +106,13 @@ func TestRelease_Create(t *testing.T) { IsTag: false, }, nil, "")) + testPlayload := "testtest" + attach, err := attachment.NewAttachment(&repo_model.Attachment{ RepoID: repo.ID, UploaderID: user.ID, Name: "test.txt", - }, strings.NewReader("testtest")) + }, strings.NewReader(testPlayload), int64(len([]byte(testPlayload)))) assert.NoError(t, err) release := repo_model.Release{ @@ -239,11 +241,12 @@ func TestRelease_Update(t *testing.T) { assert.Equal(t, tagName, release.TagName) // Add new attachments + samplePayload := "testtest" attach, err := attachment.NewAttachment(&repo_model.Attachment{ RepoID: repo.ID, UploaderID: user.ID, Name: "test.txt", - }, strings.NewReader("testtest")) + }, strings.NewReader(samplePayload), int64(len([]byte(samplePayload)))) assert.NoError(t, err) assert.NoError(t, UpdateRelease(user, gitRepo, release, []string{attach.UUID}, nil, nil)) diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 280c4cc03..94b2c3f3d 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -26,7 +26,7 @@ import ( ) // AdoptRepository adopts pre-existing repository files for the user/organization. -func AdoptRepository(doer, u *user_model.User, opts repo_module.CreateRepoOptions) (*repo_model.Repository, error) { +func AdoptRepository(ctx context.Context, doer, u *user_model.User, opts repo_module.CreateRepoOptions) (*repo_model.Repository, error) { if !doer.IsAdmin && !u.CanCreateRepo() { return nil, repo_model.ErrReachLimitOfRepo{ Limit: u.MaxRepoCreation, @@ -53,7 +53,7 @@ func AdoptRepository(doer, u *user_model.User, opts repo_module.CreateRepoOption IsEmpty: !opts.AutoInit, } - if err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { + if err := db.WithTx(ctx, func(ctx context.Context) error { repoPath := repo_model.RepoPath(u.Name, repo.Name) isExist, err := util.IsExist(repoPath) if err != nil { @@ -95,7 +95,7 @@ func AdoptRepository(doer, u *user_model.User, opts repo_module.CreateRepoOption return nil, err } - notification.NotifyCreateRepository(db.DefaultContext, doer, u, repo) + notification.NotifyCreateRepository(ctx, doer, u, repo) return repo, nil } @@ -188,7 +188,7 @@ func adoptRepository(ctx context.Context, repoPath string, u *user_model.User, r } // DeleteUnadoptedRepository deletes unadopted repository files from the filesystem -func DeleteUnadoptedRepository(doer, u *user_model.User, repoName string) error { +func DeleteUnadoptedRepository(ctx context.Context, doer, u *user_model.User, repoName string) error { if err := repo_model.IsUsableRepoName(repoName); err != nil { return err } @@ -206,7 +206,7 @@ func DeleteUnadoptedRepository(doer, u *user_model.User, repoName string) error } } - if exist, err := repo_model.IsRepositoryExist(db.DefaultContext, u, repoName); err != nil { + if exist, err := repo_model.IsRepositoryExist(ctx, u, repoName); err != nil { return err } else if exist { return repo_model.ErrRepoAlreadyExist{ @@ -232,11 +232,11 @@ func (unadopted *unadoptedRepositories) add(repository string) { unadopted.index++ } -func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unadopted *unadoptedRepositories) error { +func checkUnadoptedRepositories(ctx context.Context, userName string, repoNamesToCheck []string, unadopted *unadoptedRepositories) error { if len(repoNamesToCheck) == 0 { return nil } - ctxUser, err := user_model.GetUserByName(db.DefaultContext, userName) + ctxUser, err := user_model.GetUserByName(ctx, userName) if err != nil { if user_model.IsErrUserNotExist(err) { log.Debug("Missing user: %s", userName) @@ -271,7 +271,7 @@ func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unad } // ListUnadoptedRepositories lists all the unadopted repositories that match the provided query -func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, int, error) { +func ListUnadoptedRepositories(ctx context.Context, query string, opts *db.ListOptions) ([]string, int, error) { globUser, _ := glob.Compile("*") globRepo, _ := glob.Compile("*") @@ -315,7 +315,7 @@ func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, in if !strings.ContainsRune(path[len(root)+1:], filepath.Separator) { // Got a new user - if err = checkUnadoptedRepositories(userName, repoNamesToCheck, unadopted); err != nil { + if err = checkUnadoptedRepositories(ctx, userName, repoNamesToCheck, unadopted); err != nil { return err } repoNamesToCheck = repoNamesToCheck[:0] @@ -338,7 +338,7 @@ func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, in repoNamesToCheck = append(repoNamesToCheck, name) if len(repoNamesToCheck) >= setting.Database.IterateBufferSize { - if err = checkUnadoptedRepositories(userName, repoNamesToCheck, unadopted); err != nil { + if err = checkUnadoptedRepositories(ctx, userName, repoNamesToCheck, unadopted); err != nil { return err } repoNamesToCheck = repoNamesToCheck[:0] @@ -349,7 +349,7 @@ func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, in return nil, 0, err } - if err := checkUnadoptedRepositories(userName, repoNamesToCheck, unadopted); err != nil { + if err := checkUnadoptedRepositories(ctx, userName, repoNamesToCheck, unadopted); err != nil { return nil, 0, err } diff --git a/services/repository/adopt_test.go b/services/repository/adopt_test.go index be8897693..3f777c425 100644 --- a/services/repository/adopt_test.go +++ b/services/repository/adopt_test.go @@ -39,7 +39,7 @@ func TestCheckUnadoptedRepositories(t *testing.T) { // Non existent user // unadopted := &unadoptedRepositories{start: 0, end: 100} - err := checkUnadoptedRepositories("notauser", []string{"repo"}, unadopted) + err := checkUnadoptedRepositories(db.DefaultContext, "notauser", []string{"repo"}, unadopted) assert.NoError(t, err) assert.Equal(t, 0, len(unadopted.repositories)) // @@ -50,14 +50,14 @@ func TestCheckUnadoptedRepositories(t *testing.T) { repoName := "repo2" unadoptedRepoName := "unadopted" unadopted = &unadoptedRepositories{start: 0, end: 100} - err = checkUnadoptedRepositories(userName, []string{repoName, unadoptedRepoName}, unadopted) + err = checkUnadoptedRepositories(db.DefaultContext, userName, []string{repoName, unadoptedRepoName}, unadopted) assert.NoError(t, err) assert.Equal(t, []string{path.Join(userName, unadoptedRepoName)}, unadopted.repositories) // // Existing (adopted) repository is not returned // unadopted = &unadoptedRepositories{start: 0, end: 100} - err = checkUnadoptedRepositories(userName, []string{repoName}, unadopted) + err = checkUnadoptedRepositories(db.DefaultContext, userName, []string{repoName}, unadopted) assert.NoError(t, err) assert.Equal(t, 0, len(unadopted.repositories)) assert.Equal(t, 0, unadopted.index) @@ -72,13 +72,13 @@ func TestListUnadoptedRepositories_ListOptions(t *testing.T) { } opts := db.ListOptions{Page: 1, PageSize: 1} - repoNames, count, err := ListUnadoptedRepositories("", &opts) + repoNames, count, err := ListUnadoptedRepositories(db.DefaultContext, "", &opts) assert.NoError(t, err) assert.Equal(t, 2, count) assert.Equal(t, unadoptedList[0], repoNames[0]) opts = db.ListOptions{Page: 2, PageSize: 1} - repoNames, count, err = ListUnadoptedRepositories("", &opts) + repoNames, count, err = ListUnadoptedRepositories(db.DefaultContext, "", &opts) assert.NoError(t, err) assert.Equal(t, 2, count) assert.Equal(t, unadoptedList[1], repoNames[0]) diff --git a/services/repository/avatar.go b/services/repository/avatar.go index 5fe8bd2c7..74e5de877 100644 --- a/services/repository/avatar.go +++ b/services/repository/avatar.go @@ -20,7 +20,7 @@ import ( // UploadAvatar saves custom avatar for repository. // FIXME: split uploads to different subdirs in case we have massive number of repos. -func UploadAvatar(repo *repo_model.Repository, data []byte) error { +func UploadAvatar(ctx context.Context, repo *repo_model.Repository, data []byte) error { m, err := avatar.Prepare(data) if err != nil { return err @@ -31,7 +31,7 @@ func UploadAvatar(repo *repo_model.Repository, data []byte) error { return nil } - ctx, committer, err := db.TxContext(db.DefaultContext) + ctx, committer, err := db.TxContext(ctx) if err != nil { return err } @@ -65,7 +65,7 @@ func UploadAvatar(repo *repo_model.Repository, data []byte) error { } // DeleteAvatar deletes the repos's custom avatar. -func DeleteAvatar(repo *repo_model.Repository) error { +func DeleteAvatar(ctx context.Context, repo *repo_model.Repository) error { // Avatar not exists if len(repo.Avatar) == 0 { return nil @@ -74,7 +74,7 @@ func DeleteAvatar(repo *repo_model.Repository) error { avatarPath := repo.CustomAvatarRelativePath() log.Trace("DeleteAvatar[%d]: %s", repo.ID, avatarPath) - ctx, committer, err := db.TxContext(db.DefaultContext) + ctx, committer, err := db.TxContext(ctx) if err != nil { return err } @@ -102,7 +102,7 @@ func RemoveRandomAvatars(ctx context.Context) error { } stringifiedID := strconv.FormatInt(repository.ID, 10) if repository.Avatar == stringifiedID { - return DeleteAvatar(repository) + return DeleteAvatar(ctx, repository) } return nil }) diff --git a/services/repository/avatar_test.go b/services/repository/avatar_test.go index 5ec899ec3..4a0ba6185 100644 --- a/services/repository/avatar_test.go +++ b/services/repository/avatar_test.go @@ -9,6 +9,7 @@ import ( "image/png" "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/avatar" @@ -25,7 +26,7 @@ func TestUploadAvatar(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - err := UploadAvatar(repo, buff.Bytes()) + err := UploadAvatar(db.DefaultContext, repo, buff.Bytes()) assert.NoError(t, err) assert.Equal(t, avatar.HashAvatar(10, buff.Bytes()), repo.Avatar) } @@ -39,7 +40,7 @@ func TestUploadBigAvatar(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - err := UploadAvatar(repo, buff.Bytes()) + err := UploadAvatar(db.DefaultContext, repo, buff.Bytes()) assert.Error(t, err) } @@ -52,10 +53,10 @@ func TestDeleteAvatar(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - err := UploadAvatar(repo, buff.Bytes()) + err := UploadAvatar(db.DefaultContext, repo, buff.Bytes()) assert.NoError(t, err) - err = DeleteAvatar(repo) + err = DeleteAvatar(db.DefaultContext, repo) assert.NoError(t, err) assert.Equal(t, "", repo.Avatar) diff --git a/services/repository/branch.go b/services/repository/branch.go index 291fb4a92..a085026ae 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -10,7 +10,6 @@ import ( "strings" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -106,7 +105,7 @@ func CreateNewBranchFromCommit(ctx context.Context, doer *user_model.User, repo } // RenameBranch rename a branch -func RenameBranch(repo *repo_model.Repository, doer *user_model.User, gitRepo *git.Repository, from, to string) (string, error) { +func RenameBranch(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, gitRepo *git.Repository, from, to string) (string, error) { if from == to { return "target_exist", nil } @@ -119,7 +118,7 @@ func RenameBranch(repo *repo_model.Repository, doer *user_model.User, gitRepo *g return "from_not_exist", nil } - if err := git_model.RenameBranch(db.DefaultContext, repo, from, to, func(isDefault bool) error { + if err := git_model.RenameBranch(ctx, repo, from, to, func(isDefault bool) error { err2 := gitRepo.RenameBranch(from, to) if err2 != nil { return err2 @@ -141,8 +140,8 @@ func RenameBranch(repo *repo_model.Repository, doer *user_model.User, gitRepo *g return "", err } - notification.NotifyDeleteRef(db.DefaultContext, doer, repo, "branch", git.BranchPrefix+from) - notification.NotifyCreateRef(db.DefaultContext, doer, repo, "branch", git.BranchPrefix+to, refID) + notification.NotifyDeleteRef(ctx, doer, repo, "branch", git.BranchPrefix+from) + notification.NotifyCreateRef(ctx, doer, repo, "branch", git.BranchPrefix+to, refID) return "", nil } @@ -153,12 +152,12 @@ var ( ) // DeleteBranch delete branch -func DeleteBranch(doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error { +func DeleteBranch(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, branchName string) error { if branchName == repo.DefaultBranch { return ErrBranchIsDefault } - isProtected, err := git_model.IsBranchProtected(db.DefaultContext, repo.ID, branchName) + isProtected, err := git_model.IsBranchProtected(ctx, repo.ID, branchName) if err != nil { return err } @@ -195,7 +194,7 @@ func DeleteBranch(doer *user_model.User, repo *repo_model.Repository, gitRepo *g log.Error("Update: %v", err) } - if err := git_model.AddDeletedBranch(db.DefaultContext, repo.ID, branchName, commit.ID.String(), doer.ID); err != nil { + if err := git_model.AddDeletedBranch(ctx, repo.ID, branchName, commit.ID.String(), doer.ID); err != nil { log.Warn("AddDeletedBranch: %v", err) } diff --git a/services/repository/files/file.go b/services/repository/files/file.go index 2bac4372d..7939491ae 100644 --- a/services/repository/files/file.go +++ b/services/repository/files/file.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "net/url" - "path" "strings" "time" @@ -15,6 +14,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" ) // GetFileResponseFromCommit Constructs a FileResponse from a Commit object @@ -129,7 +129,7 @@ func GetAuthorAndCommitterUsers(author, committer *IdentityOptions, doer *user_m // CleanUploadFileName Trims a filename and returns empty string if it is a .git directory func CleanUploadFileName(name string) string { // Rebase the filename - name = strings.Trim(path.Clean("/"+name), "/") + name = strings.Trim(util.CleanPath(name), "/") // Git disallows any filenames to have a .git directory in them. for _, part := range strings.Split(name, "/") { if strings.ToLower(part) == ".git" { diff --git a/services/repository/fork.go b/services/repository/fork.go index c3ca89e02..fb93b10f1 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -189,8 +189,8 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork } // ConvertForkToNormalRepository convert the provided repo from a forked repo to normal repo -func ConvertForkToNormalRepository(repo *repo_model.Repository) error { - err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { +func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Repository) error { + err := db.WithTx(ctx, func(ctx context.Context) error { repo, err := repo_model.GetRepositoryByID(ctx, repo.ID) if err != nil { return err diff --git a/services/repository/hooks.go b/services/repository/hooks.go index a8b6f7a62..8506fa341 100644 --- a/services/repository/hooks.go +++ b/services/repository/hooks.go @@ -101,7 +101,7 @@ func GenerateWebhooks(ctx context.Context, templateRepo, generateRepo *repo_mode HookEvent: templateWebhook.HookEvent, IsActive: templateWebhook.IsActive, Type: templateWebhook.Type, - OrgID: templateWebhook.OrgID, + OwnerID: templateWebhook.OwnerID, Events: templateWebhook.Events, Meta: templateWebhook.Meta, }) diff --git a/services/repository/push.go b/services/repository/push.go index 8aa8be6aa..4b574e344 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -122,7 +122,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { tagName := opts.TagName() if opts.IsDelRef() { notification.NotifyPushCommits( - db.DefaultContext, pusher, repo, + ctx, pusher, repo, &repo_module.PushUpdateOptions{ RefFullName: git.TagPrefix + tagName, OldCommitID: opts.OldCommitID, @@ -130,7 +130,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { }, repo_module.NewPushCommits()) delTags = append(delTags, tagName) - notification.NotifyDeleteRef(db.DefaultContext, pusher, repo, "tag", opts.RefFullName) + notification.NotifyDeleteRef(ctx, pusher, repo, "tag", opts.RefFullName) } else { // is new tag newCommit, err := gitRepo.GetCommit(opts.NewCommitID) if err != nil { @@ -142,7 +142,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { commits.CompareURL = repo.ComposeCompareURL(git.EmptySHA, opts.NewCommitID) notification.NotifyPushCommits( - db.DefaultContext, pusher, repo, + ctx, pusher, repo, &repo_module.PushUpdateOptions{ RefFullName: git.TagPrefix + tagName, OldCommitID: git.EmptySHA, @@ -150,7 +150,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { }, commits) addTags = append(addTags, tagName) - notification.NotifyCreateRef(db.DefaultContext, pusher, repo, "tag", opts.RefFullName, opts.NewCommitID) + notification.NotifyCreateRef(ctx, pusher, repo, "tag", opts.RefFullName, opts.NewCommitID) } } else if opts.IsBranch() { // If is branch reference if pusher == nil || pusher.ID != opts.PusherID { @@ -190,7 +190,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } } // Update the is empty and default_branch columns - if err := repo_model.UpdateRepositoryCols(db.DefaultContext, repo, "default_branch", "is_empty"); err != nil { + if err := repo_model.UpdateRepositoryCols(ctx, repo, "default_branch", "is_empty"); err != nil { return fmt.Errorf("UpdateRepositoryCols: %w", err) } } @@ -199,19 +199,19 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { if err != nil { return fmt.Errorf("newCommit.CommitsBeforeLimit: %w", err) } - notification.NotifyCreateRef(db.DefaultContext, pusher, repo, "branch", opts.RefFullName, opts.NewCommitID) + notification.NotifyCreateRef(ctx, pusher, repo, "branch", opts.RefFullName, opts.NewCommitID) } else { l, err = newCommit.CommitsBeforeUntil(opts.OldCommitID) if err != nil { return fmt.Errorf("newCommit.CommitsBeforeUntil: %w", err) } - isForce, err := repo_module.IsForcePush(ctx, opts) + isForcePush, err := newCommit.IsForcePush(opts.OldCommitID) if err != nil { - log.Error("isForcePush %s:%s failed: %v", repo.FullName(), branch, err) + log.Error("IsForcePush %s:%s failed: %v", repo.FullName(), branch, err) } - if isForce { + if isForcePush { log.Trace("Push %s is a force push", opts.NewCommitID) cache.Remove(repo.GetCommitsCountCacheKey(opts.RefName(), true)) @@ -259,7 +259,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { commits.Commits = commits.Commits[:setting.UI.FeedMaxCommitNum] } - notification.NotifyPushCommits(db.DefaultContext, pusher, repo, opts, commits) + notification.NotifyPushCommits(ctx, pusher, repo, opts, commits) if err = git_model.RemoveDeletedBranchByName(ctx, repo.ID, branch); err != nil { log.Error("models.RemoveDeletedBranch %s/%s failed: %v", repo.ID, branch, err) @@ -270,7 +270,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { log.Error("repo_module.CacheRef %s/%s failed: %v", repo.ID, branch, err) } } else { - notification.NotifyDeleteRef(db.DefaultContext, pusher, repo, "branch", opts.RefFullName) + notification.NotifyDeleteRef(ctx, pusher, repo, "branch", opts.RefFullName) if err = pull_service.CloseBranchPulls(pusher, repo.ID, branch); err != nil { // close all related pulls log.Error("close related pull request failed: %v", err) @@ -278,14 +278,14 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } // Even if user delete a branch on a repository which he didn't watch, he will be watch that. - if err = repo_model.WatchIfAuto(db.DefaultContext, opts.PusherID, repo.ID, true); err != nil { + if err = repo_model.WatchIfAuto(ctx, opts.PusherID, repo.ID, true); err != nil { log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err) } } else { log.Trace("Non-tag and non-branch commits pushed.") } } - if err := PushUpdateAddDeleteTags(repo, gitRepo, addTags, delTags); err != nil { + if err := PushUpdateAddDeleteTags(ctx, repo, gitRepo, addTags, delTags); err != nil { return fmt.Errorf("PushUpdateAddDeleteTags: %w", err) } @@ -298,8 +298,8 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { } // PushUpdateAddDeleteTags updates a number of added and delete tags -func PushUpdateAddDeleteTags(repo *repo_model.Repository, gitRepo *git.Repository, addTags, delTags []string) error { - return db.WithTx(db.DefaultContext, func(ctx context.Context) error { +func PushUpdateAddDeleteTags(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, addTags, delTags []string) error { + return db.WithTx(ctx, func(ctx context.Context) error { if err := repo_model.PushUpdateDeleteTagsContext(ctx, repo, delTags); err != nil { return err } diff --git a/services/repository/repository.go b/services/repository/repository.go index 3c3e7e82c..000b1a3da 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -24,14 +24,14 @@ import ( ) // CreateRepository creates a repository for the user/organization. -func CreateRepository(doer, owner *user_model.User, opts repo_module.CreateRepoOptions) (*repo_model.Repository, error) { +func CreateRepository(ctx context.Context, doer, owner *user_model.User, opts repo_module.CreateRepoOptions) (*repo_model.Repository, error) { repo, err := repo_module.CreateRepository(doer, owner, opts) if err != nil { // No need to rollback here we should do this in CreateRepository... return nil, err } - notification.NotifyCreateRepository(db.DefaultContext, doer, owner, repo) + notification.NotifyCreateRepository(ctx, doer, owner, repo) return repo, nil } @@ -55,10 +55,10 @@ func DeleteRepository(ctx context.Context, doer *user_model.User, repo *repo_mod } // PushCreateRepo creates a repository when a new repository is pushed to an appropriate namespace -func PushCreateRepo(authUser, owner *user_model.User, repoName string) (*repo_model.Repository, error) { +func PushCreateRepo(ctx context.Context, authUser, owner *user_model.User, repoName string) (*repo_model.Repository, error) { if !authUser.IsAdmin { if owner.IsOrganization() { - if ok, err := organization.CanCreateOrgRepo(db.DefaultContext, owner.ID, authUser.ID); err != nil { + if ok, err := organization.CanCreateOrgRepo(ctx, owner.ID, authUser.ID); err != nil { return nil, err } else if !ok { return nil, fmt.Errorf("cannot push-create repository for org") @@ -68,7 +68,7 @@ func PushCreateRepo(authUser, owner *user_model.User, repoName string) (*repo_mo } } - repo, err := CreateRepository(authUser, owner, repo_module.CreateRepoOptions{ + repo, err := CreateRepository(ctx, authUser, owner, repo_module.CreateRepoOptions{ Name: repoName, IsPrivate: setting.Repository.DefaultPushCreatePrivate, }) @@ -88,8 +88,8 @@ func Init() error { } // UpdateRepository updates a repository -func UpdateRepository(repo *repo_model.Repository, visibilityChanged bool) (err error) { - ctx, committer, err := db.TxContext(db.DefaultContext) +func UpdateRepository(ctx context.Context, repo *repo_model.Repository, visibilityChanged bool) (err error) { + ctx, committer, err := db.TxContext(ctx) if err != nil { return err } diff --git a/services/repository/review.go b/services/repository/review.go index 6b5f09605..40513e6bc 100644 --- a/services/repository/review.go +++ b/services/repository/review.go @@ -4,20 +4,21 @@ package repository import ( - "code.gitea.io/gitea/models/db" + "context" + "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" ) // GetReviewerTeams get all teams can be requested to review -func GetReviewerTeams(repo *repo_model.Repository) ([]*organization.Team, error) { - if err := repo.LoadOwner(db.DefaultContext); err != nil { +func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) { + if err := repo.LoadOwner(ctx); err != nil { return nil, err } if !repo.Owner.IsOrganization() { return nil, nil } - return organization.GetTeamsWithAccessToRepo(db.DefaultContext, repo.OwnerID, repo.ID, perm.AccessModeRead) + return organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) } diff --git a/services/repository/review_test.go b/services/repository/review_test.go index 2bf4cdbf5..2db56d4e8 100644 --- a/services/repository/review_test.go +++ b/services/repository/review_test.go @@ -6,6 +6,7 @@ package repository import ( "testing" + "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -16,12 +17,12 @@ func TestRepoGetReviewerTeams(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - teams, err := GetReviewerTeams(repo2) + teams, err := GetReviewerTeams(db.DefaultContext, repo2) assert.NoError(t, err) assert.Empty(t, teams) repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - teams, err = GetReviewerTeams(repo3) + teams, err = GetReviewerTeams(db.DefaultContext, repo3) assert.NoError(t, err) assert.Len(t, teams, 2) } diff --git a/services/repository/template.go b/services/repository/template.go index 8c75948c4..42174d095 100644 --- a/services/repository/template.go +++ b/services/repository/template.go @@ -40,7 +40,7 @@ func GenerateIssueLabels(ctx context.Context, templateRepo, generateRepo *repo_m } // GenerateRepository generates a repository from a template -func GenerateRepository(doer, owner *user_model.User, templateRepo *repo_model.Repository, opts repo_module.GenerateRepoOptions) (_ *repo_model.Repository, err error) { +func GenerateRepository(ctx context.Context, doer, owner *user_model.User, templateRepo *repo_model.Repository, opts repo_module.GenerateRepoOptions) (_ *repo_model.Repository, err error) { if !doer.IsAdmin && !owner.CanCreateRepo() { return nil, repo_model.ErrReachLimitOfRepo{ Limit: owner.MaxRepoCreation, @@ -48,7 +48,7 @@ func GenerateRepository(doer, owner *user_model.User, templateRepo *repo_model.R } var generateRepo *repo_model.Repository - if err = db.WithTx(db.DefaultContext, func(ctx context.Context) error { + if err = db.WithTx(ctx, func(ctx context.Context) error { generateRepo, err = repo_module.GenerateRepository(ctx, doer, owner, templateRepo, opts) if err != nil { return err @@ -101,7 +101,7 @@ func GenerateRepository(doer, owner *user_model.User, templateRepo *repo_model.R return nil, err } - notification.NotifyCreateRepository(db.DefaultContext, doer, owner, generateRepo) + notification.NotifyCreateRepository(ctx, doer, owner, generateRepo) return generateRepo, nil } diff --git a/services/repository/transfer.go b/services/repository/transfer.go index 8c167552d..b9b26f314 100644 --- a/services/repository/transfer.go +++ b/services/repository/transfer.go @@ -8,7 +8,6 @@ import ( "fmt" "code.gitea.io/gitea/models" - "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" @@ -61,7 +60,7 @@ func TransferOwnership(ctx context.Context, doer, newOwner *user_model.User, rep } // ChangeRepositoryName changes all corresponding setting from old repository name to new one. -func ChangeRepositoryName(doer *user_model.User, repo *repo_model.Repository, newRepoName string) error { +func ChangeRepositoryName(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, newRepoName string) error { log.Trace("ChangeRepositoryName: %s/%s -> %s", doer.Name, repo.Name, newRepoName) oldRepoName := repo.Name @@ -78,7 +77,7 @@ func ChangeRepositoryName(doer *user_model.User, repo *repo_model.Repository, ne repoWorkingPool.CheckOut(fmt.Sprint(repo.ID)) repo.Name = newRepoName - notification.NotifyRenameRepository(db.DefaultContext, doer, repo, oldRepoName) + notification.NotifyRenameRepository(ctx, doer, repo, oldRepoName) return nil } diff --git a/services/webhook/webhook.go b/services/webhook/webhook.go index afd8e3c10..b862d5bff 100644 --- a/services/webhook/webhook.go +++ b/services/webhook/webhook.go @@ -229,16 +229,16 @@ func PrepareWebhooks(ctx context.Context, source EventSource, event webhook_modu owner = source.Repository.MustOwner(ctx) } - // check if owner is an org and append additional webhooks - if owner != nil && owner.IsOrganization() { - orgHooks, err := webhook_model.ListWebhooksByOpts(ctx, &webhook_model.ListWebhookOptions{ - OrgID: owner.ID, + // append additional webhooks of a user or organization + if owner != nil { + ownerHooks, err := webhook_model.ListWebhooksByOpts(ctx, &webhook_model.ListWebhookOptions{ + OwnerID: owner.ID, IsActive: util.OptionalBoolTrue, }) if err != nil { return fmt.Errorf("ListWebhooksByOpts: %w", err) } - ws = append(ws, orgHooks...) + ws = append(ws, ownerHooks...) } // Add any admin-defined system webhooks |