Merge branch 'version/0-43-0-RC1' into mitchell/dx-2213

cmd/state/internal/cmdtree/cmdtree.go

@@ -313,6 +313,7 @@ func newStateCommand(globals *globalOptions, prime *primer.Values) *captain.Comm
 	cmd.SetHasVariableArguments()
 	cmd.OnExecStart(cmdCall.OnExecStart)
 	cmd.OnExecStop(cmdCall.OnExecStop)
+	cmd.SetSupportsStructuredOutput()
 
 	return cmd
 }

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/commit"
 	"github.com/ActiveState/cli/internal/runbits/commitmediator"
+	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(),
+			)
+		}
+	}
+}

internal/runners/revert/revert.go

@@ -11,7 +11,6 @@ import (
 	"github.com/ActiveState/cli/internal/runbits/commit"
 	"github.com/ActiveState/cli/internal/runbits/commitmediator"
 	gqlmodel "github.com/ActiveState/cli/pkg/platform/api/graphql/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"
 	"github.com/ActiveState/cli/pkg/platform/runtime/target"
@@ -65,39 +64,33 @@ func (r *Revert) Run(params *Params) error {
 	if err != nil {
 		return errs.Wrap(err, "Unable to get local commit")
 	}
+
 	if params.CommitID == latestCommit.String() && params.To {
 		return locale.NewInputError("err_revert_to_current_commit", "The commit to revert to cannot be the latest commit")
 	}
 	r.out.Notice(locale.Tl("operating_message", "", r.project.NamespaceString(), r.project.Dir()))
-	commitID := strfmt.UUID(params.CommitID)
 
-	var targetCommit *mono_models.Commit // the commit to revert the contents of, or the commit to revert to
-	var fromCommit, toCommit strfmt.UUID
-	if !params.To {
-		priorCommits, err := model.CommitHistoryPaged(commitID, 0, 2)
-		if err != nil {
-			return errs.AddTips(
-				locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
-				locale.T("tip_private_project_auth"),
-			)
-		}
-		if priorCommits.TotalCommits < 2 {
-			return locale.NewInputError("err_revert_no_history", "Cannot revert commit {{.V0}}: no prior history", params.CommitID)
-		}
-		targetCommit = priorCommits.Commits[0]
-		fromCommit = commitID
-		toCommit = priorCommits.Commits[1].CommitID // parent commit
-	} else {
-		var err error
-		targetCommit, err = model.GetCommitWithinCommitHistory(latestCommit, commitID)
-		if err != nil {
-			return errs.AddTips(
-				locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
-				locale.T("tip_private_project_auth"),
-			)
-		}
-		fromCommit = latestCommit
-		toCommit = targetCommit.CommitID
+	bp := model.NewBuildPlannerModel(r.auth)
+	targetCommitID := params.CommitID // the commit to revert the contents of, or the commit to revert to
+	revertParams := revertParams{
+		organization:   r.project.Owner(),
+		project:        r.project.Name(),
+		parentCommitID: latestCommit.String(),
+		revertCommitID: params.CommitID,
+	}
+	revertFunc := r.revertCommit
+	preposition := ""
+	if params.To {
+		revertFunc = r.revertToCommit
+		preposition = " to" // need leading whitespace
+	}
+
+	targetCommit, err := model.GetCommitWithinCommitHistory(latestCommit, strfmt.UUID(targetCommitID))
+	if err != nil {
+		return errs.AddTips(
+			locale.WrapError(err, "err_revert_get_commit", "", params.CommitID),
+			locale.T("tip_private_project_auth"),
+		)
 	}
 
 	var orgs []gqlmodel.Organization
@@ -108,10 +101,7 @@ func (r *Revert) Run(params *Params) error {
 			return locale.WrapError(err, "err_revert_get_organizations", "Could not get organizations for current user")
 		}
 	}
-	preposition := ""
-	if params.To {
-		preposition = " to" // need leading whitespace
-	}
+
 	if !r.out.Type().IsStructured() {
 		r.out.Print(locale.Tl("revert_info", "You are about to revert{{.V0}} the following commit:", preposition))
 		commit.PrintCommit(r.out, targetCommit, orgs)
@@ -126,41 +116,72 @@ func (r *Revert) Run(params *Params) error {
 		return locale.NewInputError("err_revert_aborted", "Revert aborted by user")
 	}
 
-	revertCommit, err := model.RevertCommitWithinHistory(fromCommit, toCommit, latestCommit)
+	revertCommit, err := revertFunc(revertParams, bp)
 	if err != nil {
 		return errs.AddTips(
 			locale.WrapError(err, "err_revert_commit", "", preposition, params.CommitID),
 			locale.Tl("tip_revert_sync", "Please ensure that the local project is synchronized with the platform and that the given commit ID belongs to the current project"),
 			locale.T("tip_private_project_auth"))
 	}
 
-	err = runbits.RefreshRuntime(r.auth, r.out, r.analytics, r.project, revertCommit.CommitID, true, target.TriggerRevert, r.svcModel)
+	err = commitmediator.Set(r.project, revertCommit.String())
 	if err != nil {
-		return locale.WrapError(err, "err_refresh_runtime")
+		return errs.Wrap(err, "Unable to set local commit")
 	}
 
-	err = commitmediator.Set(r.project, revertCommit.CommitID.String())
+	err = runbits.RefreshRuntime(r.auth, r.out, r.analytics, r.project, revertCommit, true, target.TriggerRevert, r.svcModel)
 	if err != nil {
-		return errs.Wrap(err, "Unable to set local commit")
+		return locale.WrapError(err, "err_refresh_runtime")
 	}
 
 	r.out.Print(output.Prepare(
 		locale.Tl("revert_success", "Successfully reverted{{.V0}} commit: {{.V1}}", preposition, params.CommitID),
 		&struct {
 			CurrentCommitID string `json:"current_commit_id"`
 		}{
-			revertCommit.CommitID.String(),
+			revertCommit.String(),
 		},
 	))
 	r.out.Notice(locale.T("operation_success_local"))
 	return nil
 }
 
-func containsCommitID(history []*mono_models.Commit, commitID strfmt.UUID) bool {
-	for _, c := range history {
-		if c.CommitID == commitID {
-			return true
-		}
+type revertFunc func(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error)
+
+type revertParams struct {
+	organization   string
+	project        string
+	parentCommitID string
+	revertCommitID string
+}
+
+func (r *Revert) revertCommit(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error) {
+	newCommitID, err := bp.RevertCommit(params.organization, params.project, params.parentCommitID, params.revertCommitID)
+	if err != nil {
+		return "", errs.Wrap(err, "Could not revert commit")
+	}
+
+	return newCommitID, nil
+}
+
+func (r *Revert) revertToCommit(params revertParams, bp *model.BuildPlanner) (strfmt.UUID, error) {
+	buildExpression, err := bp.GetBuildExpression(params.organization, params.project, params.revertCommitID)
+	if err != nil {
+		return "", errs.Wrap(err, "Could not get build expression")
+	}
+
+	stageCommitParams := model.StageCommitParams{
+		Owner:        params.organization,
+		Project:      params.project,
+		ParentCommit: params.parentCommitID,
+		Description:  locale.Tl("revert_commit_description", "Revert to commit {{.V0}}", params.revertCommitID),
+		Expression:   buildExpression,
 	}
-	return false
+
+	newCommitID, err := bp.StageCommit(stageCommitParams)
+	if err != nil {
+		return "", errs.Wrap(err, "Could not stage commit")
+	}
+
+	return newCommitID, nil
 }