internal/app: relocate ops condition

This allows reuse and finer grained testing of fromConfig.

Signed-off-by: Ophestra <cat@gensokyo.uk>

internal/app/finalise.go

@@ -47,8 +47,6 @@ func (k *outcome) finalise(ctx context.Context, msg container.Msg, id *state.ID,
 	// only used for a nil configured env map
 	const envAllocSize = 1 << 6
 
-	var kp finaliseProcess
-
 	if ctx == nil || id == nil {
 		// unreachable
 		panic("invalid call to finalise")
@@ -73,6 +71,8 @@ func (k *outcome) finalise(ctx context.Context, msg container.Msg, id *state.ID,
 		k.ct = ct
 	}
 
+	var kp finaliseProcess
+
 	// hsu expects numerical group ids
 	kp.supp = make([]string, len(config.Groups))
 	for i, name := range config.Groups {
@@ -105,52 +105,25 @@ func (k *outcome) finalise(ctx context.Context, msg container.Msg, id *state.ID,
 	kp.runDirPath, kp.identity, kp.id = s.sc.RunDirPath, s.identity, s.id
 	sys := system.New(k.ctx, msg, s.uid.unwrap())
 
-	{
+	ops := fromConfig(config)
-		ops := []outcomeOp{
-			// must run first
-			&spParamsOp{},
 
-			// TODO(ophestra): move this late for #8 and #9
+	stateSys := outcomeStateSys{sys: sys, outcomeState: &s}
-			spFilesystemOp{},
+	for _, op := range ops {
+		if err := op.toSystem(&stateSys, config); err != nil {
+			return err
+		}
+	}
 
-			spRuntimeOp{},
+	// TODO(ophestra): move to shim
-			spTmpdirOp{},
+	stateParams := outcomeStateParams{params: &k.container, outcomeState: &s}
-			spAccountOp{},
+	if s.Container.Env == nil {
-		}
+		stateParams.env = make(map[string]string, envAllocSize)
-
+	} else {
-		et := config.Enablements.Unwrap()
+		stateParams.env = maps.Clone(s.Container.Env)
-		if et&hst.EWayland != 0 {
+	}
-			ops = append(ops, &spWaylandOp{})
+	for _, op := range ops {
-		}
+		if err := op.toContainer(&stateParams); err != nil {
-		if et&hst.EX11 != 0 {
+			return err
-			ops = append(ops, &spX11Op{})
-		}
-		if et&hst.EPulse != 0 {
-			ops = append(ops, &spPulseOp{})
-		}
-		if et&hst.EDBus != 0 {
-			ops = append(ops, &spDBusOp{})
-		}
-		ops = append(ops, spFinal{})
-
-		stateSys := outcomeStateSys{sys: sys, outcomeState: &s}
-		for _, op := range ops {
-			if err := op.toSystem(&stateSys, config); err != nil {
-				return err
-			}
-		}
-
-		// TODO(ophestra): move to shim
-		stateParams := outcomeStateParams{params: &k.container, outcomeState: &s}
-		if s.Container.Env == nil {
-			stateParams.env = make(map[string]string, envAllocSize)
-		} else {
-			stateParams.env = maps.Clone(s.Container.Env)
-		}
-		for _, op := range ops {
-			if err := op.toContainer(&stateParams); err != nil {
-				return err
-			}
 		}
 	}
 

internal/app/outcome.go

@@ -118,16 +118,12 @@ func (s *outcomeState) populateLocal(k syscallDispatcher, msg container.Msg) err
 // instancePath returns a path formatted for outcomeStateSys.instance.
 // This method must only be called from outcomeOp.toContainer if
 // outcomeOp.toSystem has already called outcomeStateSys.instance.
-func (s *outcomeState) instancePath() *check.Absolute {
+func (s *outcomeState) instancePath() *check.Absolute { return s.sc.SharePath.Append(s.id.String()) }
-	return s.sc.SharePath.Append(s.id.String())
-}
 
 // runtimePath returns a path formatted for outcomeStateSys.runtime.
 // This method must only be called from outcomeOp.toContainer if
 // outcomeOp.toSystem has already called outcomeStateSys.runtime.
-func (s *outcomeState) runtimePath() *check.Absolute {
+func (s *outcomeState) runtimePath() *check.Absolute { return s.sc.RunDirPath.Append(s.id.String()) }
-	return s.sc.RunDirPath.Append(s.id.String())
-}
 
 // outcomeStateSys wraps outcomeState and [system.I]. Used on the priv side only.
 // Implementations of outcomeOp must not access fields other than sys unless explicitly stated.
@@ -212,3 +208,37 @@ type outcomeOp interface {
 	// by flattened env map.
 	toContainer(state *outcomeStateParams) error
 }
+
+// fromConfig returns a corresponding slice of outcomeOp for [hst.Config].
+// This function assumes the caller has already called the Validate method on [hst.Config]
+// and checked that it returns nil.
+func fromConfig(config *hst.Config) (ops []outcomeOp) {
+	ops = []outcomeOp{
+		// must run first
+		&spParamsOp{},
+
+		// TODO(ophestra): move this late for #8 and #9
+		spFilesystemOp{},
+
+		spRuntimeOp{},
+		spTmpdirOp{},
+		spAccountOp{},
+	}
+
+	et := config.Enablements.Unwrap()
+	if et&hst.EWayland != 0 {
+		ops = append(ops, &spWaylandOp{})
+	}
+	if et&hst.EX11 != 0 {
+		ops = append(ops, &spX11Op{})
+	}
+	if et&hst.EPulse != 0 {
+		ops = append(ops, &spPulseOp{})
+	}
+	if et&hst.EDBus != 0 {
+		ops = append(ops, &spDBusOp{})
+	}
+
+	ops = append(ops, spFinal{})
+	return
+}

internal/app/outcome_test.go

@@ -0,0 +1,77 @@
+package app
+
+import (
+	"reflect"
+	"testing"
+
+	"hakurei.app/hst"
+	"hakurei.app/internal/app/state"
+)
+
+func TestOutcomeStateValid(t *testing.T) {
+	testCases := []struct {
+		name string
+		s    *outcomeState
+		want bool
+	}{
+		{"nil", nil, false},
+		{"zero", new(outcomeState), false},
+		{"id", &outcomeState{Container: new(hst.ContainerConfig), EnvPaths: new(EnvPaths)}, false},
+		{"container", &outcomeState{ID: new(state.ID), EnvPaths: new(EnvPaths)}, false},
+		{"envpaths", &outcomeState{ID: new(state.ID), Container: new(hst.ContainerConfig)}, false},
+		{"valid", &outcomeState{ID: new(state.ID), Container: new(hst.ContainerConfig), EnvPaths: new(EnvPaths)}, true},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := tc.s.valid(); got != tc.want {
+				t.Errorf("valid: %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestFromConfig(t *testing.T) {
+	testCases := []struct {
+		name   string
+		config *hst.Config
+		want   []outcomeOp
+	}{
+		{"ne", new(hst.Config), []outcomeOp{
+			&spParamsOp{},
+			spFilesystemOp{},
+			spRuntimeOp{},
+			spTmpdirOp{},
+			spAccountOp{},
+			spFinal{},
+		}},
+		{"wayland pulse", &hst.Config{Enablements: hst.NewEnablements(hst.EWayland | hst.EPulse)}, []outcomeOp{
+			&spParamsOp{},
+			spFilesystemOp{},
+			spRuntimeOp{},
+			spTmpdirOp{},
+			spAccountOp{},
+			&spWaylandOp{},
+			&spPulseOp{},
+			spFinal{},
+		}},
+		{"all", &hst.Config{Enablements: hst.NewEnablements(0xff)}, []outcomeOp{
+			&spParamsOp{},
+			spFilesystemOp{},
+			spRuntimeOp{},
+			spTmpdirOp{},
+			spAccountOp{},
+			&spWaylandOp{},
+			&spX11Op{},
+			&spPulseOp{},
+			&spDBusOp{},
+			spFinal{},
+		}},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := fromConfig(tc.config); !reflect.DeepEqual(got, tc.want) {
+				t.Errorf("fromConfig: %#v, want %#v", got, tc.want)
+			}
+		})
+	}
+}