Merge pull request 'A contribution depends on the seat that receives it; a collision is refused at assign (hq ADR 0210, issue 235)' (#267) from feat/0210-a-contribution-depends-on-its-seat into main

cmd/mesh-controller/acts.go

@@ -160,6 +160,11 @@ func seatDependenciesOnAssign(ctx context.Context, open *stores, node string, mo
 	if _, err := catalogue.AssignRefusal(shelf, node, assigned, adding); err != nil {
 		return nil, nil, err
 	}
+	// Two modules declaring one package, path or unit is refused before anything is recorded
+	// (novox/hq ADR 0210, 04-ISSUES/235): kept, the node would not resolve until one came off again.
+	if err := catalogue.CollisionRefusal(shelf, node, assigned, adding); err != nil {
+		return nil, nil, err
+	}
 	return shelf, assigned, nil
 }
 

internal/catalogue/contribution_dependencies_test.go

@@ -0,0 +1,100 @@
+package catalogue
+
+import (
+	"reflect"
+	"strings"
+	"testing"
+)
+
+// Defends novox/hq ADR 0210 §3: a contribution is a dependency on the seat that receives it.
+
+func TestAContributionDependsOnTheSeatThatPlacesIt(t *testing.T) {
+	env := mod("theme", nil, nil, nil)
+	env.Environment = &Environment{Variables: map[string]string{"GTK_THEME": "Adwaita:dark"}}
+	path := mod("toolchain", nil, nil, nil)
+	path.Environment = &Environment{Path: []PathEntry{{Entry: "/opt/x/bin"}}}
+	shell := mod("prompt", nil, nil, nil)
+	shell.Shell = []ShellCode{{For: "zsh", Slot: "first", Code: "true"}}
+	session := mod("wallpaper", nil, nil, nil)
+	session.Shell = []ShellCode{{For: "xinitrc", Slot: "normal", Code: "true"}}
+	resources := mod("bar", nil, nil, nil)
+	resources.Shell = []ShellCode{{For: "xresources", Slot: "normal", Code: "x: y"}}
+	empty := mod("nothing", nil, nil, nil)
+	empty.Environment = &Environment{}
+
+	cases := map[string]struct {
+		m    Manifest
+		want []string
+	}{
+		"a variable":                {env, []string{EnvironmentSeat}},
+		"a path entry":              {path, []string{EnvironmentSeat}},
+		"shell code":                {shell, []string{LoginShellSeat}},
+		"the session's start":       {session, []string{DisplayServerSeat}},
+		"the session's X resources": {resources, []string{DisplayServerSeat}},
+		"an empty environment":      {empty, nil},
+	}
+	for name, c := range cases {
+		got := DependsOn(c.m)
+		if len(got) == 0 {
+			got = nil
+		}
+		if !reflect.DeepEqual(got, c.want) {
+			t.Errorf("%s depends on %v, want %v", name, got, c.want)
+		}
+	}
+}
+
+func TestAContributionIsMetByAHolderOnTheNodeAndRefusedWithout(t *testing.T) {
+	holder := mod("node-env", nil, nil, nil, Claim{Name: EnvironmentSeat})
+	contributor := mod("theme", nil, nil, nil)
+	contributor.Environment = &Environment{Variables: map[string]string{"GTK_THEME": "Adwaita:dark"}}
+	catalogue := map[string]Manifest{"node-env": holder, "theme": contributor}
+
+	if _, err := AssignRefusal(catalogue, "laptop", []string{"node-env"}, []string{"theme"}); err != nil {
+		t.Fatalf("a contributor beside the holder is refused: %v", err)
+	}
+	_, err := AssignRefusal(catalogue, "laptop", nil, []string{"theme"})
+	if err == nil {
+		t.Fatal("a contributor on a node without the holder was accepted, and its contribution would be written nowhere")
+	}
+	if !strings.Contains(err.Error(), EnvironmentSeat) || !strings.Contains(err.Error(), "node-env") {
+		t.Errorf("the refusal names neither the seat nor its holder: %v", err)
+	}
+	if _, err := AssignRefusal(catalogue, "laptop", nil, []string{"theme", "node-env"}); err != nil {
+		t.Errorf("the contributor and the holder assigned together are refused: %v", err)
+	}
+}
+
+func TestAHolderMeetsItsOwnContribution(t *testing.T) {
+	// The display server's module contributes nothing to its own seat today, but the zsh module's
+	// environment contributions do go to another seat: a claim meets only the seat it names.
+	zsh := mod("zsh", nil, nil, nil, Claim{Name: LoginShellSeat})
+	zsh.Shell = []ShellCode{{For: "zsh", Slot: "normal", Code: "true"}}
+	zsh.Environment = &Environment{Variables: map[string]string{"EDITOR": "vim"}}
+	catalogue := map[string]Manifest{"zsh": zsh,
+		"node-env": mod("node-env", nil, nil, nil, Claim{Name: EnvironmentSeat})}
+	unheld := UnheldDependencies(catalogue, "laptop", []Manifest{zsh}, nil)
+	if len(unheld) != 1 || unheld[0].Seat != EnvironmentSeat {
+		t.Errorf("zsh alone: unheld %v, want only %s (its shell code is its own seat's)", unheld, EnvironmentSeat)
+	}
+}
+
+func TestTwoModulesDeclaringOnePackageAreRefusedBeforeAnythingIsRecorded(t *testing.T) {
+	pacman := withResources(mod("pacman", nil, nil, nil, Claim{Name: PackageManagerSeat}),
+		res("package", "pacman-contrib"))
+	bar := withResources(mod("bar", nil, nil, nil), res("package", "bar"), res("package", "pacman-contrib"))
+	other := withResources(mod("other", nil, nil, nil), res("package", "other"))
+	catalogue := map[string]Manifest{"pacman": pacman, "bar": bar, "other": other}
+
+	err := CollisionRefusal(catalogue, "laptop", []string{"pacman"}, []string{"bar"})
+	if err == nil || !strings.Contains(err.Error(), "pacman-contrib") || !strings.Contains(err.Error(), "bar") {
+		t.Fatalf("the second owner of a package was not refused by name: %v", err)
+	}
+	if err := CollisionRefusal(catalogue, "laptop", []string{"pacman"}, []string{"other"}); err != nil {
+		t.Errorf("a module declaring nothing shared is refused: %v", err)
+	}
+	// A collision already on the node is status's, not a reason to refuse an unrelated assignment.
+	if err := CollisionRefusal(catalogue, "laptop", []string{"pacman", "bar"}, []string{"other"}); err != nil {
+		t.Errorf("an unrelated assignment is refused for a collision already there: %v", err)
+	}
+}

internal/catalogue/seat_dependencies.go

@@ -6,7 +6,8 @@ import (
 	"strings"
 )
 
-// A module depends on the node seats that apply its resources (novox/hq ADR 0207).
+// A module depends on the node seats that apply its resources (novox/hq ADR 0207), and on the
+// seats it contributes to (novox/hq ADR 0210).
 //
 // Some of what a module declares is applied through software on the machine that is itself a
 // module: a service through the service manager, a package through the package manager, a container
@@ -89,6 +90,9 @@ func DependsOn(m Manifest) []string {
 			seen[seat] = true
 		}
 	}
