Put busybox's account removal back, test the sample, and read a held lock as kept only for a running walk (hq ADR 0328 review)

An earlier change dropped alpine's RemoveAccount and RemoveGroup, so alpine made no
module identity at all. The sample beneath a root in shape had no test of its own.
And an apply that found the library's lock held read the library as kept even when
what held it was a take act about to be refused for a missing disk; it now does so
only while a take walk is running, and otherwise waits and judges as always.

internal/apply/identities_test.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/novox/mesh-host/internal/declaration"
 	"github.com/novox/mesh-host/internal/store"
+	"github.com/novox/mesh-host/internal/system"
 )
 
 // The engine's half of ADR 0325 §3 and §11 (issue 567): every module given an account and a group of its own with
@@ -369,3 +370,16 @@ func TestAGrantsGroupThatCouldNotBeLeftIsTriedAgain(t *testing.T) {
 		t.Fatalf("the group was not taken back on the next apply: %+v (%s)", rec, groups)
 	}
 }
+
+// Every system the engine runs on that makes accounts makes module identities: arch and alpine alike.
+func TestArchAndAlpineMakeModuleIdentities(t *testing.T) {
+	for _, name := range []string{"arch", "alpine"} {
+		sys, err := system.For(name)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := toolFor(sys); err != nil {
+			t.Errorf("%s makes no module identity: %v", name, err)
+		}
+	}
+}

internal/apply/library.go

@@ -92,6 +92,10 @@ func libraryLock(name string) *sync.Mutex {
 	return l.(*sync.Mutex)
 }
 
+// libraryWalks is every library a take walk is running under now, with the filesystem it walks: set once the walk
+// has been judged and started, cleared when it ends.
+var libraryWalks sync.Map
+
 // LibraryAccess is the access a library's wait and its take act name.
 func LibraryAccess(name string) string { return declaration.LibraryAccessPrefix + name }
 
