From e5cb1e0a046d2a68d2b86fcfde21d558d3c0e444 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 3 Oct 2023 16:43:12 -0400 Subject: [PATCH] Revert "Merge pull request #2774 from ActiveState/mitchell/dx-1961" This reverts commit 7b0308d215e4dd74fdbaea58d883bb8949313299, reversing changes made to 30a336cb4185db84569a7fbab6dadee5c4ec4719. --- internal/runbits/buildscript/buildscript.go | 7 +- .../runbits/buildscript/buildscript_test.go | 11 +- internal/runners/pull/pull.go | 5 +- .../buildexpression/buildexpression.go | 4 - .../runtime/buildscript/buildexpression.go | 228 ++++++++++++++++++ .../runtime/buildscript/buildscript.go | 89 ++----- .../runtime/buildscript/buildscript_test.go | 52 ++-- pkg/platform/runtime/buildscript/json.go | 16 +- test/integration/pull_int_test.go | 1 - 9 files changed, 286 insertions(+), 127 deletions(-) create mode 100644 pkg/platform/runtime/buildscript/buildexpression.go diff --git a/internal/runbits/buildscript/buildscript.go b/internal/runbits/buildscript/buildscript.go index e5d8114cc1..9414554e17 100644 --- a/internal/runbits/buildscript/buildscript.go +++ b/internal/runbits/buildscript/buildscript.go @@ -57,6 +57,11 @@ func Sync(proj *project.Project, commitID *strfmt.UUID, out output.Outputer, aut return false, nil // nothing to do } logging.Debug("Merging changes") + expr, err = script.ToBuildExpression() + if err != nil { + return false, errs.Wrap(err, "Unable to translate local build script to build expression") + } + out.Notice(locale.Tl("buildscript_update", "Updating project to reflect build script changes...")) localCommitID, err := localcommit.Get(proj.Dir()) @@ -69,7 +74,7 @@ func Sync(proj *project.Project, commitID *strfmt.UUID, out output.Outputer, aut Owner: proj.Owner(), Project: proj.Name(), ParentCommit: localCommitID.String(), - Expression: script.Expr, + Expression: expr, }) if err != nil { return false, errs.Wrap(err, "Could not update project to reflect build script changes.") diff --git a/internal/runbits/buildscript/buildscript_test.go b/internal/runbits/buildscript/buildscript_test.go index 83daaf4b5e..9ec51bbbf7 100644 --- a/internal/runbits/buildscript/buildscript_test.go +++ b/internal/runbits/buildscript/buildscript_test.go @@ -1,11 +1,9 @@ package buildscript import ( - "encoding/json" "testing" "github.com/ActiveState/cli/internal/rtutils/ptr" - "github.com/ActiveState/cli/pkg/platform/runtime/buildexpression" "github.com/ActiveState/cli/pkg/platform/runtime/buildscript" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -31,16 +29,11 @@ in: runtime`)) require.NoError(t, err) - // Make a copy of the original expression. - bytes, err := json.Marshal(script.Expr) - require.NoError(t, err) - expr, err := buildexpression.New(bytes) + expr, err := script.ToBuildExpression() require.NoError(t, err) - // Modify the build script. - (*script.Expr.Let.Assignments[0].Value.Ap.Arguments[0].Assignment.Value.List)[0].Str = ptr.To(`77777`) + (*script.Let.Assignments[0].Value.FuncCall.Arguments[0].Assignment.Value.List)[0].Str = ptr.To(`"77777"`) - // Generate the difference between the modified script and the original expression. result, err := generateDiff(script, expr) require.NoError(t, err) assert.Equal(t, `let: diff --git a/internal/runners/pull/pull.go b/internal/runners/pull/pull.go index e20eae3560..9be11843f9 100644 --- a/internal/runners/pull/pull.go +++ b/internal/runners/pull/pull.go @@ -227,7 +227,10 @@ func (p *Pull) mergeBuildScript(strategies *mono_models.MergeStrategies, remoteC } // Get the local and remote build expressions to merge. - exprA := script.Expr + exprA, err := script.ToBuildExpression() + if err != nil { + return errs.Wrap(err, "Unable to transform local buildscript into buildexpression") + } bp := model.NewBuildPlannerModel(p.auth) exprB, err := bp.GetBuildExpression(p.project.Owner(), p.project.Name(), remoteCommit.String()) if err != nil { diff --git a/pkg/platform/runtime/buildexpression/buildexpression.go b/pkg/platform/runtime/buildexpression/buildexpression.go index 187ac9a5ec..d759e99819 100644 --- a/pkg/platform/runtime/buildexpression/buildexpression.go +++ b/pkg/platform/runtime/buildexpression/buildexpression.go @@ -302,10 +302,6 @@ func newValue(path []string, valueInterface interface{}) (*Value, error) { case float64: value.Float = ptr.To(v) - case nil: - // An empty value is interpreted as JSON null. - value.Null = &Null{} - default: logging.Debug("Unknown type: %T at path %s", v, strings.Join(path, ".")) // An empty value is interpreted as JSON null. diff --git a/pkg/platform/runtime/buildscript/buildexpression.go b/pkg/platform/runtime/buildscript/buildexpression.go new file mode 100644 index 0000000000..4a4864b293 --- /dev/null +++ b/pkg/platform/runtime/buildscript/buildexpression.go @@ -0,0 +1,228 @@ +package buildscript + +import ( + "encoding/json" + "sort" + "strings" + + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/multilog" + "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/pkg/platform/runtime/buildexpression" +) + +const SolveFunction = "solve" +const SolveLegacyFunction = "solve_legacy" +const MergeFunction = "merge" + +func NewScriptFromBuildExpression(expr *buildexpression.BuildExpression) (*Script, error) { + data, err := json.Marshal(expr) + if err != nil { + return nil, errs.Wrap(err, "Unable to marshal buildexpression to JSON") + } + + m := make(map[string]interface{}) + err = json.Unmarshal(data, &m) + if err != nil { // this really should not happen + return nil, errs.Wrap(err, "Could not unmarshal buildexpression") + } + + letValue, ok := m["let"] + if !ok { + return nil, errs.New("Build expression has no 'let' key") + } + letMap, ok := letValue.(map[string]interface{}) + if !ok { + return nil, errs.New("'let' key is not a JSON object") + } + inValue, ok := letMap["in"] + if !ok { + return nil, errs.New("Build expression's 'let' object has no 'in' key") + } + delete(letMap, "in") // prevent duplication of "in" field when writing the build script + + let, err := newLet(letMap) + if err != nil { + return nil, errs.Wrap(err, "Could not parse 'let' key") + } + + in, err := newIn(inValue) + if err != nil { + return nil, errs.Wrap(err, "Could not parse 'in' key's value: %v", inValue) + } + + return &Script{let, in}, nil +} + +func newLet(m map[string]interface{}) (*Let, error) { + assignments, err := newAssignments(m) + if err != nil { + return nil, errs.Wrap(err, "Could not parse 'let' key") + } + return &Let{Assignments: *assignments}, nil +} + +func isFunction(name string) bool { + return name == SolveFunction || name == SolveLegacyFunction || name == MergeFunction +} + +func newValue(valueInterface interface{}, preferIdent bool) (*Value, error) { + value := &Value{} + + switch v := valueInterface.(type) { + case map[string]interface{}: + // Examine keys first to see if this is a function call. + for key := range v { + if isFunction(key) { + f, err := newFuncCall(v) + if err != nil { + return nil, errs.Wrap(err, "Could not parse '%s' function's value: %v", key, v) + } + value.FuncCall = f + } + } + + if value.FuncCall == nil { + // It's not a function call, but an object. + object, err := newAssignments(v) + if err != nil { + return nil, errs.Wrap(err, "Could not parse object: %v", v) + } + value.Object = object + } + + case []interface{}: + values := []*Value{} + for _, item := range v { + value, err := newValue(item, false) + if err != nil { + return nil, errs.Wrap(err, "Could not parse list: %v", v) + } + values = append(values, value) + } + value.List = &values + + case string: + if preferIdent { + value.Ident = &v + } else { + b, err := json.Marshal(v) + if err != nil { + return nil, errs.Wrap(err, "Could not marshal string '%s'", v) + } + value.Str = ptr.To(string(b)) + } + + case float64: + value.Number = ptr.To(v) + + default: + // An empty value is interpreted as JSON null. + value.Null = &Null{} + } + + return value, nil +} + +func newFuncCall(m map[string]interface{}) (*FuncCall, error) { + // Look in the given object for the function's name and argument object or list. + var name string + var argsInterface interface{} + for key, value := range m { + if isFunction(key) { + name = key + argsInterface = value + break + } + } + + args := []*Value{} + + switch v := argsInterface.(type) { + case map[string]interface{}: + for key, valueInterface := range v { + value, err := newValue(valueInterface, name == MergeFunction) + if err != nil { + return nil, errs.Wrap(err, "Could not parse '%s' function's argument '%s': %v", name, key, valueInterface) + } + args = append(args, &Value{Assignment: &Assignment{Key: key, Value: value}}) + } + sort.SliceStable(args, func(i, j int) bool { return args[i].Assignment.Key < args[j].Assignment.Key }) + + case []interface{}: + for _, item := range v { + value, err := newValue(item, false) + if err != nil { + return nil, errs.Wrap(err, "Could not parse '%s' function's argument list item: %v", name, item) + } + args = append(args, value) + } + + default: + return nil, errs.New("Function '%s' expected to be object or list", name) + } + + return &FuncCall{Name: name, Arguments: args}, nil +} + +func newAssignments(m map[string]interface{}) (*[]*Assignment, error) { + assignments := []*Assignment{} + for key, valueInterface := range m { + value, err := newValue(valueInterface, false) + if err != nil { + return nil, errs.Wrap(err, "Could not parse '%s' key's value: %v", key, valueInterface) + } + assignments = append(assignments, &Assignment{Key: key, Value: value}) + } + sort.SliceStable(assignments, func(i, j int) bool { return assignments[i].Key < assignments[j].Key }) + return &assignments, nil +} + +func newIn(inValue interface{}) (*In, error) { + in := &In{} + + switch v := inValue.(type) { + case map[string]interface{}: + f, err := newFuncCall(v) + if err != nil { + return nil, errs.Wrap(err, "'in' object is not a function call") + } + in.FuncCall = f + + case string: + in.Name = ptr.To(strings.TrimPrefix(v, "$")) + + default: + return nil, errs.New("'in' value expected to be a function call or string") + } + + return in, nil +} + +func (s *Script) EqualsBuildExpressionBytes(exprBytes []byte) bool { + expr, err := buildexpression.New(exprBytes) + if err != nil { + multilog.Error("Unable to create buildexpression from incoming JSON: %v", err) + return false + } + return s.EqualsBuildExpression(expr) +} + +func (s *Script) EqualsBuildExpression(expr *buildexpression.BuildExpression) bool { + myJson, err := json.Marshal(s) + if err != nil { + multilog.Error("Unable to marshal this buildscript to JSON: %v", err) + return false + } + otherScript, err := NewScriptFromBuildExpression(expr) + if err != nil { + multilog.Error("Unable to transform buildexpression to buildscript: %v", err) + return false + } + otherJson, err := json.Marshal(otherScript) + if err != nil { + multilog.Error("Unable to marshal other buildscript to JSON: %v", err) + return false + } + return string(myJson) == string(otherJson) +} diff --git a/pkg/platform/runtime/buildscript/buildscript.go b/pkg/platform/runtime/buildscript/buildscript.go index 42207ce514..e26355f112 100644 --- a/pkg/platform/runtime/buildscript/buildscript.go +++ b/pkg/platform/runtime/buildscript/buildscript.go @@ -2,27 +2,18 @@ package buildscript import ( "bytes" - "encoding/json" "fmt" "strconv" "strings" "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/errs" - "github.com/ActiveState/cli/internal/multilog" - "github.com/ActiveState/cli/pkg/platform/runtime/buildexpression" "github.com/alecthomas/participle/v2" ) -// Script's tagged fields will be initially filled in by Participle. -// Expr will be constructed later and is this script's buildexpression. We keep a copy of the build -// expression here with any changes that have been applied before either writing it to disk or -// submitting it to the build planner. It's easier to operate on build expressions directly than to -// modify or manually populate the Participle-produced fields and re-generate a build expression. type Script struct { - Let *Let `parser:"'let' ':' @@"` - In *In `parser:"'in' ':' @@"` - Expr *buildexpression.BuildExpression + Let *Let `parser:"'let' ':' @@"` + In *In `parser:"'in' ':' @@"` } type Let struct { @@ -71,24 +62,9 @@ func NewScript(data []byte) (*Script, error) { return nil, errs.Wrap(err, "Could not parse build script") } - // Construct the equivalent buildexpression. - bytes, err := json.Marshal(script) - if err != nil { - return nil, errs.Wrap(err, "Could not marshal build script to build expression") - } - expr, err := buildexpression.New(bytes) - if err != nil { - return nil, errs.Wrap(err, "Could not construct build expression") - } - script.Expr = expr - return script, nil } -func NewScriptFromBuildExpression(expr *buildexpression.BuildExpression) (*Script, error) { - return &Script{Expr: expr}, nil -} - func indent(s string) string { return fmt.Sprintf("\t%s", strings.ReplaceAll(s, "\n", "\n\t")) } @@ -96,34 +72,34 @@ func indent(s string) string { func (s *Script) String() string { buf := strings.Builder{} buf.WriteString("let:\n") - for _, assignment := range s.Expr.Let.Assignments { - buf.WriteString(indent(assignmentString(assignment))) + for _, assignment := range s.Let.Assignments { + buf.WriteString(indent(assignment.String())) } buf.WriteString("\n\n") buf.WriteString("in:\n") switch { - case s.Expr.Let.In.FuncCall != nil: - buf.WriteString(indent(apString(s.Expr.Let.In.FuncCall))) - case s.Expr.Let.In.Name != nil: - buf.WriteString(indent(*s.Expr.Let.In.Name)) + case s.In.FuncCall != nil: + buf.WriteString(indent(s.In.FuncCall.String())) + case s.In.Name != nil: + buf.WriteString(indent(*s.In.Name)) } return buf.String() } -func assignmentString(a *buildexpression.Var) string { - return fmt.Sprintf("%s = %s", a.Name, valueString(a.Value)) +func (a *Assignment) String() string { + return fmt.Sprintf("%s = %s", a.Key, a.Value.String()) } -func valueString(v *buildexpression.Value) string { +func (v *Value) String() string { switch { - case v.Ap != nil: - return apString(v.Ap) + case v.FuncCall != nil: + return v.FuncCall.String() case v.List != nil: buf := bytes.Buffer{} buf.WriteString("[\n") for i, item := range *v.List { - buf.WriteString(indent(valueString(item))) + buf.WriteString(indent(item.String())) if i+1 < len(*v.List) { buf.WriteString(",") } @@ -133,22 +109,22 @@ func valueString(v *buildexpression.Value) string { return buf.String() case v.Str != nil: - return strconv.Quote(*v.Str) + return *v.Str - case v.Float != nil: - return strconv.FormatFloat(*v.Float, 'G', -1, 64) // 64-bit float with minimum digits on display + case v.Number != nil: + return strconv.FormatFloat(*v.Number, 'G', -1, 64) // 64-bit float with minimum digits on display case v.Null != nil: return "null" case v.Assignment != nil: - return assignmentString(v.Assignment) + return v.Assignment.String() case v.Object != nil: buf := bytes.Buffer{} buf.WriteString("{\n") for i, pair := range *v.Object { - buf.WriteString(indent(assignmentString(pair))) + buf.WriteString(indent(pair.String())) if i+1 < len(*v.Object) { buf.WriteString(",") } @@ -164,11 +140,11 @@ func valueString(v *buildexpression.Value) string { return fmt.Sprintf("[\n]") // participle does not create v.List if it's empty } -func apString(f *buildexpression.Ap) string { +func (f *FuncCall) String() string { buf := bytes.Buffer{} buf.WriteString(fmt.Sprintf("%s(\n", f.Name)) for i, argument := range f.Arguments { - buf.WriteString(indent(valueString(argument))) + buf.WriteString(indent(argument.String())) if i+1 < len(f.Arguments) { buf.WriteString(",") } @@ -177,26 +153,3 @@ func apString(f *buildexpression.Ap) string { buf.WriteString(")") return buf.String() } - -func (s *Script) EqualsBuildExpressionBytes(exprBytes []byte) bool { - expr, err := buildexpression.New(exprBytes) - if err != nil { - multilog.Error("Unable to create buildexpression from incoming JSON: %v", err) - return false - } - return s.EqualsBuildExpression(expr) -} - -func (s *Script) EqualsBuildExpression(expr *buildexpression.BuildExpression) bool { - myJson, err := json.Marshal(s.Expr) - if err != nil { - multilog.Error("Unable to marshal this buildscript to JSON: %v", err) - return false - } - otherJson, err := json.Marshal(expr) - if err != nil { - multilog.Error("Unable to marshal other buildscript to JSON: %v", err) - return false - } - return string(myJson) == string(otherJson) -} diff --git a/pkg/platform/runtime/buildscript/buildscript_test.go b/pkg/platform/runtime/buildscript/buildscript_test.go index d05c3f6f1d..26920ef376 100644 --- a/pkg/platform/runtime/buildscript/buildscript_test.go +++ b/pkg/platform/runtime/buildscript/buildscript_test.go @@ -12,18 +12,6 @@ import ( "github.com/stretchr/testify/require" ) -// toBuildExpression converts given script constructed by Participle into a buildexpression. -// This function should not be used to convert an arbitrary script to buildexpression. -// NewScript*() populates the expr field with the equivalent build expression. -// This function exists solely for testing that functionality. -func toBuildExpression(script *Script) (*buildexpression.BuildExpression, error) { - bytes, err := json.Marshal(script) - if err != nil { - return nil, err - } - return buildexpression.New(bytes) -} - func TestBasic(t *testing.T) { script, err := NewScript([]byte( `let: @@ -51,9 +39,6 @@ in: `)) require.NoError(t, err) - expr, err := toBuildExpression(script) - require.NoError(t, err) - assert.Equal(t, &Script{ &Let{ []*Assignment{ @@ -88,12 +73,10 @@ in: }, }, &In{Name: ptr.To("runtime")}, - expr, }, script) } -func NoTestComplex(t *testing.T) { - t.Skip("Multiple solve notes are not supported now") // DX-2238 +func TestComplex(t *testing.T) { script, err := NewScript([]byte( `let: linux_runtime = solve( @@ -114,16 +97,13 @@ func NoTestComplex(t *testing.T) { ) in: - merge( + merge( win_installer(win_runtime), tar_installer(linux_runtime) ) `)) require.NoError(t, err) - expr, err := toBuildExpression(script) - require.NoError(t, err) - assert.Equal(t, &Script{ &Let{ []*Assignment{ @@ -165,7 +145,6 @@ in: {FuncCall: &FuncCall{"win_installer", []*Value{{Ident: ptr.To("win_runtime")}}}}, {FuncCall: &FuncCall{"tar_installer", []*Value{{Ident: ptr.To("linux_runtime")}}}}, }}}, - expr, }, script) } @@ -198,9 +177,6 @@ func TestExample(t *testing.T) { script, err := NewScript([]byte(example)) require.NoError(t, err) - expr, err := toBuildExpression(script) - require.NoError(t, err) - assert.Equal(t, &Script{ &Let{ []*Assignment{ @@ -241,7 +217,6 @@ func TestExample(t *testing.T) { }, }, &In{Name: ptr.To("runtime")}, - expr, }, script) } @@ -249,8 +224,8 @@ func TestString(t *testing.T) { script, err := NewScript([]byte( `let: runtime = solve( - platforms=["12345", "67890"], - requirements=[{name="language/python"}] + requirements=[{name="language/python"}], + platforms=["12345", "67890"] ) in: runtime @@ -260,14 +235,14 @@ in: assert.Equal(t, `let: runtime = solve( - platforms = [ - "12345", - "67890" - ], requirements = [ { name = "language/python" } + ], + platforms = [ + "12345", + "67890" ] ) @@ -387,7 +362,8 @@ func TestBuildExpression(t *testing.T) { script, err := NewScriptFromBuildExpression(expr) require.NoError(t, err) require.NotNil(t, script) - //newExpr := script.Expr + //newExpr, err := script.ToBuildExpression() + //require.NoError(t, err) exprBytes, err := json.Marshal(expr) require.NoError(t, err) //newExprBytes, err := json.Marshal(newExpr) @@ -403,12 +379,12 @@ func TestBuildExpression(t *testing.T) { // Verify null JSON value is handled correctly. var null *string nullHandled := false - for _, assignment := range script.Expr.Let.Assignments { - if assignment.Name == "runtime" { - args := assignment.Value.Ap.Arguments + for _, assignment := range script.Let.Assignments { + if assignment.Key == "runtime" { + args := assignment.Value.FuncCall.Arguments require.NotNil(t, args) for _, arg := range args { - if arg.Assignment != nil && arg.Assignment.Name == "solver_version" { + if arg.Assignment != nil && arg.Assignment.Key == "solver_version" { assert.Equal(t, null, arg.Assignment.Value.Str) nullHandled = true } diff --git a/pkg/platform/runtime/buildscript/json.go b/pkg/platform/runtime/buildscript/json.go index da159a1d24..0f22a8a451 100644 --- a/pkg/platform/runtime/buildscript/json.go +++ b/pkg/platform/runtime/buildscript/json.go @@ -6,11 +6,19 @@ import ( "fmt" "sort" "strings" + + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/pkg/platform/runtime/buildexpression" ) -// MarshalJSON marshals the Participle-produced Script into an equivalent buildexpression. -// Users of buildscripts do not need to do this manually; the Expr field contains the -// equivalent buildexpression. +func (s *Script) ToBuildExpression() (*buildexpression.BuildExpression, error) { + data, err := json.Marshal(s) + if err != nil { + return nil, errs.Wrap(err, "Unable to marshal buildscript into JSON") + } + return buildexpression.New(data) +} + func (s *Script) MarshalJSON() ([]byte, error) { m := make(map[string]interface{}) let := make(map[string]interface{}) @@ -71,8 +79,6 @@ func (f *FuncCall) MarshalJSON() ([]byte, error) { switch { case argument.Assignment != nil: args[argument.Assignment.Key] = argument.Assignment.Value - case argument.FuncCall != nil: - args[argument.FuncCall.Name] = argument.FuncCall.Arguments default: return nil, errors.New(fmt.Sprintf("Cannot marshal %v (arg %v)", f, argument)) } diff --git a/test/integration/pull_int_test.go b/test/integration/pull_int_test.go index 4f8cc3ff0b..b4ff02e38d 100644 --- a/test/integration/pull_int_test.go +++ b/test/integration/pull_int_test.go @@ -133,7 +133,6 @@ func (suite *PullIntegrationTestSuite) TestPull_RestoreNamespace() { } func (suite *PullIntegrationTestSuite) TestMergeBuildScript() { - suite.T().Skip("Skipping since build script/build expression equality cannot be easily computed") // DX-2221 suite.OnlyRunForTags(tagsuite.Pull) ts := e2e.New(suite.T(), false) defer ts.Close()