feat: [CODE-1474]: add check PR rebaseability (#2763)

* update migration number
* rename UpdateConflict->UpdateMergeOutcome
* small fixes
* handle mergeability for all merge methods
* alway separate rebaseability check
* add check PR rebaseability
pull/3571/head
Marko Gaćeša 2024-10-04 12:36:32 +00:00 committed by Harness
parent 4e2c9fae28
commit 2892378036
13 changed files with 255 additions and 190 deletions

View File

@ -58,14 +58,16 @@ func (in *MergeInput) sanitize() error {
return usererror.BadRequest("source SHA must be provided") return usererror.BadRequest("source SHA must be provided")
} }
if in.Method != "" { if in.Method == "" {
in.Method = enum.MergeMethodMerge
}
method, ok := in.Method.Sanitize() method, ok := in.Method.Sanitize()
if !ok { if !ok {
return usererror.BadRequestf("unsupported merge method: %s", in.Method) return usererror.BadRequestf("unsupported merge method: %s", in.Method)
} }
in.Method = method in.Method = method
}
// cleanup title / message (NOTE: git doesn't support white space only) // cleanup title / message (NOTE: git doesn't support white space only)
in.Title = strings.TrimSpace(in.Title) in.Title = strings.TrimSpace(in.Title)
@ -233,17 +235,44 @@ func (c *Controller) Merge(
// has advanced. It's possible that the merge base commit is different too. // has advanced. It's possible that the merge base commit is different too.
// So, the next time the API gets called for the same PR the mergeability status will not be unchecked. // So, the next time the API gets called for the same PR the mergeability status will not be unchecked.
// Without dry-run the execution would proceed below and would either merge the PR or set the conflict status. // Without dry-run the execution would proceed below and would either merge the PR or set the conflict status.
if pr.MergeCheckStatus == enum.MergeCheckStatusUnchecked {
mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ var mergeOutput git.MergeOutput
// We distinguish two types when checking mergeability: Rebase and Non-Rebase.
// * Merge methods Merge and Squash will always have the same results.
// * Merge method Rebase is special because it must always check all commits, one at a time.
// * Merge method Fast-Forward can never have conflicts,
// but for it the merge base SHA must be equal to target branch SHA.
// The result of the tests will be stored (think cached) in the database for these two types
// in the fields merge_check_status and rebase_check_status.
checkMergeability := func(method enum.MergeMethod) bool {
switch method {
case enum.MergeMethodMerge, enum.MergeMethodSquash:
return pr.MergeCheckStatus == enum.MergeCheckStatusUnchecked
case enum.MergeMethodRebase:
return pr.RebaseCheckStatus == enum.MergeCheckStatusUnchecked
case enum.MergeMethodFastForward:
// Always check for ff merge. There can never be conflicts,
// but we are interested in if it returns the conflict error and merge-output data.
return true
default:
return true // should not happen
}
}(in.Method)
if checkMergeability {
mergeOutput, err = c.git.Merge(ctx, &git.MergeParams{
WriteParams: targetWriteParams, WriteParams: targetWriteParams,
BaseBranch: pr.TargetBranch, BaseBranch: pr.TargetBranch,
HeadRepoUID: sourceRepo.GitUID, HeadRepoUID: sourceRepo.GitUID,
HeadBranch: pr.SourceBranch, HeadBranch: pr.SourceBranch,
RefType: gitenum.RefTypeUndefined, // update no refs -> no commit will be created RefType: gitenum.RefTypeUndefined, // update no refs -> no commit will be created
HeadExpectedSHA: sha.Must(in.SourceSHA), HeadExpectedSHA: sha.Must(in.SourceSHA),
Method: gitenum.MergeMethod(in.Method),
}) })
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("merge check execution failed: %w", err) return nil, nil, fmt.Errorf("failed merge check with method=%s: %w", in.Method, err)
} }
pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error { pr, err = c.pullreqStore.UpdateOptLock(ctx, pr, func(pr *types.PullReq) error {
@ -255,25 +284,19 @@ func (c *Controller) Merge(
return usererror.BadRequest("Pull request must be open") return usererror.BadRequest("Pull request must be open")
} }
if len(mergeOutput.ConflictFiles) > 0 {
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
pr.MergeSHA = nil
pr.MergeConflicts = mergeOutput.ConflictFiles
} else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String() pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String()) pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
pr.MergeSHA = nil // dry-run doesn't create a merge commit so output is empty. pr.MergeSHA = nil // dry-run doesn't create a merge commit so output is empty.
pr.MergeConflicts = nil
} pr.UpdateMergeOutcome(in.Method, mergeOutput.ConflictFiles)
pr.Stats.DiffStats = types.NewDiffStats( pr.Stats.DiffStats = types.NewDiffStats(
mergeOutput.CommitCount, mergeOutput.CommitCount,
mergeOutput.ChangedFileCount, mergeOutput.ChangedFileCount,
mergeOutput.Additions, mergeOutput.Additions,
mergeOutput.Deletions, mergeOutput.Deletions,
) )
return nil return nil
}) })
if err != nil { if err != nil {
@ -286,6 +309,13 @@ func (c *Controller) Merge(
} }
} }
var conflicts []string
if in.Method == enum.MergeMethodRebase {
conflicts = pr.RebaseConflicts
} else {
conflicts = pr.MergeConflicts
}
// With in.DryRun=true this function never returns types.MergeViolations // With in.DryRun=true this function never returns types.MergeViolations
out := &types.MergeResponse{ out := &types.MergeResponse{
BranchDeleted: ruleOut.DeleteSourceBranch, BranchDeleted: ruleOut.DeleteSourceBranch,
@ -293,7 +323,8 @@ func (c *Controller) Merge(
// values only returned by dry run // values only returned by dry run
DryRun: true, DryRun: true,
ConflictFiles: pr.MergeConflicts, Mergeable: len(conflicts) == 0,
ConflictFiles: conflicts,
AllowedMethods: ruleOut.AllowedMethods, AllowedMethods: ruleOut.AllowedMethods,
RequiresCodeOwnersApproval: ruleOut.RequiresCodeOwnersApproval, RequiresCodeOwnersApproval: ruleOut.RequiresCodeOwnersApproval,
RequiresCodeOwnersApprovalLatest: ruleOut.RequiresCodeOwnersApprovalLatest, RequiresCodeOwnersApprovalLatest: ruleOut.RequiresCodeOwnersApprovalLatest,
@ -399,11 +430,10 @@ func (c *Controller) Merge(
} }
// update all Merge specific information // update all Merge specific information
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String() pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String()) pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = mergeOutput.ConflictFiles pr.UpdateMergeOutcome(in.Method, mergeOutput.ConflictFiles)
pr.Stats.DiffStats = types.NewDiffStats( pr.Stats.DiffStats = types.NewDiffStats(
mergeOutput.CommitCount, mergeOutput.CommitCount,
mergeOutput.ChangedFileCount, mergeOutput.ChangedFileCount,
@ -447,12 +477,11 @@ func (c *Controller) Merge(
// update all Merge specific information (might be empty if previous merge check failed) // update all Merge specific information (might be empty if previous merge check failed)
// since this is the final operation on the PR, we update any sha that might've changed by now. // since this is the final operation on the PR, we update any sha that might've changed by now.
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.SourceSHA = mergeOutput.HeadSHA.String() pr.SourceSHA = mergeOutput.HeadSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String()) pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String() pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeSHA = ptr.String(mergeOutput.MergeSHA.String()) pr.MergeSHA = ptr.String(mergeOutput.MergeSHA.String())
pr.MergeConflicts = nil pr.MarkAsMerged()
pr.Stats.DiffStats = types.NewDiffStats( pr.Stats.DiffStats = types.NewDiffStats(
mergeOutput.CommitCount, mergeOutput.CommitCount,
mergeOutput.ChangedFileCount, mergeOutput.ChangedFileCount,

View File

@ -202,9 +202,10 @@ func newPullReq(
ActivitySeq: 0, ActivitySeq: 0,
MergedBy: nil, MergedBy: nil,
Merged: nil, Merged: nil,
MergeCheckStatus: enum.MergeCheckStatusUnchecked,
MergeMethod: nil, MergeMethod: nil,
MergeBaseSHA: mergeBaseSHA.String(), MergeBaseSHA: mergeBaseSHA.String(),
MergeCheckStatus: enum.MergeCheckStatusUnchecked,
RebaseCheckStatus: enum.MergeCheckStatusUnchecked,
Author: *session.Principal.ToPrincipalInfo(), Author: *session.Principal.ToPrincipalInfo(),
Merger: nil, Merger: nil,
} }

View File

@ -146,11 +146,11 @@ func (c *Controller) State(ctx context.Context,
nowMilli := time.Now().UnixMilli() nowMilli := time.Now().UnixMilli()
// clear all merge (check) related fields // clear all merge (check) related fields
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = nil
pr.MergeTargetSHA = nil pr.MergeTargetSHA = nil
pr.Closed = &nowMilli pr.Closed = &nowMilli
pr.MarkAsMergeUnchecked()
case changeReopen: case changeReopen:
pr.SourceSHA = sourceSHA.String() pr.SourceSHA = sourceSHA.String()
pr.MergeBaseSHA = mergeBaseSHA.String() pr.MergeBaseSHA = mergeBaseSHA.String()

View File

@ -277,12 +277,11 @@ func (r *repoImportState) convertPullReq(
mergeMethod := enum.MergeMethodMerge // Don't know mergeMethod := enum.MergeMethodMerge // Don't know
pr.MergeMethod = &mergeMethod pr.MergeMethod = &mergeMethod
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.SourceSHA = extPullReq.Head.SHA pr.SourceSHA = extPullReq.Head.SHA
pr.MergeTargetSHA = &extPullReq.Base.SHA pr.MergeTargetSHA = &extPullReq.Base.SHA
pr.MergeBaseSHA = extPullReq.Base.SHA pr.MergeBaseSHA = extPullReq.Base.SHA
pr.MergeSHA = nil // Don't have this. pr.MergeSHA = nil // Don't have this.
pr.MergeConflicts = nil pr.MarkAsMerged()
case enum.PullReqStateClosed: case enum.PullReqStateClosed:
// For closed PR's it's not important to verify existence of branches and commits. // For closed PR's it's not important to verify existence of branches and commits.
@ -290,10 +289,11 @@ func (r *repoImportState) convertPullReq(
pr.SourceSHA = extPullReq.Head.SHA pr.SourceSHA = extPullReq.Head.SHA
pr.MergeTargetSHA = &extPullReq.Base.SHA pr.MergeTargetSHA = &extPullReq.Base.SHA
pr.MergeBaseSHA = extPullReq.Base.SHA pr.MergeBaseSHA = extPullReq.Base.SHA
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = nil pr.MergeConflicts = nil
pr.MergeTargetSHA = nil pr.MergeTargetSHA = nil
pr.MarkAsMergeUnchecked()
pr.Closed = &pr.Updated pr.Closed = &pr.Updated
case enum.PullReqStateOpen: case enum.PullReqStateOpen:
@ -330,7 +330,7 @@ func (r *repoImportState) convertPullReq(
pr.SourceSHA = sourceSHA pr.SourceSHA = sourceSHA
pr.MergeTargetSHA = &targetSHA pr.MergeTargetSHA = &targetSHA
pr.MergeBaseSHA = mergeBase.MergeBaseSHA.String() pr.MergeBaseSHA = mergeBase.MergeBaseSHA.String()
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked pr.MarkAsMergeUnchecked()
} }
log.Debug().Str("pullreq.state", string(pr.State)).Msg("importing pull request") log.Debug().Str("pullreq.state", string(pr.State)).Msg("importing pull request")

View File

@ -122,11 +122,11 @@ func (s *Service) triggerPREventOnBranchUpdate(ctx context.Context,
pr.MergeBaseSHA = newMergeBase.String() pr.MergeBaseSHA = newMergeBase.String()
// reset merge-check fields for new run // reset merge-check fields for new run
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = nil
pr.Stats.DiffStats.Commits = nil pr.Stats.DiffStats.Commits = nil
pr.Stats.DiffStats.FilesChanged = nil pr.Stats.DiffStats.FilesChanged = nil
pr.MarkAsMergeUnchecked()
return nil return nil
}) })
@ -198,9 +198,8 @@ func (s *Service) closePullReqOnBranchDelete(ctx context.Context,
activitySeqPRClosed = pr.ActivitySeq activitySeqPRClosed = pr.ActivitySeq
pr.State = enum.PullReqStateClosed pr.State = enum.PullReqStateClosed
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = nil pr.MarkAsMergeUnchecked()
return nil return nil
}) })

