engine: resources: pkg: Implement the CompatibleRes interface

This signals to an interested consumer that two or more compatible
resources can be merged safely. This is so that we can avoid the
"duplicate resource" design problem that puppet had.

To test this, you can run:

./mgmt run --tmp-prefix lang --lang 'pkg "cowsay" { state => "installed", } pkg "cowsay" { state => "newest", }'

which should work.

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

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",

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