diff --git a/engine/resources/pkg.go b/engine/resources/pkg.go index ad1b1745..b2a4cf50 100644 --- a/engine/resources/pkg.go +++ b/engine/resources/pkg.go @@ -366,6 +366,29 @@ func (obj *PkgRes) Cmp(r engine.Res) error { if obj.State != res.State { return fmt.Errorf("state differs: %s vs %s", obj.State, res.State) } + + return obj.Adapts(res) +} + +// Adapts compares two resources and returns an error if they are not able to be +// equivalently output compatible. +func (obj *PkgRes) Adapts(r engine.CompatibleRes) error { + res, ok := r.(*PkgRes) + if !ok { + return fmt.Errorf("res is not the same kind") + } + + if obj.State != res.State { + e := fmt.Errorf("state differs in an incompatible way: %s vs %s", obj.State, res.State) + if obj.State == PkgStateUninstalled || res.State == PkgStateUninstalled { + return e + } + if stateIsVersion(obj.State) || stateIsVersion(res.State) { + return e + } + // one must be installed, and the other must be "newest" + } + if obj.AllowUntrusted != res.AllowUntrusted { return fmt.Errorf("allowuntrusted differs: %t vs %t", obj.AllowUntrusted, res.AllowUntrusted) } @@ -379,6 +402,50 @@ func (obj *PkgRes) Cmp(r engine.Res) error { return nil } +// Merge returns the best equivalent of the two resources. They must satisfy the +// Adapts test for this to work. +func (obj *PkgRes) Merge(r engine.CompatibleRes) (engine.CompatibleRes, error) { + res, ok := r.(*PkgRes) + if !ok { + return nil, fmt.Errorf("res is not the same kind") + } + + if err := obj.Adapts(r); err != nil { + return nil, errwrap.Wrapf(err, "can't merge resources that aren't compatible") + } + + // modify the copy, not the original + x, err := engine.ResCopy(obj) // don't call our .Copy() directly! + if err != nil { + return nil, err + } + result, ok := x.(*PkgRes) + if !ok { + // bug! + return nil, fmt.Errorf("res is not the same kind") + } + + // if these two were compatible then if they're not identical, then one + // must be PkgStateNewest and the other is PkgStateInstalled, so we + // upgrade to the best common denominator + if obj.State != res.State { + result.State = PkgStateNewest + } + + return result, nil +} + +// Copy copies the resource. Don't call it directly, use engine.ResCopy instead. +// TODO: should this copy internal state? +func (obj *PkgRes) Copy() engine.CopyableRes { + return &PkgRes{ + State: obj.State, + AllowUntrusted: obj.AllowUntrusted, + AllowNonFree: obj.AllowNonFree, + AllowUnsupported: obj.AllowUnsupported, + } +} + // PkgUID is the main UID struct for PkgRes. type PkgUID struct { engine.BaseUID diff --git a/lang/interpret_test.go b/lang/interpret_test.go index 7f9758ca..9e2f55fc 100644 --- a/lang/interpret_test.go +++ b/lang/interpret_test.go @@ -367,6 +367,29 @@ func TestAstFunc0(t *testing.T) { // graph: graph, // }) //} + // // FIXME: blocked by: https://github.com/purpleidea/mgmt/issues/199 + //{ + // graph, _ := pgraph.NewGraph("g") + // v1, v2 := vtex("str(cowsay)"), vtex("str(cowsay)") + // v3, v4 := vtex("str(installed)"), vtex("str(newest)") + // + // graph.AddVertex(&v1, &v2, &v3, &v4) + // + // testCases = append(testCases, test{ + // name: "duplicate resource", + // code: ` + // # these two are allowed because they are compatible + // pkg "cowsay" { + // state => "installed", + // } + // pkg "cowsay" { + // state => "newest", + // } + // `, + // fail: false, + // graph: graph, + // }) + //} { testCases = append(testCases, test{ name: "variable re-declaration and type change error", diff --git a/lang/lang_test.go b/lang/lang_test.go index 4e96d6db..ebe40ee0 100644 --- a/lang/lang_test.go +++ b/lang/lang_test.go @@ -942,6 +942,76 @@ func TestInterpretMany(t *testing.T) { graph: graph, }) } + { + graph, _ := pgraph.NewGraph("g") + r1, _ := engine.NewNamedResource("pkg", "cowsay") + x1 := r1.(*resources.PkgRes) + x1.State = "newest" + graph.AddVertex(x1) + // this second vertex gets merged in because they're compatible + //r2, _ := engine.NewNamedResource("pkg", "cowsay") + //x2 := r2.(*resources.PkgRes) + //x2.State = "installed" + //graph.AddVertex(x2) + testCases = append(testCases, test{ + name: "duplicate compatible pkg resource", + code: ` + pkg "cowsay" { + state => "newest", + } + pkg "cowsay" { + state => "installed", + } + `, + fail: false, + graph: graph, + }) + } + { + // this test ensures that edges are preserved appropriately when + // two or more compatible resources and merged together in graph + graph, _ := pgraph.NewGraph("g") + t1, _ := engine.NewNamedResource("test", "t1") + t2, _ := engine.NewNamedResource("test", "t2") + t3, _ := engine.NewNamedResource("test", "t3") + t4, _ := engine.NewNamedResource("test", "t4") + r1, _ := engine.NewNamedResource("pkg", "cowsay") + x1 := r1.(*resources.PkgRes) + x1.State = "newest" + graph.AddVertex(t1, t2, t3, t4, x1) + // this second vertex gets merged in because they're compatible + //r2, _ := engine.NewNamedResource("pkg", "cowsay") + //x2 := r2.(*resources.PkgRes) + //x2.State = "installed" + //graph.AddVertex(x2) + graph.AddEdge(x1, t1, &engine.Edge{Name: "pkg[cowsay] -> test[t1]"}) // cowsay -> t1 + graph.AddEdge(t2, x1, &engine.Edge{Name: "test[t2] -> pkg[cowsay]"}) // t2 -> cowsay + graph.AddEdge(x1, t3, &engine.Edge{Name: "pkg[cowsay] -> test[t3]"}) // cowsay -> t3 + graph.AddEdge(t4, x1, &engine.Edge{Name: "test[t4] -> pkg[cowsay]"}) // t4 -> cowsay + testCases = append(testCases, test{ + name: "duplicate compatible pkg resource with edges", + code: ` + test "t1" {} + test "t2" {} + test "t3" {} + test "t4" {} + pkg "cowsay" { + state => "newest", + + Before => Test["t1"], # cowsay -> t1 + Depend => Test["t2"], # t2 -> cowsay + } + pkg "cowsay" { + state => "installed", + + Before => Test["t3"], # cowsay -> t3 + Depend => Test["t4"], # t4 -> cowsay + } + `, + fail: false, + graph: graph, + }) + } names := []string{} for index, tc := range testCases { // run all the tests