Merge branch version/0-42-0-RC1 to adopt changes from PR #2863

internal/runners/pull/pull.go

@@ -1,7 +1,6 @@
 package pull
 
 import (
-	"errors"
 	"path/filepath"
 	"strings"
 
@@ -10,13 +9,15 @@ import (
 	"github.com/ActiveState/cli/internal/constants"
 	"github.com/ActiveState/cli/internal/errs"
 	"github.com/ActiveState/cli/internal/locale"
+	"github.com/ActiveState/cli/internal/logging"
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/primer"
 	"github.com/ActiveState/cli/internal/prompt"
 	"github.com/ActiveState/cli/internal/runbits"
 	buildscriptRunbits "github.com/ActiveState/cli/internal/runbits/buildscript"
 	"github.com/ActiveState/cli/internal/runbits/commitmediator"
 	"github.com/ActiveState/cli/pkg/cmdlets/commit"
+	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"
@@ -77,7 +78,9 @@ func (o *pullOutput) MarshalStructured(format output.Format) interface{} {
 	return o
 }
 
-func (p *Pull) Run(params *PullParams) error {
+func (p *Pull) Run(params *PullParams) (rerr error) {
+	defer rationalizeError(&rerr)
+
 	if p.project == nil {
 		return locale.NewInputError("err_no_project")
 	}
@@ -125,21 +128,31 @@ func (p *Pull) Run(params *PullParams) error {
 	resultingCommit := remoteCommit // resultingCommit is the commit we want to update the local project file with
 
 	if localCommit != nil {
-		strategies, err := model.MergeCommit(*remoteCommit, *localCommit)
-		if err != nil {
-			if errors.Is(err, model.ErrMergeFastForward) {
-				// No merge necessary
-				resultingCommit = localCommit
-			} else if !errors.Is(err, model.ErrMergeCommitInHistory) {
-				return locale.WrapError(err, "err_mergecommit", "Could not detect if merge is necessary.")
-			}
+		// Attempt to fast-forward merge. This will succeed if the commits are
+		// compatible, meaning that we can simply update the local commit ID to
+		// the remoteCommit ID. The commitID returned from MergeCommit with this
+		// strategy should just be the remote commit ID.
+		// If this call fails then we will try a recursive merge.
+		bp := model.NewBuildPlannerModel(p.auth)
+		params := &model.MergeCommitParams{
+			Owner:     remoteProject.Owner,
+			Project:   remoteProject.Project,
+			TargetRef: localCommit.String(),
+			OtherRef:  remoteCommit.String(),
+			Strategy:  bpModel.MergeCommitStrategyFastForward,
 		}
-		if err == nil && strategies != nil {
-			c, err := p.performMerge(strategies, *remoteCommit, *localCommit, remoteProject, p.project.BranchName())
+
+		resultCommit, mergeErr := bp.MergeCommit(params)
+		if mergeErr != nil {
+			logging.Debug("Merge with fast-forward failed with error: %s, trying recursive overwrite", mergeErr.Error())
+			c, err := p.performMerge(*remoteCommit, *localCommit, remoteProject, p.project.BranchName())
 			if err != nil {
 				return errs.Wrap(err, "performing merge commit failed")
 			}
 			resultingCommit = &c
+		} else {
+			logging.Debug("Fast-forward merge succeeded, setting commit ID to %s", resultCommit.String())
+			resultingCommit = &resultCommit
 		}
 	}
 
@@ -180,7 +193,7 @@ func (p *Pull) Run(params *PullParams) error {
 	return nil
 }
 
-func (p *Pull) performMerge(strategies *mono_models.MergeStrategies, remoteCommit strfmt.UUID, localCommit strfmt.UUID, namespace *project.Namespaced, branchName string) (strfmt.UUID, error) {
+func (p *Pull) performMerge(remoteCommit, localCommit strfmt.UUID, namespace *project.Namespaced, branchName string) (strfmt.UUID, error) {
 	// Re-enable in DX-2307.
 	//err := p.mergeBuildScript(strategies, remoteCommit)
 	//if err != nil {
@@ -190,15 +203,18 @@ func (p *Pull) performMerge(strategies *mono_models.MergeStrategies, remoteCommi
 	p.out.Notice(output.Title(locale.Tl("pull_diverged", "Merging history")))
 	p.out.Notice(locale.Tr(
 		"pull_diverged_message",
-		namespace.String(), branchName, localCommit.String(), remoteCommit.String()))
+		namespace.String(), branchName, localCommit.String(), remoteCommit.String()),
+	)
 
-	commitID, err := commitmediator.Get(p.project)
-	if err != nil {
-		return "", errs.Wrap(err, "Unable to get local commit")
+	bp := model.NewBuildPlannerModel(p.auth)
+	params := &model.MergeCommitParams{
+		Owner:     namespace.Owner,
+		Project:   namespace.Project,
+		TargetRef: localCommit.String(),
+		OtherRef:  remoteCommit.String(),
+		Strategy:  bpModel.MergeCommitStrategyRecursiveOverwriteOnConflict,
 	}
-
-	commitMessage := locale.Tr("pull_merge_commit", remoteCommit.String(), commitID.String())
-	resultCommit, err := model.CommitChangeset(remoteCommit, commitMessage, strategies.OverwriteChanges)
+	resultCommit, err := bp.MergeCommit(params)
 	if err != nil {
 		return "", locale.WrapError(err, "err_pull_merge_commit", "Could not create merge commit.")
 	}
@@ -210,7 +226,8 @@ func (p *Pull) performMerge(strategies *mono_models.MergeStrategies, remoteCommi
 	changes, _ := commit.FormatChanges(cmit)
 	p.out.Notice(locale.Tl(
 		"pull_diverged_changes",
-		"The following changes will be merged:\n{{.V0}}\n", strings.Join(changes, "\n")))
+		"The following changes will be merged:\n{{.V0}}\n", strings.Join(changes, "\n")),
+	)
 
 	return resultCommit, nil
 }

internal/runners/pull/rationalize.go

@@ -0,0 +1,31 @@
+package pull
+
+import (
+	"errors"
+
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/internal/locale"
+	"github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"
+)
+
+func rationalizeError(err *error) {
+	if err == nil {
+		return
+	}
+
+	var mergeCommitErr *model.MergedCommitError
+
+	switch {
+	case errors.As(*err, &mergeCommitErr):
+		switch mergeCommitErr.Type {
+		// Custom target does not have a compatible history
+		case model.NoCommonBaseFoundType:
+			*err = errs.WrapUserFacing(*err,
+				locale.Tl("err_pull_no_common_base",
+					"Could not merge, no common base found between local and remote commits",
+				),
+				errs.SetInput(),
+			)
+		}
+	}
+}

pkg/platform/api/buildplanner/model/buildplan.go

@@ -89,6 +89,8 @@ const (
 	MergeConflictType                = "MergeConflict"
 	FastForwardErrorType             = "FastForwardError"
 	NoCommonBaseFoundType            = "NoCommonBaseFound"
+	ValidationErrorType              = "ValidationError"
+	MergeConflictErrorType           = "MergeConflict"
 )
 
 func IsStateToolArtifact(mimeType string) bool {
@@ -289,7 +291,8 @@ func IsErrorResponse(errorType string) bool {
 		errorType == NotFoundErrorType ||
 		errorType == MergeConflictType ||
 		errorType == FastForwardErrorType ||
-		errorType == NoCommonBaseFoundType
+		errorType == NoCommonBaseFoundType ||
+		errorType == ValidationErrorType
 }
 
 func ProcessCommitError(commit *Commit, fallbackMessage string) error {
@@ -409,6 +412,9 @@ type projectCreated struct {
 	Type   string  `json:"__typename"`
 	Commit *Commit `json:"commit"`
 	*Error
+	*NotFoundError
+	*ParseError
+	*ForbiddenError
 }
 
 type CreateProjectResult struct {
@@ -431,6 +437,13 @@ type mergedCommit struct {
 	Type   string  `json:"__typename"`
 	Commit *Commit `json:"commit"`
 	*Error
+	*MergeConflictError
+	*MergeError
+	*NotFoundError
+	*ParseError
+	*ForbiddenError
+	*HeadOnBranchMovedError
+	*NoChangeSinceLastCommitError
 }
 
 // MergeCommitResult is the result of a merge commit mutation.
@@ -642,6 +655,20 @@ type NoChangeSinceLastCommitError struct {
 	NoChangeCommitID strfmt.UUID `json:"commitId"`
 }
 
+// MergeConflictError represents an error that occurred because of a merge conflict.
+type MergeConflictError struct {
+	CommonAncestorID strfmt.UUID `json:"commonAncestorId"`
+	ConflictPaths    []string    `json:"conflictPaths"`
+}
+
+// MergeError represents two different errors in the BuildPlanner's graphQL
+// schema with the same fields. Those errors being: FastForwardError and
+// NoCommonBaseFound. Inspect the Type field to determine which error it is.
+type MergeError struct {
+	TargetVCSRef strfmt.UUID `json:"targetVcsRef"`
+	OtherVCSRef  strfmt.UUID `json:"otherVcsRef"`
+}
+
 // BuildExprLocation represents a location in the build script where an error occurred.
 type BuildExprLocation struct {
 	Type             string                        `json:"__typename"`

test/integration/pull_int_test.go

@@ -76,7 +76,7 @@ func (suite *PullIntegrationTestSuite) TestPullSetProjectUnrelated() {
 	cp.ExpectNotExitCode(0)
 
 	cp = ts.Spawn("pull", "--non-interactive", "--set-project", "ActiveState-CLI/Python3")
-	cp.Expect("Could not detect common parent")
+	cp.Expect("no common base")
 	cp.ExpectExitCode(1)
 }
 
@@ -130,7 +130,7 @@ func (suite *PullIntegrationTestSuite) TestPull_RestoreNamespace() {
 
 	// Attempt to update to unrelated project.
 	cp := ts.Spawn("pull", "--non-interactive", "--set-project", "ActiveState-CLI/Python3")
-	cp.Expect("Could not detect common parent")
+	cp.Expect("no common base")
 	cp.ExpectNotExitCode(0)
 
 	// Verify namespace is unchanged.