diff --git a/app/api/controller/pullreq/merge.go b/app/api/controller/pullreq/merge.go index f2dac6911..7d5e14593 100644 --- a/app/api/controller/pullreq/merge.go +++ b/app/api/controller/pullreq/merge.go @@ -389,7 +389,7 @@ func (c *Controller) Merge( Method: gitenum.MergeMethod(in.Method), }) if err != nil { - return nil, nil, fmt.Errorf("merge check execution failed: %w", err) + return nil, nil, fmt.Errorf("merge execution failed: %w", err) } //nolint:nestif if mergeOutput.MergeSHA.String() == "" || len(mergeOutput.ConflictFiles) > 0 { diff --git a/app/api/controller/repo/rebase.go b/app/api/controller/repo/rebase.go index 436af84f1..1e5fd9c35 100644 --- a/app/api/controller/repo/rebase.go +++ b/app/api/controller/repo/rebase.go @@ -34,6 +34,7 @@ type RebaseInput struct { HeadBranch string `json:"head_branch"` HeadCommitSHA sha.SHA `json:"head_commit_sha"` + DryRun bool `json:"dry_run"` DryRunRules bool `json:"dry_run_rules"` BypassRules bool `json:"bypass_rules"` } @@ -90,11 +91,10 @@ func (c *Controller) Rebase( } if in.DryRunRules { + // DryRunRules is true: Just return rule violations and don't attempt to rebase. return &types.RebaseResponse{ - DryRunRulesOutput: types.DryRunRulesOutput{ - DryRunRules: true, - RuleViolations: violations, - }, + RuleViolations: violations, + DryRunRules: true, }, nil, nil } @@ -125,7 +125,7 @@ func (c *Controller) Rebase( if !headBranch.Branch.SHA.Equal(in.HeadCommitSHA) { return nil, nil, usererror.BadRequestf("The commit %s isn't the latest commit on the branch %s", - in.HeadCommitSHA, headBranch.Branch.SHA.String()) + in.HeadCommitSHA, headBranch.Branch.Name) } isAncestor, err := c.git.IsAncestor(ctx, git.IsAncestorParams{ @@ -140,9 +140,8 @@ func (c *Controller) Rebase( if isAncestor.Ancestor { // The head branch already contains the latest commit from the base branch - nothing to do. return &types.RebaseResponse{ - DryRunRulesOutput: types.DryRunRulesOutput{ - RuleViolations: violations, - }, + AlreadyAncestor: true, + RuleViolations: violations, }, nil, nil } @@ -151,13 +150,20 @@ func (c *Controller) Rebase( return nil, nil, fmt.Errorf("failed to create RPC write params: %w", err) } + refType := gitenum.RefTypeBranch + refName := in.HeadBranch + if in.DryRun { + refType = gitenum.RefTypeUndefined + refName = "" + } + mergeOutput, err := c.git.Merge(ctx, &git.MergeParams{ WriteParams: writeParams, BaseBranch: in.BaseBranch, HeadRepoUID: repo.GitUID, HeadBranch: in.HeadBranch, - RefType: gitenum.RefTypeBranch, - RefName: in.HeadBranch, + RefType: refType, + RefName: refName, HeadExpectedSHA: in.HeadCommitSHA, Method: gitenum.MergeMethodRebase, }) @@ -165,6 +171,16 @@ func (c *Controller) Rebase( return nil, nil, fmt.Errorf("rebase execution failed: %w", err) } + if in.DryRun { + // DryRun is true: Just return rule violations and list of conflicted files. + // No reference is updated, so don't return the resulting commit SHA. + return &types.RebaseResponse{ + RuleViolations: violations, + DryRun: true, + ConflictFiles: mergeOutput.ConflictFiles, + }, nil, nil + } + if mergeOutput.MergeSHA.IsEmpty() || len(mergeOutput.ConflictFiles) > 0 { return nil, &types.MergeViolations{ ConflictFiles: mergeOutput.ConflictFiles, @@ -174,9 +190,7 @@ func (c *Controller) Rebase( } return &types.RebaseResponse{ - DryRunRulesOutput: types.DryRunRulesOutput{ - RuleViolations: violations, - }, NewHeadBranchSHA: mergeOutput.MergeSHA, + RuleViolations: violations, }, nil, nil } diff --git a/app/services/usergroup/list_users.go b/app/services/usergroup/list_users.go index cb19b7312..e6f480fbf 100644 --- a/app/services/usergroup/list_users.go +++ b/app/services/usergroup/list_users.go @@ -16,7 +16,6 @@ package usergroup import ( "context" - "fmt" "github.com/harness/gitness/app/auth" "github.com/harness/gitness/types" @@ -27,9 +26,9 @@ func (s *searchService) ListUsers( _ *auth.Session, _ *types.UserGroup, ) ([]string, error) { - return nil, fmt.Errorf("not implemented") + return nil, nil } func (s *searchService) ListUserIDsByGroupIDs(_ context.Context, _ []int64) ([]int64, error) { - return nil, fmt.Errorf("not implemented") + return nil, nil } diff --git a/git/hook/refupdate.go b/git/hook/refupdate.go index 25a64ab2e..ad6082c0f 100644 --- a/git/hook/refupdate.go +++ b/git/hook/refupdate.go @@ -130,6 +130,10 @@ func (u *RefUpdater) Init(ctx context.Context, oldValue, newValue sha.SHA) error } func (u *RefUpdater) InitOld(ctx context.Context, oldValue sha.SHA) error { + if u == nil { + return nil + } + if u.state != stateInitOld { return fmt.Errorf("invalid operation order: init old requires state=%s, current state=%s", stateInitOld, u.state) @@ -154,6 +158,10 @@ func (u *RefUpdater) InitOld(ctx context.Context, oldValue sha.SHA) error { } func (u *RefUpdater) InitNew(_ context.Context, newValue sha.SHA) error { + if u == nil { + return nil + } + if u.state != stateInitNew { return fmt.Errorf("invalid operation order: init new requires state=%s, current state=%s", stateInitNew, u.state) diff --git a/git/merge.go b/git/merge.go index d6daa1115..7c76abfb7 100644 --- a/git/merge.go +++ b/git/merge.go @@ -227,7 +227,11 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, // handle simple merge check - if params.RefType == enum.RefTypeUndefined { + 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, @@ -281,13 +285,17 @@ func (s *Service) Merge(ctx context.Context, params *MergeParams) (MergeOutput, // merge - refUpdater, err := hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, refPath) - if err != nil { - return MergeOutput{}, errors.Internal(err, "failed to create ref updater object") - } + var refUpdater *hook.RefUpdater - if err := refUpdater.InitOld(ctx, refOldValue); err != nil { - return MergeOutput{}, errors.Internal(err, "failed to set old reference value for ref updater") + if params.RefType != enum.RefTypeUndefined { + refUpdater, err = hook.CreateRefUpdater(s.hookClientFactory, params.EnvVars, repoPath, refPath) + if err != nil { + return MergeOutput{}, errors.Internal(err, "failed to create ref updater object") + } + + if err := refUpdater.InitOld(ctx, refOldValue); err != nil { + return MergeOutput{}, errors.Internal(err, "failed to set old reference value for ref updater") + } } mergeCommitSHA, conflicts, err := mergeFunc( diff --git a/types/rebase.go b/types/rebase.go index 33d02eb65..df10afbda 100644 --- a/types/rebase.go +++ b/types/rebase.go @@ -14,11 +14,14 @@ package types -import ( - "github.com/harness/gitness/git/sha" -) +import "github.com/harness/gitness/git/sha" type RebaseResponse struct { - DryRunRulesOutput - NewHeadBranchSHA sha.SHA `json:"new_head_branch_sha"` + AlreadyAncestor bool `json:"already_ancestor,omitempty"` + NewHeadBranchSHA sha.SHA `json:"new_head_branch_sha"` + RuleViolations []RuleViolations `json:"rule_violations,omitempty"` + + DryRunRules bool `json:"dry_run_rules,omitempty"` + DryRun bool `json:"dry_run,omitempty"` + ConflictFiles []string `json:"conflict_files,omitempty"` }