From feb8b391c652886f792d283dda100572437796b6 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Wed, 6 May 2026 14:00:00 -0400 Subject: [PATCH 1/2] fix overlay ctime stat cache --- e2e_git_test.go | 30 ++++++++ internal/fusefs/fuse_unix.go | 38 ++++++---- internal/fusefs/fuse_unix_test.go | 36 +++++++++ internal/fusefs/merged.go | 12 ++- internal/fusefs/merged_test.go | 120 +++++++++++++++++++++++++++--- internal/fusefs/ops.go | 36 ++++++--- internal/model/types.go | 2 + internal/overlay/store.go | 94 +++++++++++++++++++---- internal/overlay/store_test.go | 71 ++++++++++++++++++ 9 files changed, 384 insertions(+), 55 deletions(-) create mode 100644 internal/fusefs/fuse_unix_test.go diff --git a/e2e_git_test.go b/e2e_git_test.go index 3614b85..1b026ff 100644 --- a/e2e_git_test.go +++ b/e2e_git_test.go @@ -134,6 +134,36 @@ func TestE2EGitCleanState(t *testing.T) { gitCmdQuiet(t, repo.mountPath, "diff", "--cached", "--quiet") } +func TestE2EGitStatusDetectsSameSizeRewriteAfterMtimeRestore(t *testing.T) { + repo := newMountedE2ERepo(t) + + readmePath := filepath.Join(repo.mountPath, "README.md") + assertGitStatus(t, repo.mountPath, map[string]string{}) + st, err := os.Stat(readmePath) + if err != nil { + t.Fatal(err) + } + indexedMtime := st.ModTime() + + updated := []byte(readFileEventually(t, readmePath)) + if len(updated) == 0 { + t.Fatal("README.md is empty") + } + if updated[0] == 'x' { + updated[0] = 'y' + } else { + updated[0] = 'x' + } + if err := os.WriteFile(readmePath, updated, 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(readmePath, indexedMtime, indexedMtime); err != nil { + t.Fatal(err) + } + + assertGitStatus(t, repo.mountPath, map[string]string{"README.md": " M"}) +} + func TestE2EGitStatusPorcelain(t *testing.T) { repo := newMountedE2ERepo(t) diff --git a/internal/fusefs/fuse_unix.go b/internal/fusefs/fuse_unix.go index f830cba..94518a1 100644 --- a/internal/fusefs/fuse_unix.go +++ b/internal/fusefs/fuse_unix.go @@ -183,7 +183,7 @@ func (fs *ArtifactFuse) LookUpInode(_ context.Context, op *fuseops.LookUpInodeOp return nil } - mode, size, typ, mtime, err := fs.resolver.Getattr(childPath) + mode, size, typ, mtime, ctime, err := fs.resolver.Getattr(childPath) if err != nil { if errors.Is(err, iofs.ErrNotExist) { return syscall.ENOENT @@ -196,7 +196,7 @@ func (fs *ArtifactFuse) LookUpInode(_ context.Context, op *fuseops.LookUpInodeOp fs.mu.Unlock() op.Entry.Child = ref.ID - op.Entry.Attributes = inodeAttrs(mode, uint64(size), typ, mtime) + op.Entry.Attributes = inodeAttrs(mode, uint64(size), typ, mtime, ctime) setChildEntryExpiry(&op.Entry, time.Second) return nil } @@ -213,11 +213,11 @@ func (fs *ArtifactFuse) GetInodeAttributes(_ context.Context, op *fuseops.GetIno return nil } - mode, size, typ, mtime, err := fs.resolver.Getattr(ref.Path) + mode, size, typ, mtime, ctime, err := fs.resolver.Getattr(ref.Path) if err != nil { return syscall.ENOENT } - op.Attributes = inodeAttrs(mode, uint64(size), typ, mtime) + op.Attributes = inodeAttrs(mode, uint64(size), typ, mtime, ctime) op.AttributesExpiration = attrExpiry(time.Second) return nil } @@ -234,17 +234,18 @@ func (fs *ArtifactFuse) SetInodeAttributes(ctx context.Context, op *fuseops.SetI } // Handle mtime updates (e.g., from touch) if op.Mtime != nil { - fs.engine.SetMtime(ctx, ref.Path, *op.Mtime) + if err := fs.engine.SetMtime(ctx, ref.Path, *op.Mtime); err != nil { + if errors.Is(err, iofs.ErrInvalid) { + return syscall.ENOTSUP + } + return syscall.EIO + } } - mode, size, typ, mtime, err := fs.resolver.Getattr(ref.Path) + mode, size, typ, mtime, ctime, err := fs.resolver.Getattr(ref.Path) if err != nil { return syscall.EIO } - // If caller set mtime, return that instead of the stored value - if op.Mtime != nil { - mtime = *op.Mtime - } - op.Attributes = inodeAttrs(mode, uint64(size), typ, mtime) + op.Attributes = inodeAttrs(mode, uint64(size), typ, mtime, ctime) op.AttributesExpiration = attrExpiry(time.Second) return nil } @@ -404,7 +405,8 @@ func (fs *ArtifactFuse) CreateFile(ctx context.Context, op *fuseops.CreateFileOp fs.mu.Unlock() op.Entry.Child = ref.ID - op.Entry.Attributes = inodeAttrs(uint32(op.Mode), 0, "file", time.Now()) + now := time.Now() + op.Entry.Attributes = inodeAttrs(uint32(op.Mode), 0, "file", now, now) setChildEntryExpiry(&op.Entry, time.Second) op.Handle = handle return nil @@ -423,7 +425,8 @@ func (fs *ArtifactFuse) MkDir(ctx context.Context, op *fuseops.MkDirOp) error { fs.mu.Unlock() op.Entry.Child = ref.ID - op.Entry.Attributes = inodeAttrs(uint32(op.Mode)|uint32(os.ModeDir), 4096, "dir", time.Now()) + now := time.Now() + op.Entry.Attributes = inodeAttrs(uint32(op.Mode)|uint32(os.ModeDir), 4096, "dir", now, now) setChildEntryExpiry(&op.Entry, time.Second) return nil } @@ -599,7 +602,7 @@ func TryUnmount(mountPoint string) error { return err } -func inodeAttrs(mode uint32, size uint64, typ string, mtime time.Time) fuseops.InodeAttributes { +func inodeAttrs(mode uint32, size uint64, typ string, mtime time.Time, ctime time.Time) fuseops.InodeAttributes { m := os.FileMode(mode & 0o777) if m == 0 { if typ == "dir" { @@ -623,7 +626,9 @@ func inodeAttrs(mode uint32, size uint64, typ string, mtime time.Time) fuseops.I Mode: m, Uid: uint32(os.Getuid()), Gid: uint32(os.Getgid()), + Atime: mtime, Mtime: mtime, + Ctime: ctime, } } @@ -642,12 +647,15 @@ func setChildEntryExpiry(entry *fuseops.ChildInodeEntry, ttl time.Duration) { } func (fs *ArtifactFuse) gitFileAttrs() fuseops.InodeAttributes { + now := time.Now() return fuseops.InodeAttributes{ Size: uint64(len(fs.gitfileContent)), Mode: 0o644, Nlink: 1, Uid: uint32(os.Getuid()), Gid: uint32(os.Getgid()), - Mtime: time.Now(), + Atime: now, + Mtime: now, + Ctime: now, } } diff --git a/internal/fusefs/fuse_unix_test.go b/internal/fusefs/fuse_unix_test.go new file mode 100644 index 0000000..ba1cdf5 --- /dev/null +++ b/internal/fusefs/fuse_unix_test.go @@ -0,0 +1,36 @@ +//go:build !windows + +package fusefs + +import ( + "testing" + "time" +) + +func TestInodeAttrsPreservesSeparateTimes(t *testing.T) { + mtime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + ctime := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC) + + attr := inodeAttrs(0o644, 12, "file", mtime, ctime) + if !attr.Atime.Equal(mtime) { + t.Fatalf("atime = %v, want %v", attr.Atime, mtime) + } + if !attr.Mtime.Equal(mtime) { + t.Fatalf("mtime = %v, want %v", attr.Mtime, mtime) + } + if !attr.Ctime.Equal(ctime) { + t.Fatalf("ctime = %v, want %v", attr.Ctime, ctime) + } +} + +func TestGitFileAttrsUsesOneTimestamp(t *testing.T) { + fs := &ArtifactFuse{gitfileContent: []byte("gitdir: /tmp/repo/.git\n")} + + attr := fs.gitFileAttrs() + if attr.Mtime.IsZero() || attr.Atime.IsZero() || attr.Ctime.IsZero() { + t.Fatalf("expected non-zero times: atime=%v mtime=%v ctime=%v", attr.Atime, attr.Mtime, attr.Ctime) + } + if !attr.Atime.Equal(attr.Mtime) || !attr.Ctime.Equal(attr.Mtime) { + t.Fatalf("expected .git attrs to use one timestamp: atime=%v mtime=%v ctime=%v", attr.Atime, attr.Mtime, attr.Ctime) + } +} diff --git a/internal/fusefs/merged.go b/internal/fusefs/merged.go index 4b783e0..eb09247 100644 --- a/internal/fusefs/merged.go +++ b/internal/fusefs/merged.go @@ -73,15 +73,16 @@ func (r *Resolver) Lookup(parent, name string) (ResolvedNode, error) { return r.ResolvePath(p) } -func (r *Resolver) Getattr(path string) (mode uint32, size int64, nodeType string, mtime time.Time, err error) { +func (r *Resolver) Getattr(path string) (mode uint32, size int64, nodeType string, mtime time.Time, ctime time.Time, err error) { n, err := r.ResolvePath(path) if err != nil { - return 0, 0, "", time.Time{}, err + return 0, 0, "", time.Time{}, time.Time{}, err } if n.FromOverlay { typ := n.Overlay.NodeType() mt := time.Unix(0, n.Overlay.MtimeUnixNs) - return n.Overlay.Mode, n.Overlay.SizeBytes, typ, mt, nil + ct := time.Unix(0, n.Overlay.CtimeUnixNs) + return n.Overlay.Mode, n.Overlay.SizeBytes, typ, mt, ct, nil } mode = normalizeMode(n.Base.Mode, n.Base.Type) // Base files use the HEAD commit timestamp for mtime so tools like @@ -91,7 +92,7 @@ func (r *Resolver) Getattr(path string) (mode uint32, size int64, nodeType strin ct = r.Generation() // fallback: commit time unavailable } mt := time.Unix(ct, 0) - return mode, n.Base.SizeBytes, n.Base.Type, mt, nil + return mode, n.Base.SizeBytes, n.Base.Type, mt, mt, nil } // normalizeMode ensures sane permission bits. Git tree entries have mode 040000 @@ -141,6 +142,9 @@ func (r *Resolver) ReaddirTyped(ctx context.Context, path string) ([]ReaddirEntr ovEntries, err := r.Overlay.ListByPrefix(ctx, path) if err == nil { for _, e := range ovEntries { + if e.Path == path { + continue + } name, ok := childName(path, e.Path) if !ok { continue diff --git a/internal/fusefs/merged_test.go b/internal/fusefs/merged_test.go index da8d299..fb0b475 100644 --- a/internal/fusefs/merged_test.go +++ b/internal/fusefs/merged_test.go @@ -3,6 +3,7 @@ package fusefs import ( "context" "errors" + iofs "io/fs" "testing" "time" @@ -43,8 +44,14 @@ func (f *fakeOverlay) Get(path string) (model.OverlayEntry, bool) { func (f *fakeOverlay) ListByPrefix(_ context.Context, _ string) ([]model.OverlayEntry, error) { return f.list, nil } -func (f *fakeOverlay) EnsureCopyOnWrite(_ context.Context, _ model.RepoConfig, _ string, _ model.BaseNode) (model.OverlayEntry, error) { - return model.OverlayEntry{}, nil +func (f *fakeOverlay) EnsureCopyOnWrite(_ context.Context, _ model.RepoConfig, path string, base model.BaseNode) (model.OverlayEntry, error) { + if f.entries == nil { + f.entries = map[string]model.OverlayEntry{} + } + now := time.Now().UnixNano() + e := model.OverlayEntry{Path: model.CleanPath(path), Kind: model.OverlayKindModify, Mode: base.Mode, MtimeUnixNs: now, CtimeUnixNs: now, SourceOID: base.ObjectOID} + f.entries[e.Path] = e + return e, nil } func (f *fakeOverlay) CreateFile(_ context.Context, _ string, _ uint32) (model.OverlayEntry, error) { return model.OverlayEntry{}, nil @@ -52,10 +59,24 @@ func (f *fakeOverlay) CreateFile(_ context.Context, _ string, _ uint32) (model.O func (f *fakeOverlay) WriteFile(_ context.Context, _ string, _ int64, _ []byte) (int, error) { return 0, nil } -func (f *fakeOverlay) Remove(_ context.Context, _ string) error { return nil } -func (f *fakeOverlay) Rename(_ context.Context, _, _ string) error { return nil } -func (f *fakeOverlay) Mkdir(_ context.Context, _ string, _ uint32) error { return nil } -func (f *fakeOverlay) SetMtime(_ context.Context, _ string, _ time.Time) error { return nil } +func (f *fakeOverlay) Truncate(_ context.Context, _ string, _ int64) error { return nil } +func (f *fakeOverlay) Remove(_ context.Context, _ string) error { return nil } +func (f *fakeOverlay) Rename(_ context.Context, _, _ string) error { return nil } +func (f *fakeOverlay) Mkdir(_ context.Context, path string, mode uint32) error { + if f.entries == nil { + f.entries = map[string]model.OverlayEntry{} + } + now := time.Now().UnixNano() + f.entries[model.CleanPath(path)] = model.OverlayEntry{Path: model.CleanPath(path), Kind: model.OverlayKindMkdir, Mode: mode, MtimeUnixNs: now, CtimeUnixNs: now} + return nil +} +func (f *fakeOverlay) SetMtime(_ context.Context, path string, t time.Time) error { + e := f.entries[model.CleanPath(path)] + e.MtimeUnixNs = t.UnixNano() + e.CtimeUnixNs = time.Now().UnixNano() + f.entries[model.CleanPath(path)] = e + return nil +} func (f *fakeOverlay) Reconcile(_ context.Context, _ func(string) (model.BaseNode, bool)) error { return nil } @@ -94,19 +115,23 @@ func TestResolveOverlayTakesPrecedence(t *testing.T) { func TestGetattrReturnsMtime(t *testing.T) { mtime := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC).UnixNano() + ctime := time.Date(2025, 1, 2, 0, 0, 0, 0, time.UTC).UnixNano() r := newResolver( - &fakeSnapshot{}, + &fakeSnapshot{kids: map[string][]model.BaseNode{".": {}}}, &fakeOverlay{ - entries: map[string]model.OverlayEntry{"x.txt": {Path: "x.txt", Kind: model.OverlayKindCreate, Mode: 0o644, SizeBytes: 10, MtimeUnixNs: mtime}}, + entries: map[string]model.OverlayEntry{"x.txt": {Path: "x.txt", Kind: model.OverlayKindCreate, Mode: 0o644, SizeBytes: 10, MtimeUnixNs: mtime, CtimeUnixNs: ctime}}, }, ) - _, _, _, mt, err := r.Getattr("x.txt") + _, _, _, mt, ct, err := r.Getattr("x.txt") if err != nil { t.Fatal(err) } if mt.UnixNano() != mtime { t.Fatalf("mtime = %v, want %v", mt, time.Unix(0, mtime)) } + if ct.UnixNano() != ctime { + t.Fatalf("ctime = %v, want %v", ct, time.Unix(0, ctime)) + } } func TestGetattrBaseFileUsesCommitTime(t *testing.T) { @@ -118,7 +143,7 @@ func TestGetattrBaseFileUsesCommitTime(t *testing.T) { commitTS := int64(1700000000) // 2023-11-14 r.SetCommitTime(commitTS) - _, _, _, mt, err := r.Getattr("b.txt") + _, _, _, mt, ct, err := r.Getattr("b.txt") if err != nil { t.Fatal(err) } @@ -126,6 +151,9 @@ func TestGetattrBaseFileUsesCommitTime(t *testing.T) { if !mt.Equal(expected) { t.Fatalf("mtime = %v, want %v", mt, expected) } + if !ct.Equal(expected) { + t.Fatalf("ctime = %v, want %v", ct, expected) + } } func TestGetattrBaseFileFallsBackToGeneration(t *testing.T) { @@ -134,7 +162,7 @@ func TestGetattrBaseFileFallsBackToGeneration(t *testing.T) { &fakeOverlay{entries: map[string]model.OverlayEntry{}}, ) // Don't set commit time -- should fall back to generation. - _, _, _, mt, err := r.Getattr("b.txt") + _, _, _, mt, ct, err := r.Getattr("b.txt") if err != nil { t.Fatal(err) } @@ -142,6 +170,56 @@ func TestGetattrBaseFileFallsBackToGeneration(t *testing.T) { if !mt.Equal(expected) { t.Fatalf("mtime = %v, want %v", mt, expected) } + if !ct.Equal(expected) { + t.Fatalf("ctime = %v, want %v", ct, expected) + } +} + +func TestSetMtimePromotesBaseFileWithSeparateCtime(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{}} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"b.txt": {Path: "b.txt", Type: "file", Mode: 0o644, SizeState: "known", SizeBytes: 5}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + target := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) + + if err := engine.SetMtime(context.Background(), "b.txt", target); err != nil { + t.Fatal(err) + } + got, ok := ov.entries["b.txt"] + if !ok { + t.Fatal("expected base file to be promoted") + } + if got.MtimeUnixNs != target.UnixNano() { + t.Fatalf("mtime = %v, want %v", time.Unix(0, got.MtimeUnixNs), target) + } + if got.CtimeUnixNs == target.UnixNano() || got.CtimeUnixNs == 0 { + t.Fatalf("ctime should be non-zero and independent from caller mtime: %v", time.Unix(0, got.CtimeUnixNs)) + } +} + +func TestSetMtimeRejectsRootAndBaseSymlink(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{}} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{ + ".": {Path: ".", Type: "dir", Mode: 0o755}, + "link": {Path: "link", Type: "symlink", Mode: 0o120000}, + }}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + target := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) + + if err := engine.SetMtime(context.Background(), ".", target); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("root SetMtime err = %v, want ErrInvalid", err) + } + if _, ok := ov.entries["."]; ok { + t.Fatal("root SetMtime should not create an overlay entry") + } + if err := engine.SetMtime(context.Background(), "link", target); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("symlink SetMtime err = %v, want ErrInvalid", err) + } } func TestReaddirMergesSnapshotAndOverlay(t *testing.T) { @@ -172,6 +250,26 @@ func TestReaddirMergesSnapshotAndOverlay(t *testing.T) { } } +func TestReaddirSkipsOverlayEntryForListedDirectory(t *testing.T) { + r := newResolver( + &fakeSnapshot{kids: map[string][]model.BaseNode{".": {}}}, + &fakeOverlay{ + entries: map[string]model.OverlayEntry{".": {Path: ".", Kind: model.OverlayKindMkdir}}, + list: []model.OverlayEntry{{Path: ".", Kind: model.OverlayKindMkdir}}, + }, + ) + + names, err := r.Readdir(context.Background(), ".") + if err != nil { + t.Fatal(err) + } + for _, name := range names { + if name == "." { + t.Fatal("root overlay entry should not appear as a child") + } + } +} + func TestReaddirWhiteoutRemovesEntry(t *testing.T) { r := newResolver( &fakeSnapshot{ diff --git a/internal/fusefs/ops.go b/internal/fusefs/ops.go index 580cde8..e0c9425 100644 --- a/internal/fusefs/ops.go +++ b/internal/fusefs/ops.go @@ -112,21 +112,39 @@ func (e *Engine) Rmdir(ctx context.Context, path string) error { return e.Overlay.Remove(ctx, path) } -// SetMtime updates the mtime for a path in the overlay. No-op for base files -// not yet promoted to the overlay (their mtime comes from the generation epoch). -func (e *Engine) SetMtime(ctx context.Context, path string, t time.Time) { - _ = e.Overlay.SetMtime(ctx, path, t) +// SetMtime promotes base files/directories before updating mtime so the +// caller-controlled timestamp never overwrites base snapshot attrs. +func (e *Engine) SetMtime(ctx context.Context, path string, t time.Time) error { + path = model.CleanPath(path) + if path == "." { + return fs.ErrInvalid + } + if _, ok := e.Overlay.Get(path); !ok { + n, err := e.Resolver.ResolvePath(path) + if err != nil { + return err + } + switch n.Base.Type { + case "dir": + if err := e.Overlay.Mkdir(ctx, path, n.Base.Mode); err != nil { + return err + } + case "file": + if err := e.ensureOverlay(ctx, path); err != nil { + return err + } + default: + return fs.ErrInvalid + } + } + return e.Overlay.SetMtime(ctx, path, t) } func (e *Engine) Truncate(ctx context.Context, path string, size int64) error { if err := e.ensureOverlay(ctx, path); err != nil { return err } - ov, ok := e.Overlay.Get(path) - if !ok { - return os.ErrNotExist - } - return os.Truncate(ov.BackingPath, size) + return e.Overlay.Truncate(ctx, path, size) } // PrefetchDir enqueues file children of a directory for speculative hydration. diff --git a/internal/model/types.go b/internal/model/types.go index 9e7372d..eddbedc 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -74,6 +74,7 @@ type OverlayEntry struct { Mode uint32 SizeBytes int64 MtimeUnixNs int64 + CtimeUnixNs int64 SourceOID string TargetPath string } @@ -159,6 +160,7 @@ type OverlayStore interface { EnsureCopyOnWrite(ctx context.Context, repo RepoConfig, path string, base BaseNode) (OverlayEntry, error) CreateFile(ctx context.Context, path string, mode uint32) (OverlayEntry, error) WriteFile(ctx context.Context, path string, off int64, data []byte) (int, error) + Truncate(ctx context.Context, path string, size int64) error Remove(ctx context.Context, path string) error Rename(ctx context.Context, oldPath, newPath string) error Mkdir(ctx context.Context, path string, mode uint32) error diff --git a/internal/overlay/store.go b/internal/overlay/store.go index 6c91e23..26b6430 100644 --- a/internal/overlay/store.go +++ b/internal/overlay/store.go @@ -23,6 +23,7 @@ var migrations = []string{ mode INTEGER NOT NULL, size_bytes INTEGER NOT NULL DEFAULT 0, mtime_unix_ns INTEGER NOT NULL, + ctime_unix_ns INTEGER NOT NULL, source_oid TEXT, target_path TEXT );`, @@ -43,6 +44,9 @@ func New(ctx context.Context, cfg model.RepoConfig) (*Store, error) { if err := meta.ExecMigrations(ctx, db, migrations); err != nil { return nil, err } + if err := ensureOverlaySchema(ctx, db); err != nil { + return nil, err + } upperDir := filepath.Join(cfg.OverlayDir, "upper") if err := os.MkdirAll(upperDir, 0o755); err != nil { return nil, err @@ -50,16 +54,48 @@ func New(ctx context.Context, cfg model.RepoConfig) (*Store, error) { return &Store{db: db, repo: cfg, upperDir: upperDir}, nil } +func ensureOverlaySchema(ctx context.Context, db *sql.DB) error { + rows, err := db.QueryContext(ctx, `PRAGMA table_info(overlay_entries)`) + if err != nil { + return err + } + defer rows.Close() + hasCtime := false + for rows.Next() { + var cid int + var name, typ string + var notNull int + var defaultValue any + var pk int + if err := rows.Scan(&cid, &name, &typ, ¬Null, &defaultValue, &pk); err != nil { + return err + } + if name == "ctime_unix_ns" { + hasCtime = true + } + } + if err := rows.Err(); err != nil { + return err + } + if !hasCtime { + if _, err := db.ExecContext(ctx, `ALTER TABLE overlay_entries ADD COLUMN ctime_unix_ns INTEGER NOT NULL DEFAULT 0`); err != nil { + return err + } + } + _, err = db.ExecContext(ctx, `UPDATE overlay_entries SET ctime_unix_ns=? WHERE ctime_unix_ns=0`, time.Now().UnixNano()) + return err +} + func (s *Store) Close() error { return s.db.Close() } // overlayCols is the column list for overlay_entries queries. Keep in sync with // the Scan call in queryEntries and the single-row scan in Get. -const overlayCols = `path, kind, backing_path, mode, size_bytes, mtime_unix_ns, source_oid, target_path` +const overlayCols = `path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path` func (s *Store) Get(path string) (model.OverlayEntry, bool) { row := s.db.QueryRow(`SELECT `+overlayCols+` FROM overlay_entries WHERE path=?`, model.CleanPath(path)) var e model.OverlayEntry - if err := row.Scan(&e.Path, &e.Kind, &e.BackingPath, &e.Mode, &e.SizeBytes, &e.MtimeUnixNs, &e.SourceOID, &e.TargetPath); err != nil { + if err := row.Scan(&e.Path, &e.Kind, &e.BackingPath, &e.Mode, &e.SizeBytes, &e.MtimeUnixNs, &e.CtimeUnixNs, &e.SourceOID, &e.TargetPath); err != nil { return model.OverlayEntry{}, false } e.RepoID = s.repo.ID @@ -96,6 +132,7 @@ func (s *Store) EnsureCopyOnWrite(ctx context.Context, repo model.RepoConfig, pa if err != nil { return model.OverlayEntry{}, err } + now := time.Now().UnixNano() e := model.OverlayEntry{ RepoID: s.repo.ID, Path: model.CleanPath(path), @@ -103,7 +140,8 @@ func (s *Store) EnsureCopyOnWrite(ctx context.Context, repo model.RepoConfig, pa BackingPath: backing, Mode: base.Mode, SizeBytes: st.Size(), - MtimeUnixNs: time.Now().UnixNano(), + MtimeUnixNs: now, + CtimeUnixNs: now, SourceOID: base.ObjectOID, } if err := s.upsertEntry(ctx, e); err != nil { @@ -120,7 +158,8 @@ func (s *Store) CreateFile(ctx context.Context, path string, mode uint32) (model if err := os.WriteFile(backing, nil, os.FileMode(mode)); err != nil { return model.OverlayEntry{}, err } - e := model.OverlayEntry{RepoID: s.repo.ID, Path: model.CleanPath(path), Kind: model.OverlayKindCreate, BackingPath: backing, Mode: mode, MtimeUnixNs: time.Now().UnixNano()} + now := time.Now().UnixNano() + e := model.OverlayEntry{RepoID: s.repo.ID, Path: model.CleanPath(path), Kind: model.OverlayKindCreate, BackingPath: backing, Mode: mode, MtimeUnixNs: now, CtimeUnixNs: now} if err := s.upsertEntry(ctx, e); err != nil { return model.OverlayEntry{}, err } @@ -142,20 +181,41 @@ func (s *Store) WriteFile(ctx context.Context, path string, off int64, data []by return n, err } st, _ := f.Stat() + now := time.Now().UnixNano() e.SizeBytes = st.Size() - e.MtimeUnixNs = time.Now().UnixNano() + e.MtimeUnixNs = now + e.CtimeUnixNs = now if e.Kind != model.OverlayKindCreate { e.Kind = model.OverlayKindModify } return n, s.upsertEntry(ctx, e) } +func (s *Store) Truncate(ctx context.Context, path string, size int64) error { + e, ok := s.Get(path) + if !ok || e.IsDeleted() { + return os.ErrNotExist + } + if err := os.Truncate(e.BackingPath, size); err != nil { + return err + } + now := time.Now().UnixNano() + e.SizeBytes = size + e.MtimeUnixNs = now + e.CtimeUnixNs = now + if e.Kind != model.OverlayKindCreate { + e.Kind = model.OverlayKindModify + } + return s.upsertEntry(ctx, e) +} + func (s *Store) Remove(ctx context.Context, path string) error { path = model.CleanPath(path) if e, ok := s.Get(path); ok && e.BackingPath != "" { _ = os.Remove(e.BackingPath) } - e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindDelete, Mode: 0, MtimeUnixNs: time.Now().UnixNano()} + now := time.Now().UnixNano() + e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindDelete, Mode: 0, MtimeUnixNs: now, CtimeUnixNs: now} return s.upsertEntry(ctx, e) } @@ -181,10 +241,10 @@ func (s *Store) Rename(ctx context.Context, oldPath, newPath string) error { if _, err := tx.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, oldPath); err != nil { return err } - if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?)`, newPath, model.OverlayKindRename, newBacking, e.Mode, e.SizeBytes, now, e.SourceOID, newPath); err != nil { + if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, newPath, model.OverlayKindRename, newBacking, e.Mode, e.SizeBytes, now, now, e.SourceOID, newPath); err != nil { return err } - if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?) ON CONFLICT(path) DO UPDATE SET kind='delete',mtime_unix_ns=excluded.mtime_unix_ns`, oldPath, model.OverlayKindDelete, "", 0, 0, now, "", ""); err != nil { + if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?) ON CONFLICT(path) DO UPDATE SET kind='delete',mtime_unix_ns=excluded.mtime_unix_ns,ctime_unix_ns=excluded.ctime_unix_ns`, oldPath, model.OverlayKindDelete, "", 0, 0, now, now, "", ""); err != nil { return err } if err := tx.Commit(); err != nil { @@ -197,7 +257,7 @@ func (s *Store) Rename(ctx context.Context, oldPath, newPath string) error { // state. If this also fails, the overlay is inconsistent -- logged // upstream by the daemon. s.db.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, newPath) - s.db.ExecContext(ctx, `INSERT OR REPLACE INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?)`, oldPath, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.SourceOID, e.TargetPath) + s.db.ExecContext(ctx, `INSERT OR REPLACE INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, oldPath, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.CtimeUnixNs, e.SourceOID, e.TargetPath) return err } } @@ -209,15 +269,16 @@ func (s *Store) Mkdir(ctx context.Context, path string, mode uint32) error { if err := os.MkdirAll(s.backingPath(path), os.FileMode(mode)); err != nil { return err } - e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindMkdir, BackingPath: s.backingPath(path), Mode: mode, MtimeUnixNs: time.Now().UnixNano()} + now := time.Now().UnixNano() + e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindMkdir, BackingPath: s.backingPath(path), Mode: mode, MtimeUnixNs: now, CtimeUnixNs: now} return s.upsertEntry(ctx, e) } func (s *Store) SetMtime(ctx context.Context, path string, t time.Time) error { path = model.CleanPath(path) _, err := s.db.ExecContext(ctx, - `UPDATE overlay_entries SET mtime_unix_ns=? WHERE path=?`, - t.UnixNano(), path) + `UPDATE overlay_entries SET mtime_unix_ns=?, ctime_unix_ns=? WHERE path=?`, + t.UnixNano(), time.Now().UnixNano(), path) return err } @@ -333,7 +394,7 @@ func (s *Store) queryEntries(ctx context.Context, query string, args ...any) ([] var out []model.OverlayEntry for rows.Next() { var e model.OverlayEntry - if err := rows.Scan(&e.Path, &e.Kind, &e.BackingPath, &e.Mode, &e.SizeBytes, &e.MtimeUnixNs, &e.SourceOID, &e.TargetPath); err != nil { + if err := rows.Scan(&e.Path, &e.Kind, &e.BackingPath, &e.Mode, &e.SizeBytes, &e.MtimeUnixNs, &e.CtimeUnixNs, &e.SourceOID, &e.TargetPath); err != nil { return nil, err } e.RepoID = s.repo.ID @@ -347,16 +408,17 @@ func (s *Store) upsertEntry(ctx context.Context, e model.OverlayEntry) error { return errors.New("empty path") } _, err := s.db.ExecContext(ctx, ` - INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, source_oid, target_path) - VALUES(?,?,?,?,?,?,?,?) + INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) + VALUES(?,?,?,?,?,?,?,?,?) ON CONFLICT(path) DO UPDATE SET kind=excluded.kind, backing_path=excluded.backing_path, mode=excluded.mode, size_bytes=excluded.size_bytes, mtime_unix_ns=excluded.mtime_unix_ns, + ctime_unix_ns=excluded.ctime_unix_ns, source_oid=excluded.source_oid, - target_path=excluded.target_path`, e.Path, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.SourceOID, e.TargetPath) + target_path=excluded.target_path`, e.Path, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.CtimeUnixNs, e.SourceOID, e.TargetPath) return err } diff --git a/internal/overlay/store_test.go b/internal/overlay/store_test.go index f835c13..dfe3e5a 100644 --- a/internal/overlay/store_test.go +++ b/internal/overlay/store_test.go @@ -50,11 +50,37 @@ func TestCreateAndGet(t *testing.T) { } } +func TestNewRepairsZeroCtimeBackfill(t *testing.T) { + s, cfg := testStore(t) + ctx := context.Background() + + if _, err := s.CreateFile(ctx, "old.txt", 0o644); err != nil { + t.Fatal(err) + } + if _, err := s.db.ExecContext(ctx, `UPDATE overlay_entries SET ctime_unix_ns=0 WHERE path=?`, "old.txt"); err != nil { + t.Fatal(err) + } + + reopened, err := New(ctx, cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { reopened.Close() }) + got, ok := reopened.Get("old.txt") + if !ok { + t.Fatal("expected entry") + } + if got.CtimeUnixNs == 0 { + t.Fatal("expected zero ctime to be repaired") + } +} + func TestWriteAndRead(t *testing.T) { s, _ := testStore(t) ctx := context.Background() s.CreateFile(ctx, "f.txt", 0o644) + created, _ := s.Get("f.txt") n, err := s.WriteFile(ctx, "f.txt", 0, []byte("hello")) if err != nil { t.Fatal(err) @@ -71,6 +97,12 @@ func TestWriteAndRead(t *testing.T) { if string(data) != "hello" { t.Fatalf("got %q, want %q", data, "hello") } + if e.CtimeUnixNs < created.CtimeUnixNs { + t.Fatalf("ctime moved backward: got %d before %d", e.CtimeUnixNs, created.CtimeUnixNs) + } + if e.MtimeUnixNs != e.CtimeUnixNs { + t.Fatalf("write should set mtime and ctime together: mtime=%d ctime=%d", e.MtimeUnixNs, e.CtimeUnixNs) + } } func TestRemoveCreatesWhiteout(t *testing.T) { @@ -167,6 +199,39 @@ func TestMkdir(t *testing.T) { } } +func TestTruncateUpdatesSizeAndTimes(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if _, err := s.CreateFile(ctx, "trunc.txt", 0o644); err != nil { + t.Fatal(err) + } + if _, err := s.WriteFile(ctx, "trunc.txt", 0, []byte("hello")); err != nil { + t.Fatal(err) + } + target := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) + if err := s.SetMtime(ctx, "trunc.txt", target); err != nil { + t.Fatal(err) + } + if err := s.Truncate(ctx, "trunc.txt", 2); err != nil { + t.Fatal(err) + } + + e, ok := s.Get("trunc.txt") + if !ok { + t.Fatal("expected entry") + } + if e.SizeBytes != 2 { + t.Fatalf("size = %d, want 2", e.SizeBytes) + } + if e.MtimeUnixNs == target.UnixNano() || e.CtimeUnixNs == target.UnixNano() { + t.Fatalf("truncate should replace user mtime with mutation time: mtime=%d ctime=%d", e.MtimeUnixNs, e.CtimeUnixNs) + } + if e.MtimeUnixNs != e.CtimeUnixNs { + t.Fatalf("truncate should set mtime and ctime together: mtime=%d ctime=%d", e.MtimeUnixNs, e.CtimeUnixNs) + } +} + func TestDirtyCount(t *testing.T) { s, _ := testStore(t) ctx := context.Background() @@ -336,4 +401,10 @@ func TestSetMtime(t *testing.T) { if !got.Equal(target) { t.Fatalf("mtime = %v, want %v", got, target) } + if e.CtimeUnixNs == target.UnixNano() { + t.Fatalf("ctime should not be caller-controlled: ctime=%v target=%v", time.Unix(0, e.CtimeUnixNs), target) + } + if e.CtimeUnixNs == 0 { + t.Fatal("ctime should be non-zero") + } } From 760fc2ee5ecb7f4efbecdb778c54dec85d3e4c20 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Mon, 8 Jun 2026 13:11:28 -0400 Subject: [PATCH 2/2] fix overlay rename stat edge cases --- internal/fusefs/fuse_unix.go | 10 +- internal/fusefs/fuse_unix_test.go | 12 + internal/fusefs/merged_test.go | 177 ++++++++++++- internal/fusefs/ops.go | 46 +++- internal/model/types.go | 1 + internal/overlay/store.go | 213 +++++++++++++-- internal/overlay/store_test.go | 414 +++++++++++++++++++++++++++++- 7 files changed, 832 insertions(+), 41 deletions(-) diff --git a/internal/fusefs/fuse_unix.go b/internal/fusefs/fuse_unix.go index 10ed8e5..ef1b633 100644 --- a/internal/fusefs/fuse_unix.go +++ b/internal/fusefs/fuse_unix.go @@ -467,6 +467,9 @@ func (fs *ArtifactFuse) Rename(ctx context.Context, op *fuseops.RenameOp) error oldPath := cleanChildPath(oldParent.Path, op.OldName) newPath := cleanChildPath(newParent.Path, op.NewName) if err := fs.engine.Rename(ctx, oldPath, newPath); err != nil { + if errors.Is(err, iofs.ErrInvalid) { + return syscall.ENOTSUP + } return syscall.EIO } return nil @@ -592,13 +595,6 @@ func TryUnmount(mountPoint string) error { func inodeAttrs(mode uint32, size uint64, typ string, mtime time.Time, ctime time.Time) fuseops.InodeAttributes { m := os.FileMode(mode & 0o777) - if m == 0 { - if typ == "dir" { - m = 0o755 - } else { - m = 0o644 - } - } switch typ { case "dir": m |= os.ModeDir diff --git a/internal/fusefs/fuse_unix_test.go b/internal/fusefs/fuse_unix_test.go index ba1cdf5..4df7438 100644 --- a/internal/fusefs/fuse_unix_test.go +++ b/internal/fusefs/fuse_unix_test.go @@ -23,6 +23,18 @@ func TestInodeAttrsPreservesSeparateTimes(t *testing.T) { } } +func TestInodeAttrsPreservesExplicitZeroDirMode(t *testing.T) { + now := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC) + + attr := inodeAttrs(0, 4096, "dir", now, now) + if attr.Mode.Perm() != 0 { + t.Fatalf("mode perms = %#o, want 0", attr.Mode.Perm()) + } + if !attr.Mode.IsDir() { + t.Fatalf("expected directory mode, got %#o", attr.Mode) + } +} + func TestGitFileAttrsUsesOneTimestamp(t *testing.T) { fs := &ArtifactFuse{gitfileContent: []byte("gitdir: /tmp/repo/.git\n")} diff --git a/internal/fusefs/merged_test.go b/internal/fusefs/merged_test.go index fb0b475..490b60e 100644 --- a/internal/fusefs/merged_test.go +++ b/internal/fusefs/merged_test.go @@ -61,7 +61,25 @@ func (f *fakeOverlay) WriteFile(_ context.Context, _ string, _ int64, _ []byte) } func (f *fakeOverlay) Truncate(_ context.Context, _ string, _ int64) error { return nil } func (f *fakeOverlay) Remove(_ context.Context, _ string) error { return nil } -func (f *fakeOverlay) Rename(_ context.Context, _, _ string) error { return nil } +func (f *fakeOverlay) Rename(_ context.Context, oldPath, newPath string) error { + oldPath = model.CleanPath(oldPath) + newPath = model.CleanPath(newPath) + e := f.entries[oldPath] + delete(f.entries, oldPath) + e.Path = newPath + f.entries[newPath] = e + return nil +} +func (f *fakeOverlay) RenameAndMarkModifiedFromBase(_ context.Context, oldPath, newPath string, sourceOID string) error { + if err := f.Rename(context.Background(), oldPath, newPath); err != nil { + return err + } + e := f.entries[model.CleanPath(newPath)] + e.Kind = model.OverlayKindModify + e.SourceOID = sourceOID + f.entries[model.CleanPath(newPath)] = e + return nil +} func (f *fakeOverlay) Mkdir(_ context.Context, path string, mode uint32) error { if f.entries == nil { f.entries = map[string]model.OverlayEntry{} @@ -222,6 +240,163 @@ func TestSetMtimeRejectsRootAndBaseSymlink(t *testing.T) { } } +func TestRenameRejectsBaseDirectory(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{}} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"src": {Path: "src", Type: "dir", Mode: 0o40000}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "src", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename base dir err = %v, want ErrInvalid", err) + } + if _, ok := ov.entries["src"]; ok { + t.Fatal("base directory rename should not create source overlay entry") + } + if _, ok := ov.entries["dst"]; ok { + t.Fatal("base directory rename should not create destination overlay entry") + } +} + +func TestRenameBaseFileRejectsBaseDirectoryDestination(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{}} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{ + "a.txt": {Path: "a.txt", Type: "file", Mode: 0o644}, + "dst": {Path: "dst", Type: "dir", Mode: 0o40000}, + }}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "a.txt", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename base file over base dir err = %v, want ErrInvalid", err) + } + if _, ok := ov.entries["a.txt"]; ok { + t.Fatal("invalid rename should not promote source into overlay") + } +} + +func TestRenameAllowsOverlayFileShadowingBaseDirectory(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "src": {Path: "src", Kind: model.OverlayKindCreate, Mode: 0o644}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"src": {Path: "src", Type: "dir", Mode: 0o40000}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "src", "dst"); err != nil { + t.Fatalf("Rename overlay file shadowing base dir: %v", err) + } +} + +func TestRenameCreateOverBaseBecomesModify(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "tmp.txt": {Path: "tmp.txt", Kind: model.OverlayKindCreate, Mode: 0o644}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"dst.txt": {Path: "dst.txt", Type: "file", Mode: 0o644, ObjectOID: "dst-oid"}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "tmp.txt", "dst.txt"); err != nil { + t.Fatalf("Rename create over base: %v", err) + } + got, ok := ov.entries["dst.txt"] + if !ok { + t.Fatal("expected destination overlay entry") + } + if got.Kind != model.OverlayKindModify || got.SourceOID != "dst-oid" { + t.Fatalf("destination entry = %+v, want modify from dst-oid", got) + } +} + +func TestRenameModifyRejectsBaseDirectoryDestination(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "src.txt": {Path: "src.txt", Kind: model.OverlayKindModify, Mode: 0o644, SourceOID: "src-oid"}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"dst": {Path: "dst", Type: "dir", Mode: 0o40000}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "src.txt", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename modify over base dir err = %v, want ErrInvalid", err) + } +} + +func TestRenameRejectsOverlayMkdirShadowingBaseDirectory(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "src": {Path: "src", Kind: model.OverlayKindMkdir, Mode: 0o755}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"src": {Path: "src", Type: "dir", Mode: 0o40000}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "src", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename overlay mkdir shadowing base dir err = %v, want ErrInvalid", err) + } +} + +func TestRenameRejectsOverlayMkdirShadowingBaseFile(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "src": {Path: "src", Kind: model.OverlayKindMkdir, Mode: 0o755}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"src": {Path: "src", Type: "file", Mode: 0o644}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "src", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename overlay mkdir shadowing base file err = %v, want ErrInvalid", err) + } +} + +func TestRenameOverlayMkdirRejectsBaseFileDestination(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{ + "tmpdir": {Path: "tmpdir", Kind: model.OverlayKindMkdir, Mode: 0o755}, + }} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{"dst.txt": {Path: "dst.txt", Type: "file", Mode: 0o644}}}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "tmpdir", "dst.txt"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("Rename overlay mkdir over base file err = %v, want ErrInvalid", err) + } +} + +func TestRenameSameBasePathNoop(t *testing.T) { + ov := &fakeOverlay{entries: map[string]model.OverlayEntry{}} + r := newResolver( + &fakeSnapshot{nodes: map[string]model.BaseNode{ + "a.txt": {Path: "a.txt", Type: "file", Mode: 0o644}, + "src": {Path: "src", Type: "dir", Mode: 0o40000}, + }}, + ov, + ) + engine := &Engine{Resolver: r, Overlay: ov} + + if err := engine.Rename(context.Background(), "a.txt", "a.txt"); err != nil { + t.Fatalf("Rename same file path: %v", err) + } + if err := engine.Rename(context.Background(), "src", "src"); err != nil { + t.Fatalf("Rename same dir path: %v", err) + } + if len(ov.entries) != 0 { + t.Fatalf("same-path rename should not mutate overlay: %+v", ov.entries) + } +} + func TestReaddirMergesSnapshotAndOverlay(t *testing.T) { r := newResolver( &fakeSnapshot{ diff --git a/internal/fusefs/ops.go b/internal/fusefs/ops.go index e0c9425..f68af19 100644 --- a/internal/fusefs/ops.go +++ b/internal/fusefs/ops.go @@ -80,18 +80,46 @@ func (e *Engine) Unlink(ctx context.Context, path string) error { } func (e *Engine) Rename(ctx context.Context, oldPath, newPath string) error { - if _, ok := e.Overlay.Get(oldPath); !ok { - n, err := e.Resolver.ResolvePath(oldPath) - if err != nil { - return err + oldPath = model.CleanPath(oldPath) + newPath = model.CleanPath(newPath) + if oldPath == newPath { + _, err := e.Resolver.ResolvePath(oldPath) + return err + } + if ov, ok := e.Overlay.Get(oldPath); ok { + if ov.IsDeleted() { + return os.ErrNotExist } - if n.Base.Type == "dir" { - if err := e.Overlay.Mkdir(ctx, oldPath, n.Base.Mode); err != nil { - return err + if ov.Kind == model.OverlayKindMkdir { + if _, ok := e.Resolver.Snapshot.GetNode(e.Resolver.Generation(), oldPath); ok { + return fs.ErrInvalid } - } else if err := e.ensureOverlay(ctx, oldPath); err != nil { - return err } + if dst, ok := e.Resolver.Snapshot.GetNode(e.Resolver.Generation(), newPath); ok { + if dst.Type == "dir" || ov.Kind == model.OverlayKindMkdir { + return fs.ErrInvalid + } + if ov.Kind == model.OverlayKindCreate || ov.Kind == model.OverlayKindSymlink { + return e.Overlay.RenameAndMarkModifiedFromBase(ctx, oldPath, newPath, dst.ObjectOID) + } + } + return e.Overlay.Rename(ctx, oldPath, newPath) + } + if n, ok := e.Resolver.Snapshot.GetNode(e.Resolver.Generation(), oldPath); ok && n.Type == "dir" { + return fs.ErrInvalid + } + n, err := e.Resolver.ResolvePath(oldPath) + if err != nil { + return err + } + if n.Base.Type == "dir" { + return fs.ErrInvalid + } + if dst, ok := e.Resolver.Snapshot.GetNode(e.Resolver.Generation(), newPath); ok && dst.Type == "dir" { + return fs.ErrInvalid + } + if err := e.ensureOverlay(ctx, oldPath); err != nil { + return err } return e.Overlay.Rename(ctx, oldPath, newPath) } diff --git a/internal/model/types.go b/internal/model/types.go index 3e04f7b..7897453 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -167,6 +167,7 @@ type OverlayStore interface { Truncate(ctx context.Context, path string, size int64) error Remove(ctx context.Context, path string) error Rename(ctx context.Context, oldPath, newPath string) error + RenameAndMarkModifiedFromBase(ctx context.Context, oldPath, newPath string, sourceOID string) error Mkdir(ctx context.Context, path string, mode uint32) error SetMtime(ctx context.Context, path string, t time.Time) error Reconcile(ctx context.Context, baseLookup func(path string) (BaseNode, bool)) error diff --git a/internal/overlay/store.go b/internal/overlay/store.go index 26b6430..75c014b 100644 --- a/internal/overlay/store.go +++ b/internal/overlay/store.go @@ -6,9 +6,11 @@ import ( "errors" "fmt" "io" + iofs "io/fs" "os" "path/filepath" "slices" + "strings" "time" "github.com/cloudflare/artifact-fs/internal/meta" @@ -226,10 +228,47 @@ func (s *Store) Rename(ctx context.Context, oldPath, newPath string) error { if !ok || e.IsDeleted() { return os.ErrNotExist } + if oldPath == newPath { + return nil + } newBacking := s.backingPath(newPath) if err := os.MkdirAll(filepath.Dir(newBacking), 0o755); err != nil { return err } + newKind := model.OverlayKindRename + newTargetPath := oldPath + writeSourceWhiteout := true + if e.Kind == model.OverlayKindRename && e.TargetPath != "" { + newTargetPath = e.TargetPath + } + mode := e.Mode + normalizedDirMode := false + var deletedDescendants []model.OverlayEntry + switch e.Kind { + case model.OverlayKindCreate, model.OverlayKindSymlink: + newKind = e.Kind + newTargetPath = "" + writeSourceWhiteout = false + case model.OverlayKindMkdir: + hasDescendants, err := s.hasOverlayDescendants(ctx, oldPath) + if err != nil { + return err + } + if hasDescendants { + return iofs.ErrInvalid + } + deletedDescendants, err = s.deletedDescendants(ctx, oldPath) + if err != nil { + return err + } + newKind = model.OverlayKindMkdir + newTargetPath = "" + writeSourceWhiteout = false + if normalizedMode, normalized := normalizeGitDirMode(mode); normalized { + mode = normalizedMode + normalizedDirMode = true + } + } // DB transaction first, then filesystem rename. If the DB commit fails, // nothing has moved on disk and the overlay remains consistent. now := time.Now().UnixNano() @@ -241,21 +280,87 @@ func (s *Store) Rename(ctx context.Context, oldPath, newPath string) error { if _, err := tx.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, oldPath); err != nil { return err } - if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, newPath, model.OverlayKindRename, newBacking, e.Mode, e.SizeBytes, now, now, e.SourceOID, newPath); err != nil { + if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, newPath, newKind, newBacking, mode, e.SizeBytes, e.MtimeUnixNs, now, e.SourceOID, newTargetPath); err != nil { return err } - if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?) ON CONFLICT(path) DO UPDATE SET kind='delete',mtime_unix_ns=excluded.mtime_unix_ns,ctime_unix_ns=excluded.ctime_unix_ns`, oldPath, model.OverlayKindDelete, "", 0, 0, now, now, "", ""); err != nil { - return err + if e.Kind == model.OverlayKindMkdir { + prefix := oldPath + "/" + if _, err := tx.ExecContext(ctx, `DELETE FROM overlay_entries WHERE kind=? AND substr(path, 1, length(?))=?`, model.OverlayKindDelete, prefix, prefix); err != nil { + return err + } + } + if writeSourceWhiteout { + if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?) ON CONFLICT(path) DO UPDATE SET kind='delete',mtime_unix_ns=excluded.mtime_unix_ns,ctime_unix_ns=excluded.ctime_unix_ns,target_path=excluded.target_path`, oldPath, model.OverlayKindDelete, "", 0, 0, now, now, "", newTargetPath); err != nil { + return err + } } if err := tx.Commit(); err != nil { return err } // Filesystem rename after successful commit. if e.BackingPath != "" { + restoreDB := func() { + s.db.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, newPath) + s.db.ExecContext(ctx, `INSERT OR REPLACE INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, oldPath, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.CtimeUnixNs, e.SourceOID, e.TargetPath) + for _, d := range deletedDescendants { + s.db.ExecContext(ctx, `INSERT OR REPLACE INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, d.Path, d.Kind, d.BackingPath, d.Mode, d.SizeBytes, d.MtimeUnixNs, d.CtimeUnixNs, d.SourceOID, d.TargetPath) + } + } if err := os.Rename(e.BackingPath, newBacking); err != nil { // DB is committed but file didn't move. Attempt to roll back the DB // state. If this also fails, the overlay is inconsistent -- logged // upstream by the daemon. + restoreDB() + return err + } + if normalizedDirMode { + if err := os.Chmod(newBacking, os.FileMode(mode)); err != nil { + restoreDB() + _ = os.Rename(newBacking, e.BackingPath) + return err + } + } + } + return nil +} + +func (s *Store) deletedDescendants(ctx context.Context, path string) ([]model.OverlayEntry, error) { + prefix := model.CleanPath(path) + "/" + return s.queryEntries(ctx, `SELECT `+overlayCols+` FROM overlay_entries WHERE kind=? AND substr(path, 1, length(?))=? ORDER BY path`, model.OverlayKindDelete, prefix, prefix) +} + +func (s *Store) RenameAndMarkModifiedFromBase(ctx context.Context, oldPath, newPath string, sourceOID string) error { + oldPath = model.CleanPath(oldPath) + newPath = model.CleanPath(newPath) + e, ok := s.Get(oldPath) + if !ok || e.IsDeleted() { + return os.ErrNotExist + } + if oldPath == newPath { + _, err := s.db.ExecContext(ctx, `UPDATE overlay_entries SET kind=?, source_oid=?, target_path='' WHERE path=?`, model.OverlayKindModify, sourceOID, oldPath) + return err + } + newBacking := s.backingPath(newPath) + if err := os.MkdirAll(filepath.Dir(newBacking), 0o755); err != nil { + return err + } + now := time.Now().UnixNano() + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + if _, err := tx.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, oldPath); err != nil { + return err + } + if _, err := tx.ExecContext(ctx, `INSERT INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, newPath, model.OverlayKindModify, newBacking, e.Mode, e.SizeBytes, e.MtimeUnixNs, now, sourceOID, ""); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return err + } + if e.BackingPath != "" { + if err := os.Rename(e.BackingPath, newBacking); err != nil { s.db.ExecContext(ctx, `DELETE FROM overlay_entries WHERE path=?`, newPath) s.db.ExecContext(ctx, `INSERT OR REPLACE INTO overlay_entries(path, kind, backing_path, mode, size_bytes, mtime_unix_ns, ctime_unix_ns, source_oid, target_path) VALUES(?,?,?,?,?,?,?,?,?)`, oldPath, e.Kind, e.BackingPath, e.Mode, e.SizeBytes, e.MtimeUnixNs, e.CtimeUnixNs, e.SourceOID, e.TargetPath) return err @@ -264,18 +369,62 @@ func (s *Store) Rename(ctx context.Context, oldPath, newPath string) error { return nil } +func (s *Store) hasOverlayDescendants(ctx context.Context, path string) (bool, error) { + rows, err := s.db.QueryContext(ctx, `SELECT path FROM overlay_entries WHERE kind <> ?`, model.OverlayKindDelete) + if err != nil { + return false, err + } + defer rows.Close() + prefix := path + "/" + for rows.Next() { + var candidate string + if err := rows.Scan(&candidate); err != nil { + return false, err + } + if strings.HasPrefix(candidate, prefix) { + return true, nil + } + } + return false, rows.Err() +} + func (s *Store) Mkdir(ctx context.Context, path string, mode uint32) error { path = model.CleanPath(path) - if err := os.MkdirAll(s.backingPath(path), os.FileMode(mode)); err != nil { + mode, normalized := normalizeGitDirMode(mode) + backing := s.backingPath(path) + if err := os.MkdirAll(backing, os.FileMode(mode)); err != nil { return err } + if normalized { + if err := os.Chmod(backing, os.FileMode(mode)); err != nil { + return err + } + } now := time.Now().UnixNano() - e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindMkdir, BackingPath: s.backingPath(path), Mode: mode, MtimeUnixNs: now, CtimeUnixNs: now} + e := model.OverlayEntry{RepoID: s.repo.ID, Path: path, Kind: model.OverlayKindMkdir, BackingPath: backing, Mode: mode, MtimeUnixNs: now, CtimeUnixNs: now} return s.upsertEntry(ctx, e) } +func normalizeGitDirMode(mode uint32) (uint32, bool) { + if mode&0o170000 == 0o40000 && mode&0o777 == 0 { + return 0o755, true + } + return mode, false +} + func (s *Store) SetMtime(ctx context.Context, path string, t time.Time) error { path = model.CleanPath(path) + if e, ok := s.Get(path); ok && e.Kind == model.OverlayKindMkdir { + if mode, normalized := normalizeGitDirMode(e.Mode); normalized { + if err := os.Chmod(e.BackingPath, os.FileMode(mode)); err != nil { + return err + } + _, err := s.db.ExecContext(ctx, + `UPDATE overlay_entries SET mode=?, mtime_unix_ns=?, ctime_unix_ns=? WHERE path=?`, + mode, t.UnixNano(), time.Now().UnixNano(), path) + return err + } + } _, err := s.db.ExecContext(ctx, `UPDATE overlay_entries SET mtime_unix_ns=?, ctime_unix_ns=? WHERE path=?`, t.UnixNano(), time.Now().UnixNano(), path) @@ -286,8 +435,9 @@ func (s *Store) SetMtime(ctx context.Context, path string, t time.Time) error { // snapshot. Called after a generation change (commit, branch switch, fetch). // // Rules for each overlay entry: -// - modify/rename with source_oid matching the new base OID: base unchanged, KEEP -// - modify/rename with source_oid != new base OID: base changed, REMOVE +// - modify with source_oid matching the base OID at path: base unchanged, KEEP +// - rename with source_oid matching the base OID at original path: base unchanged, KEEP +// - modify/rename with source_oid mismatch: base changed, REMOVE // - create where base now has the path: was committed or exists on new branch, REMOVE // - create where base doesn't have the path: user-created, KEEP // - delete (whiteout) where base doesn't have the path: irrelevant, REMOVE @@ -303,26 +453,42 @@ func (s *Store) Reconcile(ctx context.Context, baseLookup func(path string) (mod return fmt.Errorf("reconcile list: %w", err) } var toRemove []model.OverlayEntry + staleRenameSources := map[string]bool{} for _, e := range entries { base, baseExists := baseLookup(e.Path) - switch { - case e.Kind == model.OverlayKindDelete: - if !baseExists { + switch e.Kind { + case model.OverlayKindModify: + // Keep only if the base file still exists with the same OID the + // overlay was derived from. Otherwise the base changed (commit, + // branch switch) or disappeared and the overlay is stale. + if !baseExists || e.SourceOID != base.ObjectOID { toRemove = append(toRemove, e) } - case e.Kind == model.OverlayKindCreate: - if baseExists { + case model.OverlayKindRename: + sourcePath := e.TargetPath + if sourcePath == "" { + sourcePath = e.Path + } + sourceBase, sourceExists := baseLookup(sourcePath) + if !sourceExists || e.SourceOID != sourceBase.ObjectOID { toRemove = append(toRemove, e) + staleRenameSources[sourcePath] = true } - case e.Kind == model.OverlayKindMkdir: - if baseExists && base.Type == "dir" { + } + } + for _, e := range entries { + base, baseExists := baseLookup(e.Path) + switch e.Kind { + case model.OverlayKindDelete: + if staleRenameSources[e.Path] || staleRenameSources[e.TargetPath] || !baseExists { toRemove = append(toRemove, e) } - case e.Kind == model.OverlayKindModify || e.Kind == model.OverlayKindRename: - // Keep only if the base file still exists with the same OID the - // overlay was derived from. Otherwise the base changed (commit, - // branch switch) or disappeared and the overlay is stale. - if !baseExists || e.SourceOID != base.ObjectOID { + case model.OverlayKindCreate: + if baseExists { + toRemove = append(toRemove, e) + } + case model.OverlayKindMkdir: + if baseExists && base.Type == "dir" { toRemove = append(toRemove, e) } } @@ -344,10 +510,15 @@ func (s *Store) Reconcile(ctx context.Context, baseLookup func(path string) (mod return err } defer stmt.Close() + deleted := make([]model.OverlayEntry, 0, len(toRemove)) for _, e := range toRemove { - if _, err := stmt.ExecContext(ctx, e.Path, e.Kind, e.SourceOID, e.MtimeUnixNs); err != nil { + res, err := stmt.ExecContext(ctx, e.Path, e.Kind, e.SourceOID, e.MtimeUnixNs) + if err != nil { return err } + if n, err := res.RowsAffected(); err == nil && n > 0 { + deleted = append(deleted, e) + } } if err := tx.Commit(); err != nil { return err @@ -356,7 +527,7 @@ func (s *Store) Reconcile(ctx context.Context, baseLookup func(path string) (mod // before commit and the transaction rolled back, DB rows would reference // non-existent files. Reverse order so children are removed before parents // (os.Remove fails on non-empty directories). - for _, v := range slices.Backward(toRemove) { + for _, v := range slices.Backward(deleted) { if v.BackingPath != "" { _ = os.Remove(v.BackingPath) } diff --git a/internal/overlay/store_test.go b/internal/overlay/store_test.go index dfe3e5a..5ed7d70 100644 --- a/internal/overlay/store_test.go +++ b/internal/overlay/store_test.go @@ -2,6 +2,8 @@ package overlay import ( "context" + "errors" + iofs "io/fs" "os" "path/filepath" "testing" @@ -122,11 +124,24 @@ func TestRemoveCreatesWhiteout(t *testing.T) { } func TestRenameDBFirst(t *testing.T) { - s, _ := testStore(t) + s, cfg := testStore(t) ctx := context.Background() - s.CreateFile(ctx, "old.txt", 0o644) - s.WriteFile(ctx, "old.txt", 0, []byte("content")) + base := model.BaseNode{Path: "old.txt", Type: "file", Mode: 0o644, ObjectOID: "aaa"} + if _, err := s.EnsureCopyOnWrite(ctx, cfg, "old.txt", base); err != nil { + t.Fatal(err) + } + if _, err := s.WriteFile(ctx, "old.txt", 0, []byte("content")); err != nil { + t.Fatal(err) + } + target := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) + if err := s.SetMtime(ctx, "old.txt", target); err != nil { + t.Fatal(err) + } + before, ok := s.Get("old.txt") + if !ok { + t.Fatal("expected old entry") + } if err := s.Rename(ctx, "old.txt", "new.txt"); err != nil { t.Fatal(err) @@ -135,6 +150,10 @@ func TestRenameDBFirst(t *testing.T) { // Old path should have a whiteout if e, ok := s.Get("old.txt"); !ok || !e.IsDeleted() { t.Fatal("expected whiteout at old path") + } else { + if e.MtimeUnixNs == target.UnixNano() || e.CtimeUnixNs == target.UnixNano() { + t.Fatalf("whiteout times should not inherit user mtime: %+v", e) + } } // New path should exist got, ok := s.Get("new.txt") @@ -152,6 +171,329 @@ func TestRenameDBFirst(t *testing.T) { if got.SizeBytes != int64(len("content")) { t.Fatalf("size = %d, want %d", got.SizeBytes, len("content")) } + if got.MtimeUnixNs != target.UnixNano() { + t.Fatalf("mtime = %v, want %v", time.Unix(0, got.MtimeUnixNs), target) + } + if got.CtimeUnixNs == target.UnixNano() || got.CtimeUnixNs == 0 { + t.Fatalf("ctime should advance independently from mtime: %v", time.Unix(0, got.CtimeUnixNs)) + } + if got.CtimeUnixNs == before.CtimeUnixNs { + t.Fatalf("rename should bump ctime: before=%v after=%v", time.Unix(0, before.CtimeUnixNs), time.Unix(0, got.CtimeUnixNs)) + } +} + +func TestRenameSamePathNoop(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if _, err := s.CreateFile(ctx, "same.txt", 0o644); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "same.txt", "same.txt"); err != nil { + t.Fatal(err) + } + got, ok := s.Get("same.txt") + if !ok { + t.Fatal("expected entry") + } + if got.IsDeleted() { + t.Fatalf("same-path rename should not create whiteout: %+v", got) + } +} + +func TestRenameSameMissingPathReturnsNotExist(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Rename(ctx, "missing.txt", "missing.txt"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("err = %v, want os.ErrNotExist", err) + } +} + +func TestRenameCreateRemainsCreateAcrossReconcile(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if _, err := s.CreateFile(ctx, "tmp.txt", 0o644); err != nil { + t.Fatal(err) + } + if _, err := s.WriteFile(ctx, "tmp.txt", 0, []byte("content")); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "tmp.txt", "kept.txt"); err != nil { + t.Fatal(err) + } + + before, ok := s.Get("kept.txt") + if !ok { + t.Fatal("expected renamed create entry") + } + if before.Kind != model.OverlayKindCreate { + t.Fatalf("kind = %q, want create", before.Kind) + } + if _, ok := s.Get("tmp.txt"); ok { + t.Fatal("untracked create rename should not leave a source whiteout") + } + + baseLookup := func(string) (model.BaseNode, bool) { return model.BaseNode{}, false } + if err := s.Reconcile(ctx, baseLookup); err != nil { + t.Fatal(err) + } + after, ok := s.Get("kept.txt") + if !ok || after.IsDeleted() { + t.Fatalf("reconcile should keep renamed create, got %+v ok=%v", after, ok) + } +} + +func TestRenamePreservesDirectoryKindAndRepairsMode(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "src", 0o40000); err != nil { + t.Fatal(err) + } + if _, err := s.db.ExecContext(ctx, `UPDATE overlay_entries SET mode=? WHERE path=?`, 0o40000, "src"); err != nil { + t.Fatal(err) + } + if err := os.Chmod(s.backingPath("src"), 0); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "src", "dst"); err != nil { + t.Fatal(err) + } + + got, ok := s.Get("dst") + if !ok { + t.Fatal("expected renamed entry") + } + if got.Kind != model.OverlayKindMkdir || got.NodeType() != "dir" { + t.Fatalf("expected renamed directory entry, got %+v", got) + } + if got.Mode != 0o755 { + t.Fatalf("mode = %#o, want 0755", got.Mode) + } + st, err := os.Stat(got.BackingPath) + if err != nil { + t.Fatal(err) + } + if st.Mode().Perm() != 0o755 { + t.Fatalf("backing mode = %#o, want 0755", st.Mode().Perm()) + } +} + +func TestRenameRejectsNonEmptyOverlayDirectory(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "src", 0o755); err != nil { + t.Fatal(err) + } + if _, err := s.CreateFile(ctx, "src/a.txt", 0o644); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "src", "dst"); !errors.Is(err, iofs.ErrInvalid) { + t.Fatalf("err = %v, want fs.ErrInvalid", err) + } + if _, ok := s.Get("src"); !ok { + t.Fatal("source directory should remain after rejected rename") + } + if _, ok := s.Get("dst"); ok { + t.Fatal("destination directory should not exist after rejected rename") + } +} + +func TestRenameOverlayDirectoryIgnoresDeletedDescendants(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "a_b", 0o755); err != nil { + t.Fatal(err) + } + if _, err := s.CreateFile(ctx, "a_b/a.txt", 0o644); err != nil { + t.Fatal(err) + } + if err := s.Remove(ctx, "a_b/a.txt"); err != nil { + t.Fatal(err) + } + if err := s.Remove(ctx, "axb/secret.txt"); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "a_b", "dst"); err != nil { + t.Fatal(err) + } + got, ok := s.Get("dst") + if !ok || got.Kind != model.OverlayKindMkdir { + t.Fatalf("expected renamed directory, got %+v ok=%v", got, ok) + } + if e, ok := s.Get("a_b/a.txt"); ok { + t.Fatalf("deleted descendant tombstone should be removed after directory rename, got %+v", e) + } + if e, ok := s.Get("axb/secret.txt"); !ok || !e.IsDeleted() { + t.Fatalf("unrelated wildcard-like tombstone should remain, got %+v ok=%v", e, ok) + } +} + +func TestRenameOverlayDirectoryCleansUTF8DeletedDescendants(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + dir := "\u00e9" + child := dir + "/a.txt" + + if err := s.Mkdir(ctx, dir, 0o755); err != nil { + t.Fatal(err) + } + if _, err := s.CreateFile(ctx, child, 0o644); err != nil { + t.Fatal(err) + } + if err := s.Remove(ctx, child); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, dir, "dst"); err != nil { + t.Fatal(err) + } + if e, ok := s.Get(child); ok { + t.Fatalf("UTF-8 deleted descendant tombstone should be removed after directory rename, got %+v", e) + } +} + +func TestRenameOverlayDirectoryRollbackRestoresDeletedDescendants(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "src", 0o755); err != nil { + t.Fatal(err) + } + if _, err := s.CreateFile(ctx, "src/a.txt", 0o644); err != nil { + t.Fatal(err) + } + if err := s.Remove(ctx, "src/a.txt"); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(s.backingPath("dst"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(s.backingPath("dst"), "file.txt"), []byte("busy"), 0o644); err != nil { + t.Fatal(err) + } + + if err := s.Rename(ctx, "src", "dst"); err == nil { + t.Fatal("expected filesystem rename failure") + } + if e, ok := s.Get("src/a.txt"); !ok || !e.IsDeleted() { + t.Fatalf("deleted descendant tombstone should be restored on rollback, got %+v ok=%v", e, ok) + } +} + +func TestRenameChainPreservesOriginalSourceForReconcile(t *testing.T) { + s, cfg := testStore(t) + ctx := context.Background() + base := model.BaseNode{Path: "a.txt", Type: "file", Mode: 0o644, ObjectOID: "aaa"} + + if _, err := s.EnsureCopyOnWrite(ctx, cfg, "a.txt", base); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "a.txt", "b.txt"); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "b.txt", "c.txt"); err != nil { + t.Fatal(err) + } + + before, ok := s.Get("c.txt") + if !ok { + t.Fatal("expected final rename entry") + } + if before.TargetPath != "a.txt" { + t.Fatalf("target path = %q, want original source a.txt", before.TargetPath) + } + + baseLookup := func(path string) (model.BaseNode, bool) { + if path == "a.txt" { + return base, true + } + return model.BaseNode{}, false + } + if err := s.Reconcile(ctx, baseLookup); err != nil { + t.Fatal(err) + } + + after, ok := s.Get("c.txt") + if !ok || after.IsDeleted() { + t.Fatalf("reconcile should keep chained rename, got %+v ok=%v", after, ok) + } + if after.TargetPath != "a.txt" { + t.Fatalf("target path after reconcile = %q, want a.txt", after.TargetPath) + } +} + +func TestReconcileRemovesSourceWhiteoutForStaleRename(t *testing.T) { + s, cfg := testStore(t) + ctx := context.Background() + base := model.BaseNode{Path: "a.txt", Type: "file", Mode: 0o644, ObjectOID: "aaa"} + + if _, err := s.EnsureCopyOnWrite(ctx, cfg, "a.txt", base); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "a.txt", "b.txt"); err != nil { + t.Fatal(err) + } + if e, ok := s.Get("a.txt"); !ok || !e.IsDeleted() { + t.Fatalf("expected source whiteout before reconcile, got %+v ok=%v", e, ok) + } + + baseLookup := func(path string) (model.BaseNode, bool) { + if path == "a.txt" { + return model.BaseNode{Path: "a.txt", Type: "file", Mode: 0o644, ObjectOID: "bbb"}, true + } + return model.BaseNode{}, false + } + if err := s.Reconcile(ctx, baseLookup); err != nil { + t.Fatal(err) + } + if e, ok := s.Get("b.txt"); ok { + t.Fatalf("stale rename should be removed, got %+v", e) + } + if e, ok := s.Get("a.txt"); ok { + t.Fatalf("paired source whiteout should be removed with stale rename, got %+v", e) + } +} + +func TestReconcileRemovesIntermediateWhiteoutsForStaleChainedRename(t *testing.T) { + s, cfg := testStore(t) + ctx := context.Background() + base := model.BaseNode{Path: "a.txt", Type: "file", Mode: 0o644, ObjectOID: "aaa"} + + if _, err := s.EnsureCopyOnWrite(ctx, cfg, "a.txt", base); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "a.txt", "b.txt"); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "b.txt", "c.txt"); err != nil { + t.Fatal(err) + } + if e, ok := s.Get("b.txt"); !ok || !e.IsDeleted() { + t.Fatalf("expected intermediate whiteout before reconcile, got %+v ok=%v", e, ok) + } + + baseLookup := func(path string) (model.BaseNode, bool) { + switch path { + case "a.txt": + return model.BaseNode{Path: "a.txt", Type: "file", Mode: 0o644, ObjectOID: "bbb"}, true + case "b.txt": + return model.BaseNode{Path: "b.txt", Type: "file", Mode: 0o644, ObjectOID: "new-b"}, true + default: + return model.BaseNode{}, false + } + } + if err := s.Reconcile(ctx, baseLookup); err != nil { + t.Fatal(err) + } + for _, path := range []string{"a.txt", "b.txt", "c.txt"} { + if e, ok := s.Get(path); ok { + t.Fatalf("%s should be removed after stale chained rename reconcile, got %+v", path, e) + } + } } func TestEnsureCopyOnWritePreservesSize(t *testing.T) { @@ -199,6 +541,72 @@ func TestMkdir(t *testing.T) { } } +func TestMkdirNormalizesGitTreeMode(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "src", 0o40000); err != nil { + t.Fatal(err) + } + e, ok := s.Get("src") + if !ok { + t.Fatal("expected entry") + } + if e.Mode != 0o755 { + t.Fatalf("mode = %#o, want 0755", e.Mode) + } +} + +func TestMkdirPreservesExplicitZeroMode(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "private", 0); err != nil { + t.Fatal(err) + } + e, ok := s.Get("private") + if !ok { + t.Fatal("expected entry") + } + if e.Mode != 0 { + t.Fatalf("mode = %#o, want 0", e.Mode) + } +} + +func TestSetMtimeRepairsGitTreeDirectoryMode(t *testing.T) { + s, _ := testStore(t) + ctx := context.Background() + + if err := s.Mkdir(ctx, "src", 0o40000); err != nil { + t.Fatal(err) + } + if _, err := s.db.ExecContext(ctx, `UPDATE overlay_entries SET mode=? WHERE path=?`, 0o40000, "src"); err != nil { + t.Fatal(err) + } + if err := os.Chmod(s.backingPath("src"), 0); err != nil { + t.Fatal(err) + } + target := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) + if err := s.SetMtime(ctx, "src", target); err != nil { + t.Fatal(err) + } + + e, ok := s.Get("src") + if !ok { + t.Fatal("expected entry") + } + if e.Mode != 0o755 { + t.Fatalf("mode = %#o, want 0755", e.Mode) + } + st, err := os.Stat(e.BackingPath) + if err != nil { + t.Fatal(err) + } + if st.Mode().Perm() != 0o755 { + t.Fatalf("backing mode = %#o, want 0755", st.Mode().Perm()) + } +} + func TestTruncateUpdatesSizeAndTimes(t *testing.T) { s, _ := testStore(t) ctx := context.Background()