View File

@ -221,19 +221,14 @@ func (s *Service) updateMergeData(
return events.NewDiscardEventErrorf("PR SHA %s is newer than %s", pr.SourceSHA, newSHA) return events.NewDiscardEventErrorf("PR SHA %s is newer than %s", pr.SourceSHA, newSHA)
} }
if len(mergeOutput.ConflictFiles) > 0 {
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String() pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String()) pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
if mergeOutput.MergeSHA.IsEmpty() {
pr.MergeSHA = nil pr.MergeSHA = nil
pr.MergeConflicts = mergeOutput.ConflictFiles
} else { } else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeBaseSHA = mergeOutput.MergeBaseSHA.String()
pr.MergeTargetSHA = ptr.String(mergeOutput.BaseSHA.String())
pr.MergeSHA = ptr.String(mergeOutput.MergeSHA.String()) pr.MergeSHA = ptr.String(mergeOutput.MergeSHA.String())
pr.MergeConflicts = nil
} }
pr.UpdateMergeOutcome(enum.MergeMethodMerge, mergeOutput.ConflictFiles)
pr.Stats.DiffStats = types.NewDiffStats( pr.Stats.DiffStats = types.NewDiffStats(
mergeOutput.CommitCount, mergeOutput.CommitCount,
mergeOutput.ChangedFileCount, mergeOutput.ChangedFileCount,

View File

@ -0,0 +1,3 @@
ALTER TABLE pullreqs
DROP COLUMN pullreq_rebase_conflicts,
DROP COLUMN pullreq_rebase_check_status;

View File

@ -0,0 +1,3 @@
ALTER TABLE pullreqs
ADD COLUMN pullreq_rebase_check_status TEXT NOT NULL DEFAULT 'unchecked',
ADD COLUMN pullreq_rebase_conflicts TEXT;

View File

@ -0,0 +1,2 @@
ALTER TABLE pullreqs DROP COLUMN pullreq_rebase_conflicts;
ALTER TABLE pullreqs DROP COLUMN pullreq_rebase_check_status;

View File

@ -0,0 +1,2 @@
ALTER TABLE pullreqs ADD COLUMN pullreq_rebase_check_status TEXT NOT NULL DEFAULT 'unchecked';
ALTER TABLE pullreqs ADD COLUMN pullreq_rebase_conflicts TEXT;

View File

@ -85,11 +85,14 @@ type pullReq struct {
Merged null.Int `db:"pullreq_merged"` Merged null.Int `db:"pullreq_merged"`
MergeMethod null.String `db:"pullreq_merge_method"` MergeMethod null.String `db:"pullreq_merge_method"`
MergeCheckStatus enum.MergeCheckStatus `db:"pullreq_merge_check_status"`
MergeTargetSHA null.String `db:"pullreq_merge_target_sha"` MergeTargetSHA null.String `db:"pullreq_merge_target_sha"`
MergeBaseSHA string `db:"pullreq_merge_base_sha"` MergeBaseSHA string `db:"pullreq_merge_base_sha"`
MergeSHA null.String `db:"pullreq_merge_sha"` MergeSHA null.String `db:"pullreq_merge_sha"`
MergeCheckStatus enum.MergeCheckStatus `db:"pullreq_merge_check_status"`
MergeConflicts null.String `db:"pullreq_merge_conflicts"` MergeConflicts null.String `db:"pullreq_merge_conflicts"`
RebaseCheckStatus enum.MergeCheckStatus `db:"pullreq_rebase_check_status"`
RebaseConflicts null.String `db:"pullreq_rebase_conflicts,omitempty"`
CommitCount null.Int `db:"pullreq_commit_count"` CommitCount null.Int `db:"pullreq_commit_count"`
FileCount null.Int `db:"pullreq_file_count"` FileCount null.Int `db:"pullreq_file_count"`
@ -121,11 +124,13 @@ const (
,pullreq_merged_by ,pullreq_merged_by
,pullreq_merged ,pullreq_merged
,pullreq_merge_method ,pullreq_merge_method
,pullreq_merge_check_status
,pullreq_merge_target_sha ,pullreq_merge_target_sha
,pullreq_merge_base_sha ,pullreq_merge_base_sha
,pullreq_merge_sha ,pullreq_merge_sha
,pullreq_merge_check_status
,pullreq_merge_conflicts ,pullreq_merge_conflicts
,pullreq_rebase_check_status
,pullreq_rebase_conflicts
,pullreq_commit_count ,pullreq_commit_count
,pullreq_file_count ,pullreq_file_count
,pullreq_additions ,pullreq_additions
@ -218,11 +223,13 @@ func (s *PullReqStore) Create(ctx context.Context, pr *types.PullReq) error {
,pullreq_merged_by ,pullreq_merged_by
,pullreq_merged ,pullreq_merged
,pullreq_merge_method ,pullreq_merge_method
,pullreq_merge_check_status
,pullreq_merge_target_sha ,pullreq_merge_target_sha
,pullreq_merge_base_sha ,pullreq_merge_base_sha
,pullreq_merge_sha ,pullreq_merge_sha
,pullreq_merge_check_status
,pullreq_merge_conflicts ,pullreq_merge_conflicts
,pullreq_rebase_check_status
,pullreq_rebase_conflicts
,pullreq_commit_count ,pullreq_commit_count
,pullreq_file_count ,pullreq_file_count
,pullreq_additions ,pullreq_additions
@ -250,11 +257,13 @@ func (s *PullReqStore) Create(ctx context.Context, pr *types.PullReq) error {
,:pullreq_merged_by ,:pullreq_merged_by
,:pullreq_merged ,:pullreq_merged
,:pullreq_merge_method ,:pullreq_merge_method
,:pullreq_merge_check_status
,:pullreq_merge_target_sha ,:pullreq_merge_target_sha
,:pullreq_merge_base_sha ,:pullreq_merge_base_sha
,:pullreq_merge_sha ,:pullreq_merge_sha
,:pullreq_merge_check_status
,:pullreq_merge_conflicts ,:pullreq_merge_conflicts
,:pullreq_rebase_check_status
,:pullreq_rebase_conflicts
,:pullreq_commit_count ,:pullreq_commit_count
,:pullreq_file_count ,:pullreq_file_count
,:pullreq_additions ,:pullreq_additions
@ -295,11 +304,13 @@ func (s *PullReqStore) Update(ctx context.Context, pr *types.PullReq) error {
,pullreq_merged_by = :pullreq_merged_by ,pullreq_merged_by = :pullreq_merged_by
,pullreq_merged = :pullreq_merged ,pullreq_merged = :pullreq_merged
,pullreq_merge_method = :pullreq_merge_method ,pullreq_merge_method = :pullreq_merge_method
,pullreq_merge_check_status = :pullreq_merge_check_status
,pullreq_merge_target_sha = :pullreq_merge_target_sha ,pullreq_merge_target_sha = :pullreq_merge_target_sha
,pullreq_merge_base_sha = :pullreq_merge_base_sha ,pullreq_merge_base_sha = :pullreq_merge_base_sha
,pullreq_merge_sha = :pullreq_merge_sha ,pullreq_merge_sha = :pullreq_merge_sha
,pullreq_merge_check_status = :pullreq_merge_check_status
,pullreq_merge_conflicts = :pullreq_merge_conflicts ,pullreq_merge_conflicts = :pullreq_merge_conflicts
,pullreq_rebase_check_status = :pullreq_rebase_check_status
,pullreq_rebase_conflicts = :pullreq_rebase_conflicts
,pullreq_commit_count = :pullreq_commit_count ,pullreq_commit_count = :pullreq_commit_count
,pullreq_file_count = :pullreq_file_count ,pullreq_file_count = :pullreq_file_count
,pullreq_additions = :pullreq_additions ,pullreq_additions = :pullreq_additions
@ -387,10 +398,12 @@ func (s *PullReqStore) ResetMergeCheckStatus(
SET SET
pullreq_updated = $1 pullreq_updated = $1
,pullreq_version = pullreq_version + 1 ,pullreq_version = pullreq_version + 1
,pullreq_merge_check_status = $2
,pullreq_merge_target_sha = NULL ,pullreq_merge_target_sha = NULL
,pullreq_merge_sha = NULL ,pullreq_merge_sha = NULL
,pullreq_merge_check_status = $2
,pullreq_merge_conflicts = NULL ,pullreq_merge_conflicts = NULL
,pullreq_rebase_check_status = $2
,pullreq_rebase_conflicts = NULL
,pullreq_commit_count = NULL ,pullreq_commit_count = NULL
,pullreq_file_count = NULL ,pullreq_file_count = NULL
,pullreq_additions = NULL ,pullreq_additions = NULL
@ -703,10 +716,13 @@ func (s *PullReqStore) applyFilter(stmt *squirrel.SelectBuilder, opts *types.Pul
} }
func mapPullReq(pr *pullReq) *types.PullReq { func mapPullReq(pr *pullReq) *types.PullReq {
var mergeConflicts []string var mergeConflicts, rebaseConflicts []string
if pr.MergeConflicts.Valid { if pr.MergeConflicts.Valid {
mergeConflicts = strings.Split(pr.MergeConflicts.String, "\n") mergeConflicts = strings.Split(pr.MergeConflicts.String, "\n")
} }
if pr.RebaseConflicts.Valid {
rebaseConflicts = strings.Split(pr.RebaseConflicts.String, "\n")
}
return &types.PullReq{ return &types.PullReq{
ID: pr.ID, ID: pr.ID,
@ -737,6 +753,8 @@ func mapPullReq(pr *pullReq) *types.PullReq {
MergeBaseSHA: pr.MergeBaseSHA, MergeBaseSHA: pr.MergeBaseSHA,
MergeSHA: pr.MergeSHA.Ptr(), MergeSHA: pr.MergeSHA.Ptr(),
MergeConflicts: mergeConflicts, MergeConflicts: mergeConflicts,
RebaseCheckStatus: pr.RebaseCheckStatus,
RebaseConflicts: rebaseConflicts,
Author: types.PrincipalInfo{}, Author: types.PrincipalInfo{},
Merger: nil, Merger: nil,
Stats: types.PullReqStats{ Stats: types.PullReqStats{
@ -754,6 +772,7 @@ func mapPullReq(pr *pullReq) *types.PullReq {
func mapInternalPullReq(pr *types.PullReq) *pullReq { func mapInternalPullReq(pr *types.PullReq) *pullReq {
mergeConflicts := strings.Join(pr.MergeConflicts, "\n") mergeConflicts := strings.Join(pr.MergeConflicts, "\n")
rebaseConflicts := strings.Join(pr.RebaseConflicts, "\n")
m := &pullReq{ m := &pullReq{
ID: pr.ID, ID: pr.ID,
Version: pr.Version, Version: pr.Version,
@ -783,6 +802,8 @@ func mapInternalPullReq(pr *types.PullReq) *pullReq {
MergeBaseSHA: pr.MergeBaseSHA, MergeBaseSHA: pr.MergeBaseSHA,
MergeSHA: null.StringFromPtr(pr.MergeSHA), MergeSHA: null.StringFromPtr(pr.MergeSHA),
MergeConflicts: null.NewString(mergeConflicts, mergeConflicts != ""), MergeConflicts: null.NewString(mergeConflicts, mergeConflicts != ""),
RebaseCheckStatus: pr.RebaseCheckStatus,
RebaseConflicts: null.NewString(rebaseConflicts, rebaseConflicts != ""),
CommitCount: null.IntFromPtr(pr.Stats.Commits), CommitCount: null.IntFromPtr(pr.Stats.Commits),
FileCount: null.IntFromPtr(pr.Stats.FilesChanged), FileCount: null.IntFromPtr(pr.Stats.FilesChanged),
Additions: null.IntFromPtr(pr.Stats.Additions), Additions: null.IntFromPtr(pr.Stats.Additions),

View File

@ -26,8 +26,6 @@ import (
"github.com/harness/gitness/git/hook" "github.com/harness/gitness/git/hook"
"github.com/harness/gitness/git/merge" "github.com/harness/gitness/git/merge"
"github.com/harness/gitness/git/sha" "github.com/harness/gitness/git/sha"
"github.com/rs/zerolog/log"
) )
// MergeParams is input structure object for merging operation. // MergeParams is input structure object for merging operation.
@ -172,16 +170,6 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
} }
} }
// logger
log := log.Ctx(ctx).With().
Str("repo_uid", params.RepoUID).
Str("head", params.HeadBranch).
Str("base", params.BaseBranch).
Str("method", string(mergeMethod)).
Str("ref", refPath).
Logger()
// find the commit SHAs // find the commit SHAs
baseCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.BaseBranch) baseCommitSHA, err := s.git.GetFullCommitID(ctx, repoPath, params.BaseBranch)
@ -225,35 +213,6 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput,
return MergeOutput{}, fmt.Errorf("failed to find commit count for merge check: %w", err) return MergeOutput{}, fmt.Errorf("failed to find commit count for merge check: %w", err)
} }
// handle simple merge check
if params.RefType == enum.RefTypeUndefined && params.Method != enum.MergeMethodRebase {
// Merge method rebase can result in conflicts even if other methods do not.
// This is because commits are rebased one by one. And this is why for other merge method we just return
// list of conflicts, but for rebase we proceed with the rebasing (even if no ref would be updated).
_, _, conflicts, err := merge.FindConflicts(ctx, repoPath, baseCommitSHA.String(), headCommitSHA.String())
if err != nil {
return MergeOutput{}, errors.Internal(err,
"Merge check failed to find conflicts between commits %s and %s",
baseCommitSHA.String(), headCommitSHA.String())
}
log.Debug().Msg("merged check completed")
return MergeOutput{
BaseSHA: baseCommitSHA,
HeadSHA: headCommitSHA,
MergeBaseSHA: mergeBaseCommitSHA,
MergeSHA: sha.None,
CommitCount: commitCount,
ChangedFileCount: shortStat.Files,
Additions: shortStat.Additions,
Deletions: shortStat.Deletions,
ConflictFiles: conflicts,
}, nil
}
// author and committer // author and committer
now := time.Now().UTC() now := time.Now().UTC()

View File

@ -51,11 +51,14 @@ type PullReq struct {
Merged *int64 `json:"merged"` Merged *int64 `json:"merged"`
MergeMethod *enum.MergeMethod `json:"merge_method"` MergeMethod *enum.MergeMethod `json:"merge_method"`
MergeCheckStatus enum.MergeCheckStatus `json:"merge_check_status"`
MergeTargetSHA *string `json:"merge_target_sha"` MergeTargetSHA *string `json:"merge_target_sha"`
MergeBaseSHA string `json:"merge_base_sha"` MergeBaseSHA string `json:"merge_base_sha"`
MergeSHA *string `json:"-"` // TODO: either remove or ensure it's being set (merge dry-run) MergeSHA *string `json:"-"` // TODO: either remove or ensure it's being set (merge dry-run)
MergeCheckStatus enum.MergeCheckStatus `json:"merge_check_status"`
MergeConflicts []string `json:"merge_conflicts,omitempty"` MergeConflicts []string `json:"merge_conflicts,omitempty"`
RebaseCheckStatus enum.MergeCheckStatus `json:"rebase_check_status"`
RebaseConflicts []string `json:"rebase_conflicts,omitempty"`
Author PrincipalInfo `json:"author"` Author PrincipalInfo `json:"author"`
Merger *PrincipalInfo `json:"merger"` Merger *PrincipalInfo `json:"merger"`
@ -64,6 +67,53 @@ type PullReq struct {
Labels []*LabelPullReqAssignmentInfo `json:"labels,omitempty"` Labels []*LabelPullReqAssignmentInfo `json:"labels,omitempty"`
} }
func (pr *PullReq) UpdateMergeOutcome(method enum.MergeMethod, conflictFiles []string) {
switch method {
case enum.MergeMethodMerge, enum.MergeMethodSquash:
if len(conflictFiles) > 0 {
pr.MergeCheckStatus = enum.MergeCheckStatusConflict
pr.MergeConflicts = conflictFiles
} else {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeConflicts = nil
}
case enum.MergeMethodRebase:
if len(conflictFiles) > 0 {
pr.RebaseCheckStatus = enum.MergeCheckStatusConflict
pr.RebaseConflicts = conflictFiles
} else {
pr.RebaseCheckStatus = enum.MergeCheckStatusMergeable
pr.RebaseConflicts = nil
}
case enum.MergeMethodFastForward:
// fast-forward merge can't have conflicts
}
}
func (pr *PullReq) MarkAsMergeUnchecked() {
pr.MergeCheckStatus = enum.MergeCheckStatusUnchecked
pr.MergeConflicts = nil
pr.RebaseCheckStatus = enum.MergeCheckStatusUnchecked
pr.RebaseConflicts = nil
}
func (pr *PullReq) MarkAsMergeable() {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeConflicts = nil
}
func (pr *PullReq) MarkAsRebaseable() {
pr.RebaseCheckStatus = enum.MergeCheckStatusMergeable
pr.RebaseConflicts = nil
}
func (pr *PullReq) MarkAsMerged() {
pr.MergeCheckStatus = enum.MergeCheckStatusMergeable
pr.MergeConflicts = nil
pr.RebaseCheckStatus = enum.MergeCheckStatusMergeable
pr.RebaseConflicts = nil
}
// DiffStats shows total number of commits and modified files. // DiffStats shows total number of commits and modified files.
type DiffStats struct { type DiffStats struct {
Commits *int64 `json:"commits,omitempty"` Commits *int64 `json:"commits,omitempty"`
@ -201,6 +251,7 @@ type MergeResponse struct {
// values only returned on dryrun // values only returned on dryrun
DryRun bool `json:"dry_run,omitempty"` DryRun bool `json:"dry_run,omitempty"`
Mergeable bool `json:"mergeable,omitempty"`
ConflictFiles []string `json:"conflict_files,omitempty"` ConflictFiles []string `json:"conflict_files,omitempty"`
AllowedMethods []enum.MergeMethod `json:"allowed_methods,omitempty"` AllowedMethods []enum.MergeMethod `json:"allowed_methods,omitempty"`
MinimumRequiredApprovalsCount int `json:"minimum_required_approvals_count,omitempty"` MinimumRequiredApprovalsCount int `json:"minimum_required_approvals_count,omitempty"`