Do not update to merge commit if there was a buildscript conflict.

This partially reverts ActiveState#3249 because that solution was based on the buggy behavior fixed in ActiveState#3282.

Author: mitchell-as

internal/runners/pull/pull.go

@@ -157,18 +157,18 @@ func (p *Pull) Run(params *PullParams) (rerr error) {
 	}
 
 	if commitID != *resultingCommit {
-		err := localcommit.Set(p.project.Dir(), resultingCommit.String())
-		if err != nil {
-			return errs.Wrap(err, "Unable to set local commit")
-		}
-
 		if p.cfg.GetBool(constants.OptinBuildscriptsConfig) {
 			err := p.mergeBuildScript(*remoteCommit, *localCommit)
 			if err != nil {
 				return errs.Wrap(err, "Could not merge local build script with remote changes")
 			}
 		}
 
+		err := localcommit.Set(p.project.Dir(), resultingCommit.String())
+		if err != nil {
+			return errs.Wrap(err, "Unable to set local commit")
+		}
+
 		p.out.Print(&pullOutput{
 			locale.Tr("pull_updated", remoteProject.String(), resultingCommit.String()),
 			true,

test/integration/pull_int_test.go

@@ -94,6 +94,9 @@ func (suite *PullIntegrationTestSuite) TestMergeBuildScript() {
 	cp.Expect("Package added", e2e.RuntimeSourcingTimeoutOpt)
 	cp.ExpectExitCode(0)
 
+	commit, err := localcommit.Get(ts.Dirs.Work)
+	suite.Require().NoError(err)
+
 	proj, err := project.FromPath(ts.Dirs.Work)
 	suite.NoError(err, "Error loading project")
 
@@ -114,12 +117,12 @@ func (suite *PullIntegrationTestSuite) TestMergeBuildScript() {
 	suite.Assert().Contains(string(bytes), "=======", "No merge conflict markers are in build script")
 	suite.Assert().Contains(string(bytes), ">>>>>>>", "No merge conflict markers are in build script")
 
-	// Verify the local commit was updated to the merge commit.
+	// Verify the local commit was not updated to the merge commit.
 	// Note: even though the buildscript merge failed, a merge commit was still created. After resolving
-	// buildscript conflicts, `state commit` should have something new to commit.
-	commit, err := localcommit.Get(ts.Dirs.Work)
+	// buildscript conflicts, `state commit` should always have something new to commit.
+	commit2, err := localcommit.Get(ts.Dirs.Work)
 	suite.Require().NoError(err)
-	suite.Assert().NotEqual(commit.String(), "447b8363-024c-4143-bf4e-c96989314fdf", "localcommit not updated to merged commit")
+	suite.Assert().Equal(commit.String(), commit2.String(), "localcommit should not have been updated to merged commit")
 }
 
 func (suite *PullIntegrationTestSuite) assertMergeStrategyNotification(ts *e2e.Session, strategy string) {