From 33d771438f1da363af7c9ff2d5f4c368c326e84d Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Thu, 17 Sep 2026 14:03:44 +0200 Subject: {now} is the start of the run, as the spec always said MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clock was read once per directory, so a run of several directories stamped several different times - and a review that took a few seconds could put one run's files into two folders, or at midnight two dates. The spec (§7.3) says "the start of the run"; krino.conf(5) documented the behaviour rather than the intent, so the two contradicted each other. The session now carries the run's clock and every directory plans with it. Engine.Plan keeps its meaning for a caller with no session of its own; Session.Plan passes the run's own start. --- internal/engine/plan.go | 10 +++++++- internal/engine/session.go | 12 +++++++--- internal/engine/session_test.go | 53 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) (limited to 'internal') diff --git a/internal/engine/plan.go b/internal/engine/plan.go index bf4c64c..b136bdf 100644 --- a/internal/engine/plan.go +++ b/internal/engine/plan.go @@ -34,6 +34,14 @@ type DirPlan struct { // directories claiming one destination resolve the collision instead of // both silently landing on it. func (e *Engine) Plan(ctx context.Context, d *Dir, claims *plan.Claims) (*DirPlan, error) { + return e.PlanAt(ctx, d, claims, e.Now()) +} + +// PlanAt is Plan with the run's own clock: {now:FMT} is "the start of the +// run" (spec §7.3), so every directory of one run stamps the same moment. +// Reading the clock per directory let a review pause split one run's output +// across two folders, and across midnight across two dates. +func (e *Engine) PlanAt(ctx context.Context, d *Dir, claims *plan.Claims, now time.Time) (*DirPlan, error) { started := time.Now() r, err := e.Match(ctx, d) if err != nil { @@ -45,7 +53,7 @@ func (e *Engine) Plan(ctx context.Context, d *Dir, claims *plan.Claims) (*DirPla inputs[i] = plan.Input{File: fm.File, Rules: planRules(fm.Rules), NoDelete: fm.NoDelete} } - chains := plan.Build(d.Root, inputs, e.Now(), plan.OS{}, claims) + chains := plan.Build(d.Root, inputs, now, plan.OS{}, claims) return &DirPlan{Dir: d, Chains: chains, Result: r, Elapsed: time.Since(started)}, nil } diff --git a/internal/engine/session.go b/internal/engine/session.go index 8d5977a..09cf246 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -5,6 +5,7 @@ package engine import ( "context" "fmt" + "time" "git.labunix.xyz/krino/internal/journal" "git.labunix.xyz/krino/internal/lock" @@ -27,6 +28,10 @@ type Session struct { claims *plan.Claims landed []string // where this run's applied files ended up, in order dry bool + + // started is the run's own clock: what {now:FMT} means for every + // directory of this run, fixed when the session begins. + started time.Time } // NewSession starts a run. The log is opened by OpenLog, or by the first @@ -35,7 +40,7 @@ type Session struct { // create a log a dry run would not. The caller closes the session when the // run is over. func (e *Engine) NewSession(dry bool) (*Session, error) { - return &Session{e: e, claims: plan.NewClaims(), dry: dry}, nil + return &Session{e: e, claims: plan.NewClaims(), dry: dry, started: e.Now()}, nil } // OpenLog opens the log and takes the run id, once. A dry session does @@ -86,9 +91,10 @@ func (s *Session) LockDirs(ctx context.Context, names []string, wait bool) ([]*l return held, nil } -// Plan builds d's plan with the run's claims. +// Plan builds d's plan with the run's claims, and the run's clock: every +// directory of one run stamps {now:FMT} with the same moment. func (s *Session) Plan(ctx context.Context, d *Dir) (*DirPlan, error) { - return s.e.Plan(ctx, d, s.claims) + return s.e.PlanAt(ctx, d, s.claims, s.started) } // Apply carries out the approved files of dp and logs the run's steps, diff --git a/internal/engine/session_test.go b/internal/engine/session_test.go index 5cab274..750fa04 100644 --- a/internal/engine/session_test.go +++ b/internal/engine/session_test.go @@ -270,3 +270,56 @@ func TestSessionDropsUnappliedClaims(t *testing.T) { t.Errorf("beta planned %q; the declined destination should be free", got) } } + +// TestNowIsTheStartOfTheRun: {now:FMT} is documented as "the start of the +// run" (spec §7.3), and the plan a user approves must not change under +// them. Reading the clock once per directory put one run's files into two +// folders when the review took a moment - across midnight, two dates. +func TestNowIsTheStartOfTheRun(t *testing.T) { + h := sandbox(t) + dl := filepath.Join(h, "dl") + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for _, n := range []string{"a.pdf", "b.pdf"} { + p := filepath.Join(dl, n) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, old, old); err != nil { + t.Fatal(err) + } + } + conf := "(path \"~/dl\")\n(min-age 0s)\n(rule \"stamp\" (move \"Out/{now:%H%M%S}\"))\n" + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": conf}) + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + // A clock that moves on every reading, as a review pause does. + tick := time.Date(2026, 9, 17, 23, 59, 59, 0, time.UTC) + e.Now = func() time.Time { + tick = tick.Add(3 * time.Second) + return tick + } + + sess, err := e.NewSession(true) + if err != nil { + t.Fatal(err) + } + defer sess.Close() + first, err := sess.Plan(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + second, err := sess.Plan(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + a := filepath.Dir(first.Chains[0].Steps[0].Dst) + b := filepath.Dir(second.Chains[0].Steps[0].Dst) + if a != b { + t.Errorf("one run stamped two different folders: %s and %s", a, b) + } +} -- cgit v1.3