From a5413a5f98efdd3b3bbf20b8cfa522ecab52c0b6 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 31 Oct 2023 13:28:38 -0400 Subject: [PATCH 1/2] Replace `attachStagedCommit` with `mergeCommit(..., strategy=FastForward)`. --- internal/runners/push/push.go | 42 +++------- internal/runners/push/rationalize.go | 36 ++++++--- .../api/buildplanner/model/buildplan.go | 40 +++++++++- .../request/attachstagedcommit.go | 60 --------------- .../api/buildplanner/request/mergecommit.go | 76 +++++++++++++++++++ pkg/platform/model/buildplanner.go | 64 +++++++++------- 6 files changed, 185 insertions(+), 133 deletions(-) delete mode 100644 pkg/platform/api/buildplanner/request/attachstagedcommit.go create mode 100644 pkg/platform/api/buildplanner/request/mergecommit.go diff --git a/internal/runners/push/push.go b/internal/runners/push/push.go index c21a9765ed..9409dcf5f2 100644 --- a/internal/runners/push/push.go +++ b/internal/runners/push/push.go @@ -14,6 +14,7 @@ import ( "github.com/ActiveState/cli/internal/rtutils/ptr" "github.com/ActiveState/cli/internal/runbits/rationalize" "github.com/ActiveState/cli/pkg/localcommit" + bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" "github.com/ActiveState/cli/pkg/platform/api/mono/mono_models" "github.com/ActiveState/cli/pkg/platform/authentication" "github.com/ActiveState/cli/pkg/platform/model" @@ -63,10 +64,8 @@ const ( ) var ( - errNoChanges = errors.New("no changes") - errNoCommit = errors.New("no commit") - errTargetInvalidHistory = errors.New("local and remove histories do not match") - errPullNeeded = errors.New("pull needed") + errNoChanges = errors.New("no changes") + errNoCommit = errors.New("no commit") ) type errProjectNameInUse struct { @@ -230,35 +229,16 @@ func (r *Push) Run(params PushParams) (rerr error) { return errNoChanges } - // Check whether there is a conflict - if branch.CommitID != nil { - mergeStrategy, err := model.MergeCommit(*branch.CommitID, commitID) - if err != nil { - if errors.Is(err, model.ErrMergeCommitInHistory) { - return errNoChanges - } - if !errors.Is(err, model.ErrMergeFastForward) { - if params.Namespace.IsValid() { - return errTargetInvalidHistory - } - return errs.Wrap(err, "Could not detect if merge is necessary") - } - } - if mergeStrategy != nil { - return errPullNeeded - } - } - - // Perform the push. - err = bp.AttachStagedCommit(&model.AttachStagedCommitParams{ - Owner: targetNamespace.Owner, - Project: targetNamespace.Project, - ParentCommitID: *branch.CommitID, - StagedCommitID: commitID, - Branch: branch.BranchID.String(), + // Perform the (fast-forward) push. + _, err = bp.MergeCommit(&model.MergeCommitParams{ + Owner: targetNamespace.Owner, + Project: targetNamespace.Project, + TargetRef: branch.Label, // using branch name will fast-forward + OtherRef: commitID.String(), + Strategy: bpModel.MergeCommitStrategyFastForward, }) if err != nil { - return errs.Wrap(err, "Could not attach staged commit") + return errs.Wrap(err, "Could not push") } } diff --git a/internal/runners/push/rationalize.go b/internal/runners/push/rationalize.go index 8f574ab991..881f381121 100644 --- a/internal/runners/push/rationalize.go +++ b/internal/runners/push/rationalize.go @@ -6,6 +6,7 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/runbits/rationalize" + bpModel "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" ) func rationalizeError(err *error) { @@ -17,6 +18,8 @@ func rationalizeError(err *error) { var headlessErr *errHeadless + var mergeCommitErr *bpModel.MergedCommitError + switch { // Not authenticated @@ -68,17 +71,28 @@ func rationalizeError(err *error) { locale.T("err_push_create_project_aborted"), errs.SetInput()) - // Custom target does not have a compatible history - case errors.Is(*err, errTargetInvalidHistory): - *err = errs.WrapUserFacing(*err, - locale.T("err_push_target_invalid_history"), - errs.SetInput()) + case errors.As(*err, &mergeCommitErr): + switch mergeCommitErr.Type { + // Need to pull first + case bpModel.FastForwardErrorType: + *err = errs.WrapUserFacing(*err, + locale.T("err_push_outdated"), + errs.SetInput(), + errs.SetTips(locale.T("err_tip_push_outdated"))) - // Need to pull first - case errors.Is(*err, errPullNeeded): - *err = errs.WrapUserFacing(*err, - locale.T("err_push_outdated"), - errs.SetInput(), - errs.SetTips(locale.T("err_tip_push_outdated"))) + // Custom target does not have a compatible history + case bpModel.NoCommonBaseFoundType: + *err = errs.WrapUserFacing(*err, + locale.T("err_push_target_invalid_history"), + errs.SetInput()) + + // No changes made + case bpModel.NoChangeSinceLastCommitErrorType: + *err = errs.WrapUserFacing(*err, + locale.T("push_no_changes"), + errs.SetInput(), + ) + + } } } diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index 3217f41411..c450bea089 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -13,6 +13,8 @@ import ( type Operation int +type MergeStrategy string + const ( OperationAdded Operation = iota OperationRemoved @@ -60,6 +62,12 @@ const ( XGozipInstallerMimeType = "application/x-gozip-installer" XActiveStateBuilderMimeType = "application/x-activestate-builder" + // MergeCommit strategies + MergeCommitStrategyRecursive MergeStrategy = "Recursive" + MergeCommitStrategyRecursiveOverwriteOnConflict MergeStrategy = "RecursiveOverwriteOnConflict" + MergeCommitStrategyRecursiveKeepOnConflict MergeStrategy = "RecursiveKeepOnConflict" + MergeCommitStrategyFastForward MergeStrategy = "FastForward" + // Error types ErrorType = "Error" NotFoundErrorType = "NotFound" @@ -71,6 +79,9 @@ const ( ForbiddenErrorType = "Forbidden" RemediableSolveErrorType = "RemediableSolveError" PlanningErrorType = "PlanningError" + MergeConflictType = "MergeConflict" + FastForwardErrorType = "FastForwardError" + NoCommonBaseFoundType = "NoCommonBaseFound" ) func IsStateToolArtifact(mimeType string) bool { @@ -268,7 +279,10 @@ func IsErrorResponse(errorType string) bool { errorType == ForbiddenErrorType || errorType == RemediableSolveErrorType || errorType == PlanningErrorType || - errorType == NotFoundErrorType + errorType == NotFoundErrorType || + errorType == MergeConflictType || + errorType == FastForwardErrorType || + errorType == NoCommonBaseFoundType } func ProcessCommitError(commit *Commit, fallbackMessage string) error { @@ -354,6 +368,20 @@ type BuildExpression struct { *Error } +type MergedCommitError struct { + Type string + Message string +} + +func (m *MergedCommitError) Error() string { return m.Message } + +func ProcessMergedCommitError(mcErr *mergedCommit, fallbackMessage string) error { + if mcErr.Type != "" { + return &MergedCommitError{mcErr.Type, mcErr.Message} + } + return errs.New(fallbackMessage) +} + // PushCommitResult is the result of a push commit mutation. // It contains the resulting commit from the operation and any errors. // The resulting commit is pushed to the platform automatically. @@ -380,12 +408,18 @@ type CreateProjectResult struct { ProjectCreated *projectCreated `json:"createProject"` } -type AttachStagedCommitResult struct { +type mergedCommit struct { Type string `json:"__typename"` - Commit *Commit `json:"attachStagedCommit"` + Commit *Commit `json:"commit"` *Error } +// MergeCommitResult is the result of a merge commit mutation. +// The resulting commit is NOT pushed to the platform automatically. +type MergeCommitResult struct { + MergedCommit *mergedCommit `json:"mergeCommit"` +} + // Error contains an error message. type Error struct { Message string `json:"message"` diff --git a/pkg/platform/api/buildplanner/request/attachstagedcommit.go b/pkg/platform/api/buildplanner/request/attachstagedcommit.go deleted file mode 100644 index 48b5620f4f..0000000000 --- a/pkg/platform/api/buildplanner/request/attachstagedcommit.go +++ /dev/null @@ -1,60 +0,0 @@ -package request - -func AttachStagedCommit(owner, project, parentCommit, commit, branch string) *attachStagedCommit { - return &attachStagedCommit{map[string]interface{}{ - "organization": owner, - "project": project, - "parentCommit": parentCommit, - "commit": commit, - "branch": branch, - }} -} - -type attachStagedCommit struct { - vars map[string]interface{} -} - -func (b *attachStagedCommit) Query() string { - return ` -mutation ($organization: String!, $project: String!, $parentCommit: ID!, $commit: ID!, $branch: String!) { - attachStagedCommit(input:{organization:$organization, project:$project, parentCommitId:$parentCommit, stagedCommitId:$commit, branchRef:$branch}) { - ... on Commit { - __typename - commitId - } - ... on NotFound { - __typename - message - mayNeedAuthentication - } - ... on ParseError { - __typename - message - } - ... on ValidationError { - __typename - message - } - ... on Forbidden { - __typename - message - } - ... on HeadOnBranchMoved { - __typename - message - commitId - branchId - } - ... on NoChangeSinceLastCommit { - __typename - message - commitId - } - } -} -` -} - -func (b *attachStagedCommit) Vars() map[string]interface{} { - return b.vars -} diff --git a/pkg/platform/api/buildplanner/request/mergecommit.go b/pkg/platform/api/buildplanner/request/mergecommit.go new file mode 100644 index 0000000000..d4ce1ace25 --- /dev/null +++ b/pkg/platform/api/buildplanner/request/mergecommit.go @@ -0,0 +1,76 @@ +package request + +import "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" + +func MergeCommit(owner, project, targetRef, otherRef string, strategy model.MergeStrategy) *mergeCommit { + return &mergeCommit{map[string]interface{}{ + "organization": owner, + "project": project, + "targetRef": targetRef, + "otherRef": otherRef, + "strategy": strategy, + }} +} + +type mergeCommit struct { + vars map[string]interface{} +} + +func (b *mergeCommit) Query() string { + return ` +mutation ($organization: String!, $project: String!, $targetRef: String!, $otherRef: String!, $strategy: MergeStrategy) { + mergeCommit(input:{organization:$organization, project:$project, targetVcsRef:$targetRef, otherVcsRef:$otherRef, strategy:$strategy}) { + ... on MergedCommit { + commit { + __typename + commitId + } + } + ... on MergeConflict { + __typename + message + } + ... on FastForwardError { + __typename + message + } + ... on NoCommonBaseFound { + __typename + message + } + ... on NotFound { + __typename + message + mayNeedAuthentication + } + ... on ParseError { + __typename + message + } + ... on ValidationError { + __typename + message + } + ... on Forbidden { + __typename + message + } + ... on HeadOnBranchMoved { + __typename + message + commitId + branchId + } + ... on NoChangeSinceLastCommit { + __typename + message + commitId + } + } +} +` +} + +func (b *mergeCommit) Vars() map[string]interface{} { + return b.vars +} diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index bbfcc5cabe..f47dad2ec1 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -297,34 +297,6 @@ func (bp *BuildPlanner) StageCommit(params StageCommitParams) (strfmt.UUID, erro return resp.Commit.CommitID, nil } -type AttachStagedCommitParams struct { - Owner string - Project string - ParentCommitID strfmt.UUID - StagedCommitID strfmt.UUID - Branch string -} - -func (bp *BuildPlanner) AttachStagedCommit(params *AttachStagedCommitParams) error { - logging.Debug("AttachStagedCommit, owner: %s, project: %s", params.Owner, params.Project) - request := request.AttachStagedCommit(params.Owner, params.Project, params.ParentCommitID.String(), params.StagedCommitID.String(), params.Branch) - resp := &bpModel.AttachStagedCommitResult{} - err := bp.client.Run(request, resp) - if err != nil { - return processBuildPlannerError(err, "Failed to attach staged commit") - } - - if resp.Commit == nil { - return errs.New("Attached, staged commit is nil") - } - - if bpModel.IsErrorResponse(resp.Commit.Type) { - return bpModel.ProcessCommitError(resp.Commit, "Could not process error response from attach stage commit") - } - - return nil -} - func (bp *BuildPlanner) GetBuildExpression(owner, project, commitID string) (*buildexpression.BuildExpression, error) { logging.Debug("GetBuildExpression, owner: %s, project: %s, commitID: %s", owner, project, commitID) resp := &bpModel.BuildExpression{} @@ -423,6 +395,42 @@ func (bp *BuildPlanner) CreateProject(params *CreateProjectParams) (strfmt.UUID, return resp.ProjectCreated.Commit.CommitID, nil } +type MergeCommitParams struct { + Owner string + Project string + TargetRef string // the commit ID or branch name to merge into + OtherRef string // the commit ID or branch name to merge from + Strategy model.MergeStrategy +} + +func (bp *BuildPlanner) MergeCommit(params *MergeCommitParams) (strfmt.UUID, error) { + logging.Debug("MergeCommit, owner: %s, project: %s", params.Owner, params.Project) + request := request.MergeCommit(params.Owner, params.Project, params.TargetRef, params.OtherRef, params.Strategy) + resp := &bpModel.MergeCommitResult{} + err := bp.client.Run(request, resp) + if err != nil { + return "", processBuildPlannerError(err, "Failed to merge commit") + } + + if resp.MergedCommit == nil { + return "", errs.New("MergedCommit is nil") + } + + if bpModel.IsErrorResponse(resp.MergedCommit.Type) { + return "", bpModel.ProcessMergedCommitError(resp.MergedCommit, "Could not merge commit") + } + + if resp.MergedCommit.Commit == nil { + return "", errs.New("Merge commit's commit is nil'") + } + + if bpModel.IsErrorResponse(resp.MergedCommit.Commit.Type) { + return "", bpModel.ProcessCommitError(resp.MergedCommit.Commit, "Could not process error response from merge commit") + } + + return resp.MergedCommit.Commit.CommitID, nil +} + // processBuildPlannerError will check for special error types that should be // handled differently. If no special error type is found, the fallback message // will be used. From e16d2469b1f2ddc35b88b3105913e9ac7942a649 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 31 Oct 2023 18:08:09 -0400 Subject: [PATCH 2/2] Updated comment. --- pkg/platform/api/buildplanner/model/buildplan.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index c450bea089..de7129aa59 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -415,7 +415,8 @@ type mergedCommit struct { } // MergeCommitResult is the result of a merge commit mutation. -// The resulting commit is NOT pushed to the platform automatically. +// The resulting commit is only pushed to the platform automatically if the target ref was a named +// branch and the merge strategy was FastForward. type MergeCommitResult struct { MergedCommit *mergedCommit `json:"mergeCommit"` }