@@ -236,11 +240,15 @@ func applyLibrary(ctx context.Context, l declaration.Library) (Outcome, libraryV
 	lock := libraryLock(l.Library)
 	if !lock.TryLock() {
 		// **A take walk is running (up to its bound, half an hour): the apply does not wait for it.** The library is
-		// the walk's until it is through; it keeps it, and its containers are not held for it.
-		rec, _ := libraryRecord(l.Library)
-		out.Action = "unchanged"
-		out.Detail = "the take act is setting its group under its root now; this apply leaves the library to it"
-		return out, libraryVerdict{kind: libraryKeep, how: store.KeptByWarrant, placed: rec.KeptOn}, nil
+		// the walk's until it is through; it keeps it, and its containers are not held for it. Only a walk that
+		// has started, on the filesystem it judged: anything else holding the lock (a take act about to be
+		// refused, say) is short, and the apply waits for it and judges the library as always.
+		if on, walking := libraryWalks.Load(l.Library); walking {
+			out.Action = "unchanged"
+			out.Detail = "the take act is setting its group under its root now; this apply leaves the library to it"
+			return out, libraryVerdict{kind: libraryKeep, how: store.KeptByWarrant, placed: on.(string)}, nil
+		}
+		lock.Lock()
 	}
 	defer lock.Unlock()
 	rec, _ := libraryRecord(l.Library)

internal/apply/library_linux.go

@@ -108,6 +108,8 @@ func TakeLibrary(ctx context.Context, d *declaration.Declaration, access, path,
 // walkLibrary walks a library's root from resume with its group, and records what it did: a walk stopped at its
 // bound keeps its cursor for the next. The caller holds the library's lock.
 func walkLibrary(name, root string, gid int, warrant, by, on string, resume []string) {
+	libraryWalks.Store(name, on)
+	defer libraryWalks.Delete(name)
 	started := time.Now().UTC()
 	w, err := walkRoot(root, ActLimits, machineStats, resume, groupVisit(gid))
 	rec := actRecord(warrant, by, on, started, w, err)

internal/apply/library_test.go

@@ -774,6 +774,8 @@ func TestAnApplyDoesNotWaitForATakeWalk(t *testing.T) {
 	lock := libraryLock("media")
 	lock.Lock()
 	defer lock.Unlock()
+	libraryWalks.Store("media", "fs-test")
+	defer libraryWalks.Delete("media")
 	done := make(chan Report, 1)
 	go func() {
 		r, _, _ := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false)), store.State{})
@@ -788,3 +790,45 @@ func TestAnApplyDoesNotWaitForATakeWalk(t *testing.T) {
 		t.Fatal("the apply waited for the take walk")
 	}
 }
+
+// Review: a root in shape whose data beneath — in no requested sub-path — is not in the group waits: the sample, not
+// the root alone, says whether the data is the library's already (§7).
+func TestTheSampleBeneathARootInShapeIsJudged(t *testing.T) {
+	gid := testGID(t)
+	libraries(t, nil, nil)
+	root := filepath.Join(t.TempDir(), "media")
+	if err := os.MkdirAll(filepath.Join(root, "old"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "old", "film"), []byte("data"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chown(root, -1, gid); err != nil {
+		t.Fatal(err)
+	}
+	if err := unix.Chmod(root, 0o2775); err != nil {
+		t.Fatal(err)
+	}
+	report, _, err := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false)), store.State{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if o := outcomeOf(report, "library:media"); o.Action != "waiting" {
+		t.Fatalf("a root in shape with data beneath not in the group was kept: %+v", o)
+	}
+}
+
+// Review: an apply that finds the library's lock held by something that is no take walk (a take act about to be
+// refused) waits for it and judges the library as always: a library whose disk is not there still holds.
+func TestAnApplyWaitsForALockThatIsNoWalk(t *testing.T) {
+	gid := testGID(t)
+	libraries(t, nil, nil)
+	root := filepath.Join(t.TempDir(), "gone", "media")
+	lock := libraryLock("media")
+	lock.Lock()
+	go func() { time.Sleep(50 * time.Millisecond); lock.Unlock() }()
+	report, _, _ := applyLib(t, libraryDecl(t, libJSON("media", root, gid, true, false)), store.State{})
+	if o := outcomeOf(report, "library:media"); o.Action != "waiting" || !strings.Contains(o.Detail, DiskWords("media")) {
+		t.Fatalf("a library whose root is not there was read as kept while its lock was held: %+v", o)
+	}
+}

internal/system/identities.go

@@ -155,3 +155,19 @@ func (alpine) ModifyAccount(ctx context.Context, run Runner, name string, gid in
 	}
 	return nil
 }
+
+// RemoveAccount removes an account with busybox's deluser.
+func (alpine) RemoveAccount(ctx context.Context, run Runner, name string) error {
+	if _, err := run(ctx, "deluser", name); err != nil {
+		return fmt.Errorf("cannot remove the account %q: %w", name, err)
+	}
+	return nil
+}
+
+// RemoveGroup removes a group with busybox's delgroup.
+func (alpine) RemoveGroup(ctx context.Context, run Runner, name string) error {
+	if _, err := run(ctx, "delgroup", name); err != nil {
+		return fmt.Errorf("cannot remove the group %q: %w", name, err)
+	}
+	return nil
+}

internal/system/identities_test.go

@@ -71,3 +71,21 @@ func TestAnAccountsGroupsChangeOnlyWithNoHome(t *testing.T) {
 		t.Fatalf("the home was not moved away first, without moving files:\n%s", got)
 	}
 }
+
+// busybox removes what the mesh made of an identity with deluser and delgroup.
+func TestBusyboxRemovesAnAccountAndAGroup(t *testing.T) {
+	var ran []string
+	run := func(_ context.Context, name string, args ...string) (string, error) {
+		ran = append(ran, name+" "+strings.Join(args, " "))
+		return "", nil
+	}
+	if err := (alpine{}).RemoveAccount(context.Background(), run, "mesh-radarr"); err != nil {
+		t.Fatal(err)
+	}
+	if err := (alpine{}).RemoveGroup(context.Background(), run, "mesh-media"); err != nil {
+		t.Fatal(err)
+	}
+	if strings.Join(ran, "\n") != "deluser mesh-radarr\ndelgroup mesh-media" {
+		t.Fatalf("removed with: %v", ran)
+	}
+}