From 98ca57f54646451036a1798fcb8de34850b1c103 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 21 Oct 2024 14:35:00 -0700 Subject: [PATCH 1/8] Only set at_time if it is not already set --- pkg/platform/model/buildplanner/build.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/platform/model/buildplanner/build.go b/pkg/platform/model/buildplanner/build.go index 0b11fc1712..136b382b39 100644 --- a/pkg/platform/model/buildplanner/build.go +++ b/pkg/platform/model/buildplanner/build.go @@ -110,7 +110,9 @@ 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)) + if script.AtTime() == nil { + script.SetAtTime(time.Time(commit.AtTime)) + } return &Commit{commit, bp, script}, nil } From 11fb1d5c661f4f94f902f7d17a2a3561592e3d75 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 21 Oct 2024 15:18:53 -0700 Subject: [PATCH 2/8] Set at time consistently --- pkg/platform/model/buildplanner/build.go | 4 +--- pkg/platform/model/buildplanner/buildscript.go | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/platform/model/buildplanner/build.go b/pkg/platform/model/buildplanner/build.go index 136b382b39..0b11fc1712 100644 --- a/pkg/platform/model/buildplanner/build.go +++ b/pkg/platform/model/buildplanner/build.go @@ -110,9 +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") } - if script.AtTime() == nil { - script.SetAtTime(time.Time(commit.AtTime)) - } + script.SetAtTime(time.Time(commit.AtTime)) return &Commit{commit, bp, script}, nil } diff --git a/pkg/platform/model/buildplanner/buildscript.go b/pkg/platform/model/buildplanner/buildscript.go index 28bd980ffc..52f038ee5f 100644 --- a/pkg/platform/model/buildplanner/buildscript.go +++ b/pkg/platform/model/buildplanner/buildscript.go @@ -52,10 +52,10 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript } script := buildscript.New() - script.SetAtTime(time.Time(resp.Commit.AtTime)) if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil { return nil, errs.Wrap(err, "failed to parse build expression") } + script.SetAtTime(time.Time(resp.Commit.AtTime)) return script, nil } From bf89295f9ae59b63c5c50c379bcae33c3f490f65 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 22 Oct 2024 10:12:14 -0700 Subject: [PATCH 3/8] Don't set atTime when unmarshalling --- pkg/buildscript/unmarshal_buildexpression.go | 7 ------- pkg/platform/model/buildplanner/buildscript.go | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index f3459240f6..ecae65a5d0 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -4,9 +4,7 @@ import ( "encoding/json" "sort" "strings" - "time" - "github.com/go-openapi/strfmt" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -70,13 +68,8 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error { // reference to "TIME", which is how we want to show it in AScript format. if atTimeNode, err := b.getSolveAtTimeValue(); err == nil { if atTimeNode.Str != nil && !strings.HasPrefix(*atTimeNode.Str, `$`) { - atTime, err := strfmt.ParseDateTime(*atTimeNode.Str) - if err != nil { - return errs.Wrap(err, "Invalid timestamp: %s", *atTimeNode.Str) - } atTimeNode.Str = nil atTimeNode.Ident = ptr.To("TIME") - b.atTime = ptr.To(time.Time(atTime)) } else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" { atTimeNode.Ident = ptr.To("TIME") } diff --git a/pkg/platform/model/buildplanner/buildscript.go b/pkg/platform/model/buildplanner/buildscript.go index 52f038ee5f..28bd980ffc 100644 --- a/pkg/platform/model/buildplanner/buildscript.go +++ b/pkg/platform/model/buildplanner/buildscript.go @@ -52,10 +52,10 @@ func (b *BuildPlanner) GetBuildScript(commitID string) (*buildscript.BuildScript } script := buildscript.New() + script.SetAtTime(time.Time(resp.Commit.AtTime)) if err := script.UnmarshalBuildExpression(resp.Commit.Expression); err != nil { return nil, errs.Wrap(err, "failed to parse build expression") } - script.SetAtTime(time.Time(resp.Commit.AtTime)) return script, nil } From af34e6a05ec52889e69d3c2568845eee2eb4366e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 23 Oct 2024 16:00:11 -0700 Subject: [PATCH 4/8] Preserve solve at_time --- pkg/buildscript/buildscript.go | 14 +++++++++++++- pkg/buildscript/unmarshal.go | 2 +- pkg/buildscript/unmarshal_buildexpression.go | 8 ++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 739a097c75..1761cef4f5 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -7,6 +7,7 @@ import ( "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/logging" "github.com/brunoga/deep" ) @@ -19,6 +20,11 @@ type BuildScript struct { project string atTime *time.Time + + // solveAtTime is the original at_time found in the solve node. + // This is used to support the legacy use case where the at_time + // is found in the solve node. + solveAtTime *time.Time } func init() { @@ -53,7 +59,12 @@ func (b *BuildScript) SetProject(url string) { } func (b *BuildScript) AtTime() *time.Time { - return b.atTime + // If the atTime is set, prefer that over the + // legacy solveAtTime. + if b.atTime != nil { + return b.atTime + } + return b.solveAtTime } func (b *BuildScript) SetAtTime(t time.Time) { @@ -82,6 +93,7 @@ func (b *BuildScript) Equals(other *BuildScript) (bool, error) { if err != nil { return false, errs.Wrap(err, "Unable to marshal other buildscript") } + logging.Debug("BuildScript.Equals, myBytes: %s, otherBytes: %s", string(myBytes), string(otherBytes)) return string(myBytes) == string(otherBytes), nil } diff --git a/pkg/buildscript/unmarshal.go b/pkg/buildscript/unmarshal.go index 9602aef76c..4719b63460 100644 --- a/pkg/buildscript/unmarshal.go +++ b/pkg/buildscript/unmarshal.go @@ -79,5 +79,5 @@ func Unmarshal(data []byte) (*BuildScript, error) { atTime = ptr.To(time.Time(atTimeStr)) } - return &BuildScript{raw, project, atTime}, nil + return &BuildScript{raw, project, atTime, nil}, nil } diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index ecae65a5d0..959d6303b8 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -4,6 +4,7 @@ import ( "encoding/json" "sort" "strings" + "time" "golang.org/x/text/cases" "golang.org/x/text/language" @@ -12,6 +13,7 @@ import ( "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. @@ -68,8 +70,14 @@ func (b *BuildScript) UnmarshalBuildExpression(data []byte) error { // reference to "TIME", which is how we want to show it in AScript format. if atTimeNode, err := b.getSolveAtTimeValue(); err == nil { if atTimeNode.Str != nil && !strings.HasPrefix(*atTimeNode.Str, `$`) { + atTime, err := strfmt.ParseDateTime(*atTimeNode.Str) + if err != nil { + return errs.Wrap(err, "Invalid timestamp: %s", *atTimeNode.Str) + } atTimeNode.Str = nil atTimeNode.Ident = ptr.To("TIME") + // Preserve the original at_time found in the solve node. + b.solveAtTime = ptr.To(time.Time(atTime)) } else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" { atTimeNode.Ident = ptr.To("TIME") } From 9eab57aecb56c035a97fde9068f97709883b8a47 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 24 Oct 2024 11:15:02 -0700 Subject: [PATCH 5/8] Add unit test --- pkg/buildscript/buildscript.go | 2 +- pkg/buildscript/buildscript_test.go | 56 +++++++++++++++++++ .../buildexpression-roundtrip-legacy.json | 43 ++++++++++++++ 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 pkg/buildscript/testdata/buildexpression-roundtrip-legacy.json diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 1761cef4f5..01c719780c 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -23,7 +23,7 @@ type BuildScript struct { // solveAtTime is the original at_time found in the solve node. // This is used to support the legacy use case where the at_time - // is found in the solve node. + // is an actual time stamp and not a reference to the $at_time variable. solveAtTime *time.Time } diff --git a/pkg/buildscript/buildscript_test.go b/pkg/buildscript/buildscript_test.go index 4a96fb9766..7d2ccbe0fe 100644 --- a/pkg/buildscript/buildscript_test.go +++ b/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,58 @@ 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 + updatedTime, err := time.Parse(strfmt.RFC3339Millis, updatedTimeStamp) + require.NoError(t, err) + script.SetAtTime(updatedTime) + + // The updated time should be reflected in the build script + require.Equal(t, updatedTime, *script.AtTime()) + + 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) { diff --git a/pkg/buildscript/testdata/buildexpression-roundtrip-legacy.json b/pkg/buildscript/testdata/buildexpression-roundtrip-legacy.json new file mode 100644 index 0000000000..08212c58af --- /dev/null +++ b/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" + } +} \ No newline at end of file From a67002f979aedc9490912b009ccdbd6662c166e9 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 24 Oct 2024 11:15:43 -0700 Subject: [PATCH 6/8] Remove debug log --- pkg/buildscript/buildscript.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 01c719780c..30e9b3b264 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -7,7 +7,6 @@ import ( "github.com/ActiveState/cli/internal/condition" "github.com/ActiveState/cli/internal/errs" - "github.com/ActiveState/cli/internal/logging" "github.com/brunoga/deep" ) @@ -93,7 +92,6 @@ func (b *BuildScript) Equals(other *BuildScript) (bool, error) { if err != nil { return false, errs.Wrap(err, "Unable to marshal other buildscript") } - logging.Debug("BuildScript.Equals, myBytes: %s, otherBytes: %s", string(myBytes), string(otherBytes)) return string(myBytes) == string(otherBytes), nil } From 33bf8fb3229ee0377e1bd44b290ae38f950577b8 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 25 Oct 2024 11:30:26 -0700 Subject: [PATCH 7/8] Use override when setting atTime --- internal/runners/install/install.go | 8 ++++--- internal/runners/platforms/add.go | 2 +- internal/runners/upgrade/upgrade.go | 2 +- pkg/buildscript/buildscript.go | 17 +++++--------- pkg/buildscript/buildscript_test.go | 22 ++++++++++++++----- pkg/buildscript/merge.go | 2 +- pkg/buildscript/unmarshal.go | 2 +- pkg/buildscript/unmarshal_buildexpression.go | 2 +- pkg/platform/model/buildplanner/build.go | 2 +- .../model/buildplanner/buildscript.go | 2 +- pkg/platform/model/buildplanner/commit.go | 2 +- scripts/to-buildscript/main.go | 2 +- 12 files changed, 36 insertions(+), 29 deletions(-) diff --git a/internal/runners/install/install.go b/internal/runners/install/install.go index f0b4ce3b86..e11ca22eed 100644 --- a/internal/runners/install/install.go +++ b/internal/runners/install/install.go @@ -114,6 +114,7 @@ func (i *Install) Run(params Params) (rerr error) { var oldCommit *bpModel.Commit var reqs requirements var ts time.Time + var override bool { pg = output.StartSpinner(out, locale.T("progress_search"), constants.TerminalAnimationInterval) @@ -133,6 +134,7 @@ func (i *Install) Run(params Params) (rerr error) { if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } + override = params.Timestamp.String() != "" // Get languages used in current project languages, err := model.FetchLanguagesForBuildScript(oldCommit.BuildScript()) @@ -153,7 +155,7 @@ func (i *Install) Run(params Params) (rerr error) { // Prepare updated buildscript script := oldCommit.BuildScript() - if err := prepareBuildScript(script, reqs, ts); err != nil { + if err := prepareBuildScript(script, reqs, ts, override); err != nil { return errs.Wrap(err, "Could not prepare build script") } @@ -340,8 +342,8 @@ func (i *Install) renderUserFacing(reqs requirements) { i.prime.Output().Notice("") } -func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time) error { - script.SetAtTime(ts) +func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time, override bool) error { + script.SetAtTime(ts, override) for _, req := range requirements { requirement := types.Requirement{ Namespace: req.Resolved.Namespace, diff --git a/internal/runners/platforms/add.go b/internal/runners/platforms/add.go index b4d8d5c253..5b4d7b4300 100644 --- a/internal/runners/platforms/add.go +++ b/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, false) script.AddPlatform(*platform.PlatformID) // Update local checkout and source runtime changes diff --git a/internal/runners/upgrade/upgrade.go b/internal/runners/upgrade/upgrade.go index 953b20098c..ec1320542d 100644 --- a/internal/runners/upgrade/upgrade.go +++ b/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, params.Timestamp.String() != "") // 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. diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 30e9b3b264..79ad6b7474 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -19,11 +19,6 @@ type BuildScript struct { project string atTime *time.Time - - // solveAtTime is the original at_time found in the solve node. - // This is used to support the legacy use case where the at_time - // is an actual time stamp and not a reference to the $at_time variable. - solveAtTime *time.Time } func init() { @@ -58,15 +53,13 @@ func (b *BuildScript) SetProject(url string) { } func (b *BuildScript) AtTime() *time.Time { - // If the atTime is set, prefer that over the - // legacy solveAtTime. - if b.atTime != nil { - return b.atTime - } - return b.solveAtTime + return b.atTime } -func (b *BuildScript) SetAtTime(t time.Time) { +func (b *BuildScript) SetAtTime(t time.Time, override bool) { + if b.atTime != nil && !override { + return + } b.atTime = &t } diff --git a/pkg/buildscript/buildscript_test.go b/pkg/buildscript/buildscript_test.go index 7d2ccbe0fe..4bc18a2d06 100644 --- a/pkg/buildscript/buildscript_test.go +++ b/pkg/buildscript/buildscript_test.go @@ -127,18 +127,30 @@ func TestRoundTripFromBuildExpressionWithLegacyAtTime(t *testing.T) { assert.Contains(t, string(data), "$at_time") assert.NotContains(t, string(data), initialTimeStamp) - // Update the time in the build script + // 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) + script.SetAtTime(updatedTime, false) // The updated time should be reflected in the build script - require.Equal(t, updatedTime, *script.AtTime()) + require.Equal(t, initialTimeStamp, script.AtTime().Format(strfmt.RFC3339Millis)) data, err = script.Marshal() require.NoError(t, err) - // The marshalled build script should now contain the updated time + // 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)) @@ -158,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() diff --git a/pkg/buildscript/merge.go b/pkg/buildscript/merge.go index 889cafffb1..32db4da729 100644 --- a/pkg/buildscript/merge.go +++ b/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 diff --git a/pkg/buildscript/unmarshal.go b/pkg/buildscript/unmarshal.go index 4719b63460..9602aef76c 100644 --- a/pkg/buildscript/unmarshal.go +++ b/pkg/buildscript/unmarshal.go @@ -79,5 +79,5 @@ func Unmarshal(data []byte) (*BuildScript, error) { atTime = ptr.To(time.Time(atTimeStr)) } - return &BuildScript{raw, project, atTime, nil}, nil + return &BuildScript{raw, project, atTime}, nil } diff --git a/pkg/buildscript/unmarshal_buildexpression.go b/pkg/buildscript/unmarshal_buildexpression.go index 959d6303b8..fe7eee2eeb 100644 --- a/pkg/buildscript/unmarshal_buildexpression.go +++ b/pkg/buildscript/unmarshal_buildexpression.go @@ -77,7 +77,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.solveAtTime = ptr.To(time.Time(atTime)) + b.atTime = ptr.To(time.Time(atTime)) } else if atTimeNode.Ident != nil && *atTimeNode.Ident == "at_time" { atTimeNode.Ident = ptr.To("TIME") } diff --git a/pkg/platform/model/buildplanner/build.go b/pkg/platform/model/buildplanner/build.go index 0b11fc1712..feef9b1a28 100644 --- a/pkg/platform/model/buildplanner/build.go +++ b/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 } diff --git a/pkg/platform/model/buildplanner/buildscript.go b/pkg/platform/model/buildplanner/buildscript.go index 28bd980ffc..c6fcc6a2ee 100644 --- a/pkg/platform/model/buildplanner/buildscript.go +++ b/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") } diff --git a/pkg/platform/model/buildplanner/commit.go b/pkg/platform/model/buildplanner/commit.go index 387296b1ff..3086d32930 100644 --- a/pkg/platform/model/buildplanner/commit.go +++ b/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") } diff --git a/scripts/to-buildscript/main.go b/scripts/to-buildscript/main.go index 60c7cde520..84a2e4599f 100644 --- a/scripts/to-buildscript/main.go +++ b/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 { From 34d3e1a78ccc83206d0b3146d2b7d36e71787884 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 28 Oct 2024 13:16:41 -0700 Subject: [PATCH 8/8] Address PR review comments --- internal/runners/install/install.go | 8 +++----- internal/runners/platforms/add.go | 2 +- internal/runners/upgrade/upgrade.go | 2 +- pkg/buildscript/buildscript.go | 3 +++ 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/runners/install/install.go b/internal/runners/install/install.go index e11ca22eed..5724c7b580 100644 --- a/internal/runners/install/install.go +++ b/internal/runners/install/install.go @@ -114,7 +114,6 @@ func (i *Install) Run(params Params) (rerr error) { var oldCommit *bpModel.Commit var reqs requirements var ts time.Time - var override bool { pg = output.StartSpinner(out, locale.T("progress_search"), constants.TerminalAnimationInterval) @@ -134,7 +133,6 @@ func (i *Install) Run(params Params) (rerr error) { if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } - override = params.Timestamp.String() != "" // Get languages used in current project languages, err := model.FetchLanguagesForBuildScript(oldCommit.BuildScript()) @@ -155,7 +153,7 @@ func (i *Install) Run(params Params) (rerr error) { // Prepare updated buildscript script := oldCommit.BuildScript() - if err := prepareBuildScript(script, reqs, ts, override); err != nil { + if err := prepareBuildScript(script, reqs, ts); err != nil { return errs.Wrap(err, "Could not prepare build script") } @@ -342,8 +340,8 @@ func (i *Install) renderUserFacing(reqs requirements) { i.prime.Output().Notice("") } -func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time, override bool) error { - script.SetAtTime(ts, override) +func prepareBuildScript(script *buildscript.BuildScript, requirements requirements, ts time.Time) error { + script.SetAtTime(ts, true) for _, req := range requirements { requirement := types.Requirement{ Namespace: req.Resolved.Namespace, diff --git a/internal/runners/platforms/add.go b/internal/runners/platforms/add.go index 5b4d7b4300..3c194ff96f 100644 --- a/internal/runners/platforms/add.go +++ b/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, false) + script.SetAtTime(ts, true) script.AddPlatform(*platform.PlatformID) // Update local checkout and source runtime changes diff --git a/internal/runners/upgrade/upgrade.go b/internal/runners/upgrade/upgrade.go index ec1320542d..eb82b499c2 100644 --- a/internal/runners/upgrade/upgrade.go +++ b/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, params.Timestamp.String() != "") + 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. diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 79ad6b7474..6b118071df 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -56,6 +56,9 @@ func (b *BuildScript) AtTime() *time.Time { return b.atTime } +// 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