Merge pull request #3552 from ActiveState/DX-3115

Fix merge integration test

internal/runners/install/install.go

@@ -341,7 +341,7 @@ func (i *Install) renderUserFacing(reqs requirements) {
 }
 
 func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time) error {
-	script.SetAtTime(ts)
+	script.SetAtTime(ts, true)
 	for _, req := range requirements {
 		requirement := types.Requirement{
 			Namespace:          req.Resolved.Namespace,

internal/runners/platforms/add.go

@@ -98,7 +98,7 @@ func (a *Add) Run(params AddRunParams) (rerr error) {
 
 	// Prepare updated buildscript
 	script := oldCommit.BuildScript()
-	script.SetAtTime(ts)
+	script.SetAtTime(ts, true)
 	script.AddPlatform(*platform.PlatformID)
 
 	// Update local checkout and source runtime changes

internal/runners/upgrade/upgrade.go

@@ -111,7 +111,7 @@ func (u *Upgrade) Run(params *Params) (rerr error) {
 	if err != nil {
 		return errs.Wrap(err, "Failed to fetch latest timestamp")
 	}
-	bumpedBS.SetAtTime(ts)
+	bumpedBS.SetAtTime(ts, true)
 
 	// Since our platform is commit based we need to create a commit for the "after" buildplan, even though we may not
 	// end up using it it the user doesn't confirm the upgrade.

pkg/buildscript/buildscript.go

@@ -56,7 +56,13 @@ func (b *BuildScript) AtTime() *time.Time {
 	return b.atTime
 }
 
-func (b *BuildScript) SetAtTime(t time.Time) {
+// SetAtTime sets the AtTime value, if the buildscript already has an AtTime value
+// and `override=false` then the value passed here will be discarded.
+// Override should in most cases only be true if we are making changes to the buildscript.
+func (b *BuildScript) SetAtTime(t time.Time, override bool) {
+	if b.atTime != nil && !override {
+		return
+	}
 	b.atTime = &t
 }
 

pkg/buildscript/buildscript_test.go

@@ -1,9 +1,13 @@
 package buildscript
 
 import (
+	"fmt"
+	"path/filepath"
 	"testing"
 	"time"
 
+	"github.com/ActiveState/cli/internal/environment"
+	"github.com/ActiveState/cli/internal/fileutils"
 	"github.com/go-openapi/strfmt"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -94,6 +98,70 @@ func TestRoundTripFromBuildExpression(t *testing.T) {
 	require.Equal(t, string(basicBuildExpression), string(data))
 }
 
+func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) {
+	wd, err := environment.GetRootPath()
+	require.NoError(t, err)
+
+	initialTimeStamp := "2024-10-15T16:37:06.260Z"
+	updatedTimeStamp := "2024-10-15T16:37:06.261Z"
+
+	data, err := fileutils.ReadFile(filepath.Join(wd, "pkg", "buildscript", "testdata", "buildexpression-roundtrip-legacy.json"))
+	require.NoError(t, err)
+
+	// The initial build expression does not use the new at_time format
+	assert.NotContains(t, string(data), "$at_time")
+	assert.Contains(t, string(data), initialTimeStamp)
+
+	script := New()
+	require.NoError(t, script.UnmarshalBuildExpression(data))
+
+	// Ensure that legacy at_time is preserved in the buildscript.
+	atTime := script.AtTime()
+	require.NotNil(t, atTime)
+	require.Equal(t, initialTimeStamp, atTime.Format(strfmt.RFC3339Millis))
+
+	data, err = script.MarshalBuildExpression()
+	require.NoError(t, err)
+
+	// When the build expression is unmarshalled it should now use the new at_time format
+	assert.Contains(t, string(data), "$at_time")
+	assert.NotContains(t, string(data), initialTimeStamp)
+
+	// Update the time in the build script but don't override the existing time
+	updatedTime, err := time.Parse(strfmt.RFC3339Millis, updatedTimeStamp)
+	require.NoError(t, err)
+	script.SetAtTime(updatedTime, false)
+
+	// The updated time should be reflected in the build script
+	require.Equal(t, initialTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis))
+
+	data, err = script.Marshal()
+	require.NoError(t, err)
+
+	// The marshalled build script should NOT contain the updated time
+	// in the Time block at the top of the script.
+	assert.Contains(t, string(data), initialTimeStamp)
+	assert.NotContains(t, string(data), fmt.Sprintf("Time: %s", updatedTime))
+
+	// Now override the time in the build script
+	script.SetAtTime(updatedTime, true)
+	require.Equal(t, updatedTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis))
+
+	data, err = script.Marshal()
+	require.NoError(t, err)
+
+	// The marshalled build script should NOW contain the updated time
+	// in the Time block at the top of the script.
+	assert.Contains(t, string(data), updatedTimeStamp)
+	assert.NotContains(t, string(data), fmt.Sprintf("Time: %s", initialTimeStamp))
+
+	data, err = script.MarshalBuildExpression()
+	require.NoError(t, err)
+
+	// The build expression representation should now use the new at_time format
+	assert.Contains(t, string(data), "$at_time")
+}
+
 // TestExpressionToScript tests that creating a build script from a given Platform build expression
 // and at time produces the expected result.
 func TestExpressionToScript(t *testing.T) {
@@ -102,7 +170,7 @@ func TestExpressionToScript(t *testing.T) {
 
 	script := New()
 	script.SetProject(testProject)
-	script.SetAtTime(ts)
+	script.SetAtTime(ts, false)
 	require.NoError(t, script.UnmarshalBuildExpression(basicBuildExpression))
 
 	data, err := script.Marshal()

pkg/buildscript/merge.go

@@ -58,7 +58,7 @@ func (b *BuildScript) Merge(other *BuildScript, strategies *mono_models.MergeStr
 	// When merging build scripts we want to use the most recent timestamp
 	atTime := other.AtTime()
 	if atTime != nil && b.AtTime() != nil && atTime.After(*b.AtTime()) {
-		b.SetAtTime(*atTime)
+		b.SetAtTime(*atTime, true)
 	}
 
 	return nil

pkg/buildscript/testdata/buildexpression-roundtrip-legacy.json

@@ -0,0 +1,43 @@
+{
+  "let": {
+    "sources": {
+      "solve": {
+        "at_time": "2024-10-15T16:37:06.260Z",
+        "solver_version": null,
+        "platforms": [
+          "46a5b48f-226a-4696-9746-ba4d50d661c2",
+          "78977bc8-0f32-519d-80f3-9043f059398c",
+          "7c998ec2-7491-4e75-be4d-8885800ef5f2"
+        ],
+        "requirements": [
+          {
+            "name": "community_artifact",
+            "namespace": "internal"
+          },
+          {
+            "name": "python",
+            "namespace": "language",
+            "version_requirements": [
+              {
+                "comparator": "eq",
+                "version": "3.11.10"
+              }
+            ]
+          }
+        ]
+      }
+    },
+    "artifacts": {
+      "community_artifacts": {
+        "src": "$sources"
+      }
+    },
+    "runtime": {
+      "state_tool_artifacts": {
+        "src": "$artifacts",
+        "build_flags": []
+      }
+    },
+    "in": "$runtime"
+  }
+}

pkg/buildscript/unmarshal_buildexpression.go

@@ -6,14 +6,14 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-openapi/strfmt"
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
 
 	"github.com/ActiveState/cli/internal/errs"
 	"github.com/ActiveState/cli/internal/logging"
 	"github.com/ActiveState/cli/internal/rtutils/ptr"
 	"github.com/ActiveState/cli/internal/sliceutils"
+	"github.com/go-openapi/strfmt"
 )
 
 // At this time, there is no way to ask the Platform for an empty build expression.
@@ -76,6 +76,7 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error {
 			}
 			atTimeNode.Str = nil
 			atTimeNode.Ident = ptr.To("TIME")
+			// Preserve the original at_time found in the solve node.
 			b.atTime = ptr.To(time.Time(atTime))
 		} else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" {
 			atTimeNode.Ident = ptr.To("TIME")

pkg/platform/model/buildplanner/build.go

@@ -110,7 +110,7 @@ func (b *BuildPlanner) FetchCommit(commitID strfmt.UUID, owner, project string,
 	if err := script.UnmarshalBuildExpression(commit.Expression); err != nil {
 		return nil, errs.Wrap(err, "failed to parse build expression")
 	}
-	script.SetAtTime(time.Time(commit.AtTime))
+	script.SetAtTime(time.Time(commit.AtTime), false)
 
 	return &Commit{commit, bp, script}, nil
 }

pkg/platform/model/buildplanner/buildscript.go

@@ -52,7 +52,7 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript
 	}
 
 	script := buildscript.New()
-	script.SetAtTime(time.Time(resp.Commit.AtTime))
+	script.SetAtTime(time.Time(resp.Commit.AtTime), false)
 	if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil {
 		return nil, errs.Wrap(err, "failed to parse build expression")
 	}

pkg/platform/model/buildplanner/commit.go

@@ -82,7 +82,7 @@ func (b *BuildPlanner) StageCommit(params StageCommitParams) (*Commit, error) {
 	}
 
 	stagedScript := buildscript.New()
-	stagedScript.SetAtTime(time.Time(resp.Commit.AtTime))
+	stagedScript.SetAtTime(time.Time(resp.Commit.AtTime), false)
 	if err := stagedScript.UnmarshalBuildExpression(resp.Commit.Expression); err != nil {
 		return nil, errs.Wrap(err, "failed to parse build expression")
 	}

scripts/to-buildscript/main.go

@@ -55,7 +55,7 @@ func main() {
 	bs := buildscript.New()
 	bs.SetProject(project)
 	if atTime != nil {
-		bs.SetAtTime(*atTime)
+		bs.SetAtTime(*atTime, true)
 	}
 	err := bs.UnmarshalBuildExpression([]byte(input))
 	if err != nil {