+	for _, seat := range contributedTo(m) {
+		seen[seat] = true
+	}
 	out := make([]string, 0, len(seen))
 	for s := range seen {
 		out = append(out, s)
@@ -97,6 +101,22 @@ func DependsOn(m Manifest) []string {
 	return out
 }
 
+// contributedTo is every seat a module contributes to (novox/hq ADR 0210 §3): a contribution is
+// configuration only the seat's holder applies, so it is a dependency on that seat exactly as a
+// resource is on the seat that applies it. The environment goes to node-environment's holder
+// (ADR 0203); shell code to the holder that places it for its target — the login shell's for a
+// shell, the display server's for the session's start and resources (ADR 0204, ADR 0208 §4).
+func contributedTo(m Manifest) []string {
+	var out []string
+	if e := m.Environment; e != nil && (len(e.Variables) > 0 || len(e.Path) > 0) {
+		out = append(out, EnvironmentSeat)
+	}
+	for _, c := range m.Shell {
+		out = append(out, placerOf(c.For))
+	}
+	return out
+}
+
 // claimsSeat is whether a module claims a node seat, by its current name or one it used to have
 // (ADR 0122), so a rename leaves the dependency met.
 func claimsSeat(m Manifest, seat string) bool {
@@ -158,10 +178,8 @@ func (u Unheld) String() string {
 func UnheldDependencies(catalogue map[string]Manifest, node string, set []Manifest, judged map[string]bool) []Unheld {
 	held := map[string]bool{}
 	for _, m := range set {
-		for seat := range seatsApplying() {
+		for _, seat := range nodeSeatsClaimed(m) {
-			if claimsSeat(m, seat) {
+			held[seat] = true
-				held[seat] = true
-			}
 		}
 	}
 	var out []Unheld
@@ -186,10 +204,19 @@ func UnheldDependencies(catalogue map[string]Manifest, node string, set []Manife
 	return out
 }
 
-func seatsApplying() map[string]bool {
+// nodeSeatsClaimed is every node seat a module claims, each by its current name (ADR 0122), so
-	out := map[string]bool{}
+// a dependency on any of them — a resource's or a contribution's — is met by the claim.
-	for _, s := range appliedThrough {
+func nodeSeatsClaimed(m Manifest) []string {
-		out[s] = true
+	var out []string
+	for _, c := range m.Claims {
+		if c.At() != ScopeNode {
+			continue
+		}
+		name := c.Name
+		if s, known := SeatNamed(name); known {
+			name = s.Name
+		}
+		out = append(out, name)
 	}
 	return out
 }
@@ -284,3 +311,32 @@ func UnassignRefusal(catalogue map[string]Manifest, node string, assigned, remov
 	}
 	return &Refusal{Problems: problems}
 }
+
+// CollisionRefusal is why assigning `adding` beside `assigned` is refused for what two modules
+// would both declare, or nothing (novox/hq ADR 0210 §1, 04-ISSUES/235).
+//
+// **Refused, not kept like an unresolved provision.** An assignment is otherwise kept when the
+// node does not resolve, because assignment is not an ordering: a consumer's provider can follow.
+// A collision is not an order anything can complete — no further assignment makes two owners of one
+// package one owner — and kept, it leaves the node unresolvable, so the next push of anything drops
+// it from the mesh. Only collisions involving a module being added are refused; one already on the
+// node is `status`'s, and refusing an unrelated assignment for it would block its own remedy.
+func CollisionRefusal(catalogue map[string]Manifest, node string, assigned, adding []string) error {
+	before := map[string]bool{}
+	for _, p := range checkResources(manifestsOf(catalogue, assigned)) {
+		before[p] = true
+	}
+	var problems []string
+	for _, p := range checkResources(manifestsOf(catalogue, append(append([]string(nil), assigned...), adding...))) {
+		if before[p] {
+			continue // on the node already; not this assignment's doing
+		}
+		problems = append(problems, p+" (novox/hq ADR 0210: one owner per node; the other module "+
+			"depends on the owner's seat instead)")
+	}
+	if len(problems) == 0 {
+		return nil
+	}
+	sort.Strings(problems)
+	return &Refusal{Problems: append(problems, fmt.Sprintf("nothing was assigned to %s", node))}
+}