From ee6c471fe6e183d78e396a23b2a92fd26bb31143 Mon Sep 17 00:00:00 2001 From: Ophestra Date: Wed, 8 Oct 2025 19:39:00 +0900 Subject: [PATCH] internal/app: relocate ops condition This allows reuse and finer grained testing of fromConfig. Signed-off-by: Ophestra --- internal/app/finalise.go | 65 +++++++++--------------------- internal/app/outcome.go | 42 +++++++++++++++++--- internal/app/outcome_test.go | 77 ++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 52 deletions(-) create mode 100644 internal/app/outcome_test.go diff --git a/internal/app/finalise.go b/internal/app/finalise.go index bd9cf2b..491a775 100644 --- a/internal/app/finalise.go +++ b/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 := []outcomeOp{ - // must run first - &spParamsOp{}, + ops := fromConfig(config) - // TODO(ophestra): move this late for #8 and #9 - spFilesystemOp{}, + stateSys := outcomeStateSys{sys: sys, outcomeState: &s} + for _, op := range ops { + if err := op.toSystem(&stateSys, config); err != nil { + return err + } + } - 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{}) - - 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 - } + // 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 } } diff --git a/internal/app/outcome.go b/internal/app/outcome.go index f0880ad..671d14c 100644 --- a/internal/app/outcome.go +++ b/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 { - return s.sc.SharePath.Append(s.id.String()) -} +func (s *outcomeState) instancePath() *check.Absolute { 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 { - return s.sc.RunDirPath.Append(s.id.String()) -} +func (s *outcomeState) runtimePath() *check.Absolute { 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 +} diff --git a/internal/app/outcome_test.go b/internal/app/outcome_test.go new file mode 100644 index 0000000..a16f0a1 --- /dev/null +++ b/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) + } + }) + } +}