aboutsummaryrefslogtreecommitdiff
path: root/internal
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 13:53:01 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-09-17 13:53:01 +0200
commita80d470cbad7335fd5e534f32d935e5a49aadb24 (patch)
treec7d92ba026ebb90dfce4ffb8cb547a1db49812b2 /internal
parent78d8313791f05defc9e0a9f2bad8e9710f741a60 (diff)
downloadkrino-a80d470cbad7335fd5e534f32d935e5a49aadb24.tar.gz
krino-a80d470cbad7335fd5e534f32d935e5a49aadb24.zip
a file trashed to make room is logged the moment it is trashed
The displace was carried out first and logged only when the whole step finished - for a copy or a cross-device move, the entire data transfer later. A process killed in that window left the user's file in the Trash with nothing recording it: krino log said "nothing applied" and undo offered nothing. It is its own action with its own line (spec §9), so it is now written the moment trash.Put returns, through a hook ChainLogged calls before the step that needed the name begins. A displace that cannot be logged fails the step rather than compounding an unrecorded destructive act with a second one. Verified by killing krino -9 mid-copy with 600 MB in flight: before: krino log "nothing applied", 0 displace lines after: krino log "1 displaced", 1 displace line
Diffstat (limited to 'internal')
-rw-r--r--internal/apply/apply.go36
-rw-r--r--internal/apply/swap_test.go90
-rw-r--r--internal/engine/apply.go40
3 files changed, 140 insertions, 26 deletions
diff --git a/internal/apply/apply.go b/internal/apply/apply.go
index c0055a2..b36035b 100644
--- a/internal/apply/apply.go
+++ b/internal/apply/apply.go
@@ -45,7 +45,7 @@ type StepResult struct {
// marking the rest skipped. It never touches a file whose size or mtime no
// longer matches what the plan recorded. It is ChainLogged with no done.
func Chain(c plan.Chain) []StepResult {
- results, _ := ChainLogged(context.Background(), c, nil)
+ results, _ := ChainLogged(context.Background(), c, nil, nil)
return results
}
@@ -63,7 +63,7 @@ func Chain(c plan.Chain) []StepResult {
// Once ctx is cancelled (an interrupt), the step already under way finishes
// and every later step is skipped as "interrupted": an interrupt stops after
// the current step, not after the file's whole chain (spec §11).
-func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) {
+func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error, displaced func(i int, step plan.Step, entry string) error) ([]StepResult, error) {
results := make([]StepResult, len(c.Steps))
stopWhy := ""
@@ -86,7 +86,12 @@ func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResu
stopWhy = "an earlier step in this chain failed"
break
}
- res := runStep(step)
+ res := runStep(step, func(entry string) error {
+ if displaced == nil {
+ return nil
+ }
+ return displaced(i, step, entry)
+ })
results[i] = res
switch {
case res.Status == "failed":
@@ -133,11 +138,13 @@ func checkUnchanged(src string, f scan.File) error {
}
// runStep dispatches one already-checked, non-skipped step to the code that
-// actually carries it out.
-func runStep(step plan.Step) StepResult {
+// actually carries it out. reportDisplace is called the moment a displaced
+// file has reached the Trash, before the step that needed its name begins;
+// a step whose displace cannot be reported does not go on to use the name.
+func runStep(step plan.Step, reportDisplace func(entry string) error) StepResult {
switch step.Kind {
case plan.Copy, plan.Move, plan.Rename:
- return runFileStep(step)
+ return runFileStep(step, reportDisplace)
case plan.Trash:
return runTrashStep(step)
case plan.DeletePermanent:
@@ -155,7 +162,13 @@ func runStep(step plan.Step) StepResult {
// overwriting a file the plan never accounted for. Missing destination
// directories are created and recorded in Made, outermost first, whether or
// not the step that needed them goes on to succeed.
-func runFileStep(step plan.Step) StepResult {
+//
+// The displace is reported through reportDisplace as soon as trash.Put
+// returns, not when this step finishes: the user's file is in the Trash
+// from that moment, durably, and for a copy or a cross-device move the rest
+// of the step is the whole data transfer. A process killed in that window
+// used to leave the file in the Trash with nothing in the log to say so.
+func runFileStep(step plan.Step, reportDisplace func(entry string) error) StepResult {
dst := step.Dst
var displacedEntry string
@@ -178,6 +191,15 @@ func runFileStep(step plan.Step) StepResult {
return StepResult{Step: step, Status: "failed", Detail: "displacing the existing file: " + err.Error()}
}
displacedEntry = entry
+ if reportDisplace != nil {
+ if err := reportDisplace(entry); err != nil {
+ // The file is already in the Trash and cannot be recorded.
+ // Using the name now would compound an unlogged destructive
+ // act with a second one.
+ return StepResult{Step: step, Status: "failed", DisplacedEntry: entry,
+ Detail: "the file it replaces went to the Trash but could not be logged, so undo cannot see it: " + err.Error()}
+ }
+ }
} else if _, err := os.Lstat(dst); err == nil {
free, err := nextFreeName(dst)
if err != nil {
diff --git a/internal/apply/swap_test.go b/internal/apply/swap_test.go
index d16d3f9..297db72 100644
--- a/internal/apply/swap_test.go
+++ b/internal/apply/swap_test.go
@@ -4,6 +4,7 @@ package apply
import (
"context"
+ "errors"
"os"
"path/filepath"
"strings"
@@ -156,7 +157,7 @@ func TestChainLoggedReportsEachStepBeforeTheNext(t *testing.T) {
}
}
return nil
- })
+ }, nil)
if err != nil || len(calls) != 2 || res[1].Status != "ok" {
t.Fatalf("calls %v, err %v, results %+v", calls, err, res)
}
@@ -206,7 +207,7 @@ func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) {
cancel()
}
return nil
- })
+ }, nil)
if err != nil {
t.Fatal(err)
}
@@ -217,3 +218,88 @@ func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) {
t.Errorf("the finished step was undone or never ran: %v", err)
}
}
+
+// TestDisplaceIsReportedBeforeTheStepThatNeededIt: trashing the file in the
+// way is a destructive act of its own, and it is durable the moment
+// trash.Put returns. Reporting it only when the whole step finishes leaves
+// a window - the entire data transfer of a copy or a cross-device move -
+// in which the user's file is in the Trash with nothing recording that it
+// went there. Killed in that window, krino log said "nothing applied".
+func TestDisplaceIsReportedBeforeTheStepThatNeededIt(t *testing.T) {
+ root := t.TempDir()
+ // The Trash must be on the same filesystem as the file being trashed.
+ t.Setenv("XDG_DATA_HOME", filepath.Join(root, "share"))
+ src := filepath.Join(root, "a.pdf")
+ dst := filepath.Join(root, "W", "a.pdf")
+ if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(dst, []byte("the file already there"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ c := planned(t, root, "a.pdf", "incoming",
+ plan.Step{Kind: plan.Copy, Src: src, Dst: dst, Displaces: dst},
+ )
+
+ var order []string
+ res, err := ChainLogged(context.Background(), c,
+ func(i int, sr StepResult) error {
+ order = append(order, "step")
+ return nil
+ },
+ func(i int, step plan.Step, entry string) error {
+ order = append(order, "displace")
+ if entry == "" {
+ t.Error("the displace was reported with no trash entry")
+ }
+ // The file is in the Trash already; the copy has not begun.
+ if b, err := os.ReadFile(dst); err == nil && string(b) == "incoming" {
+ t.Error("the displace was reported only after the copy had run")
+ }
+ return nil
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(order) != 2 || order[0] != "displace" || order[1] != "step" {
+ t.Errorf("order = %v; want the displace reported first", order)
+ }
+ if res[0].Status != "ok" {
+ t.Errorf("step = %+v", res[0])
+ }
+}
+
+// TestDisplaceThatCannotBeReportedFailsTheStep: if the displace cannot be
+// written to the log, the step must not go on to use the name - the user's
+// file is already in the Trash and nothing would record it.
+func TestDisplaceThatCannotBeReportedFailsTheStep(t *testing.T) {
+ root := t.TempDir()
+ t.Setenv("XDG_DATA_HOME", filepath.Join(root, "share"))
+ src := filepath.Join(root, "a.pdf")
+ dst := filepath.Join(root, "W", "a.pdf")
+ if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(dst, []byte("the file already there"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ c := planned(t, root, "a.pdf", "incoming",
+ plan.Step{Kind: plan.Copy, Src: src, Dst: dst, Displaces: dst},
+ )
+ res, err := ChainLogged(context.Background(), c, nil,
+ func(i int, step plan.Step, entry string) error {
+ return errors.New("log is closed")
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res[0].Status != "failed" || !strings.Contains(res[0].Detail, "could not be logged") {
+ t.Errorf("step = %+v; want a failure naming the unlogged displace", res[0])
+ }
+ if res[0].DisplacedEntry == "" {
+ t.Error("the failure does not carry the trash entry, so nothing can say where the file went")
+ }
+ if b, rerr := os.ReadFile(dst); rerr == nil && string(b) == "incoming" {
+ t.Error("the copy ran even though the displace could not be logged")
+ }
+}
diff --git a/internal/engine/apply.go b/internal/engine/apply.go
index 1e59aa1..e2d4db8 100644
--- a/internal/engine/apply.go
+++ b/internal/engine/apply.go
@@ -118,6 +118,11 @@ func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, ap
return unloggedStep(rel, c.Steps[i], sr, err)
}
return nil
+ }, func(i int, step plan.Step, entry string) error {
+ // The displaced file is in the Trash from this moment, whatever
+ // becomes of the step that needed its name, so its line is written
+ // here rather than with the rest of the step.
+ return e.logDisplace(j, run, dirName, rel, i+1, step, entry)
})
if err != nil {
return FileResult{}, err
@@ -125,6 +130,24 @@ func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, ap
return FileResult{File: c.File, Steps: results}, nil
}
+// logDisplace writes the line for a file trashed to make room, as soon as
+// it has reached the Trash. It is its own action with its own line (spec
+// §9), independent of whether the step that needed the name then succeeds.
+func (e *Engine) logDisplace(j *journal.Writer, run, dirName, rel string, stepNum int, step plan.Step, entry string) error {
+ dst := filepath.Join(trash.Dir(), "files", entry)
+ size, mtime := statSizeModTime(dst)
+ return j.Append(journal.Entry{
+ Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum,
+ Action: "displace", Status: "ok", Rule: step.Rule,
+ // Detail carries the trash entry name explicitly: Dst's shape
+ // (trash.Dir()/files/<entry>) is internal/apply's and this file's
+ // own convention, not a contract undo may quietly depend on.
+ // PlanUndo/ApplyUndo read the name from here, never by taking
+ // Dst's basename.
+ Src: step.Displaces, Dst: dst, Size: size, ModTime: mtime, Detail: entry,
+ })
+}
+
// unloggedStep is the error for a step whose log entry could not be
// written. A step that ran is named with where its file is now: undo
// cannot see it, so the user must be told where to look.
@@ -233,23 +256,6 @@ func actionName(k plan.Kind) string {
// step's planned destination (informational only — PlanUndo never reverses
// a non-"ok" entry) and Size/ModTime stay zero.
func (e *Engine) logStep(j *journal.Writer, run, dirName, rel string, stepNum int, step plan.Step, sr apply.StepResult) error {
- if sr.DisplacedEntry != "" {
- dst := filepath.Join(trash.Dir(), "files", sr.DisplacedEntry)
- size, mtime := statSizeModTime(dst)
- if err := j.Append(journal.Entry{
- Time: e.Now(), Run: run, Dir: dirName, File: rel, Step: stepNum,
- Action: "displace", Status: "ok", Rule: step.Rule,
- // Detail carries the trash entry name explicitly: Dst's shape
- // (trash.Dir()/files/<entry>) is internal/apply's and this
- // file's own convention, not a contract undo may quietly
- // depend on. PlanUndo/ApplyUndo read the name from here,
- // never by taking Dst's basename.
- Src: step.Displaces, Dst: dst, Size: size, ModTime: mtime, Detail: sr.DisplacedEntry,
- }); err != nil {
- return err
- }
- }
-
for _, dir := range sr.Made {
size, mtime := statSizeModTime(dir)
if err := j.Append(journal.Entry{