Merge pull request #2959 from ActiveState/DX-2343

Requirement operations do not set commit ID on build failure

internal/runbits/refresh.go

@@ -29,7 +29,7 @@ func RefreshRuntime(
 	//if err != nil {
 	//	return locale.WrapError(err, "err_update_build_script")
 	//}
-	target := target.NewProjectTarget(proj, nil, trigger)
+	target := target.NewProjectTarget(proj, resolveCommitID(proj, &commitID), trigger)
 	isCached := true
 	rt, err := runtime.New(target, an, svcm, auth)
 	if err != nil {
@@ -64,3 +64,16 @@ func RefreshRuntime(
 
 	return nil
 }
+
+func resolveCommitID(proj *project.Project, customCommitID *strfmt.UUID) *strfmt.UUID {
+	var projectCommitID *strfmt.UUID
+	if proj != nil && proj.Namespace() != nil && proj.Namespace().CommitID != nil {
+		projectCommitID = proj.Namespace().CommitID
+	}
+
+	if projectCommitID != customCommitID {
+		return customCommitID
+	}
+
+	return nil
+}

internal/runbits/requirements/requirements.go

@@ -297,21 +297,14 @@ func (r *RequirementOperation) ExecuteRequirementOperation(
 	//	return errs.Wrap(err, "Could not get remote build expr")
 	//}
 
-	if err := commitmediator.Set(r.Project, commitID.String()); err != nil {
-		return locale.WrapError(err, "err_package_update_commit_id")
-	}
-
-	// Note: a commit ID file needs to exist at this point.
-	// Re-enable in DX-2307.
-	//err = buildscript.Update(r.Project, expr, r.Auth)
-	//if err != nil {
-	//	return locale.WrapError(err, "err_update_build_script")
-	//}
-
 	// refresh or install runtime
 	err = runbits.RefreshRuntime(r.Auth, r.Output, r.Analytics, r.Project, commitID, true, trigger, r.SvcModel)
 	if err != nil {
-		return err
+		return handleRefreshError(err, r.Project, parentCommitID)
+	}
+
+	if err := updateCommitID(r.Project, commitID); err != nil {
+		return locale.WrapError(err, "err_package_update_commit_id")
 	}
 
 	if !hasParentCommit {
@@ -348,6 +341,32 @@ func (r *RequirementOperation) ExecuteRequirementOperation(
 	return nil
 }
 
+func handleRefreshError(err error, project *project.Project, parentCommitID strfmt.UUID) error {
+	// If the error is a build error then return, if not update the commit ID then return
+	if !runbits.IsBuildError(err) {
+		if err := updateCommitID(project, parentCommitID); err != nil {
+			return locale.WrapError(err, "err_package_update_commit_id")
+		}
+	}
+	return err
+}
+
+func updateCommitID(project *project.Project, commitID strfmt.UUID) error {
+	if err := commitmediator.Set(project, commitID.String()); err != nil {
+		return locale.WrapError(err, "err_package_update_commit_id")
+	}
+
+	// Note: a commit ID file needs to exist at this point.
+	// Re-enable in DX-2307.
+	// Will have to pass the buildscript as an argument to this function.
+	//err = buildscript.Update(r.Project, expr, r.Auth)
+	//if err != nil {
+	//	return locale.WrapError(err, "err_update_build_script")
+	//}
+
+	return nil
+}
+
 func supportedLanguageByName(supported []medmodel.SupportedLanguage, langName string) medmodel.SupportedLanguage {
 	return funk.Find(supported, func(l medmodel.SupportedLanguage) bool { return l.Name == langName }).(medmodel.SupportedLanguage)
 }

internal/runbits/runbits.go

@@ -1,2 +1,17 @@
 // Package runbits comprises logic that is shared between controllers, ie., code that prints
 package runbits
+
+import (
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/pkg/platform/api/buildplanner/model"
+	"github.com/ActiveState/cli/pkg/platform/runtime/buildplan"
+	"github.com/ActiveState/cli/pkg/platform/runtime/setup"
+	"github.com/ActiveState/cli/pkg/platform/runtime/setup/buildlog"
+)
+
+func IsBuildError(err error) bool {
+	return errs.Matches(err, &setup.BuildError{}) ||
+		errs.Matches(err, &buildlog.BuildError{}) ||
+		errs.Matches(err, &model.BuildPlannerError{}) ||
+		errs.Matches(err, &buildplan.ArtifactError{})
+}

pkg/platform/runtime/buildplan/buildplan.go

@@ -19,6 +19,10 @@ type ArtifactListing struct {
 	artifactIDs      []artifact.ArtifactID
 }
 
+type ArtifactError struct {
+	*locale.LocalizedError
+}
+
 func NewArtifactListing(build *model.Build, buildtimeClosure bool) (*ArtifactListing, error) {
 	al := &ArtifactListing{build: build}
 	if buildtimeClosure {
@@ -190,7 +194,9 @@ func buildTerminals(nodeID strfmt.UUID, lookup map[strfmt.UUID]interface{}, resu
 	}
 
 	if !model.IsSuccessArtifactStatus(targetArtifact.Status) {
-		return locale.NewError("err_artifact_failed", "Artifact '{{.V0}}' failed to build", trimDisplayName(targetArtifact.DisplayName))
+		return &ArtifactError{
+			locale.NewError("err_artifact_failed", "Artifact '{{.V0}}' failed to build", trimDisplayName(targetArtifact.DisplayName)),
+		}
 	}
 
 	if model.IsStateToolArtifact(targetArtifact.MimeType) {