From 8004dc50ecb2ff01de534b86761ad493c5c99e4a Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Mon, 29 Jun 2026 08:59:47 -0400 Subject: [PATCH 1/3] optimize git status on mounts --- internal/cli/cli.go | 13 ++ internal/daemon/daemon.go | 69 ++++++++++ internal/daemon/status_test.go | 77 +++++++++++ internal/fusefs/fuse_unix.go | 155 ++++++++++++++++++----- internal/fusefs/fuse_unix_test.go | 116 +++++++++++++++++ internal/fusefs/merged.go | 37 +++++- internal/fusefs/ops_test.go | 5 +- internal/fusefs/readsymlink_unix_test.go | 8 +- internal/gitstore/gitstore.go | 104 +++++++++++++++ internal/gitstore/gitstore_test.go | 45 +++++++ internal/overlay/store.go | 4 + 11 files changed, 589 insertions(+), 44 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 4ff7860..57808f5 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -170,6 +170,19 @@ func Run(ctx context.Context, args []string, stdout io.Writer, stderr io.Writer) fmt.Fprintf(w, "unmounted %s\n", name) return nil }), + { + Name: "fsmonitor-hook", + Usage: "internal fsmonitor hook", + Hidden: true, + Flags: []ucli.Flag{ucli.StringFlag{Name: "name", Usage: "repo name (required)"}}, + Action: withService(ctx, root, stderr, func(c *ucli.Context, svc *daemon.Service) error { + name := strings.TrimSpace(c.String("name")) + if name == "" { + return fmt.Errorf("--name required") + } + return svc.FSMonitorHook(ctx, name, stdout) + }), + }, { Name: "set-refresh", Usage: "update refresh interval", diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index efea8b7..6201d0b 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -4,10 +4,12 @@ import ( "context" "errors" "fmt" + "io" "io/fs" "log/slog" "os" "path/filepath" + "sort" "strings" "sync" "time" @@ -565,6 +567,8 @@ func (s *Service) mountRepo(ctx context.Context, cfg model.RepoConfig) error { if err != nil { s.logger.Error("fuse mount failed, running without FUSE", "repo", cfg.Name, "error", err) mfs = nil + } else { + s.configureStatusOptimization(ctx, cfg) } rt := &repoRuntime{ cfg: cfg, @@ -804,6 +808,7 @@ func (s *Service) runPrepare(ctx context.Context, cfg model.RepoConfig) error { if err := s.completePreparedRuntime(ctx, cfg, headOID, headRef, gen); err != nil { return fail(err) } + s.configureStatusOptimization(ctx, cfg) return nil } @@ -972,6 +977,7 @@ func (s *Service) onHEADChanged(ctx context.Context, rt *repoRuntime) { if err := s.git.ReadTreeHEAD(ctx, rt.cfg); err != nil { s.logger.Warn("read-tree HEAD failed", "repo", rt.cfg.Name, "error", err) } + s.configureStatusOptimization(ctx, rt.cfg) s.refreshCommitTime(ctx, rt.cfg, oid, rt.resolver, "commit timestamp unavailable") // Atomically update the resolver's generation so FUSE ops see the new snapshot @@ -981,6 +987,69 @@ func (s *Service) onHEADChanged(ctx context.Context, rt *repoRuntime) { s.mu.Unlock() } +func (s *Service) configureStatusOptimization(ctx context.Context, cfg model.RepoConfig) { + if err := s.git.ConfigureStatusOptimization(ctx, cfg, s.root); err != nil { + s.logger.Warn("git status optimization setup failed", "repo", cfg.Name, "error", err) + } +} + +func (s *Service) FSMonitorHook(ctx context.Context, name string, w io.Writer) error { + cfg, err := s.registry.GetRepo(ctx, name) + if err != nil { + return err + } + s.fillPaths(&cfg) + ov, err := overlay.New(ctx, cfg) + if err != nil { + return err + } + defer ov.Close() + entries, err := ov.ListAll(ctx) + if err != nil { + return err + } + paths := fsMonitorDirtyPaths(entries) + token := fmt.Sprintf("artifact-fs:%s:%d", cfg.ID, time.Now().UnixNano()) + if _, err := io.WriteString(w, token); err != nil { + return err + } + if _, err := w.Write([]byte{0}); err != nil { + return err + } + for _, p := range paths { + if _, err := io.WriteString(w, p); err != nil { + return err + } + if _, err := w.Write([]byte{0}); err != nil { + return err + } + } + return nil +} + +func fsMonitorDirtyPaths(entries []model.OverlayEntry) []string { + set := map[string]struct{}{} + add := func(path string) { + path = model.CleanPath(path) + if path == "." { + return + } + set[path] = struct{}{} + } + for _, e := range entries { + add(e.Path) + if e.TargetPath != "" { + add(e.TargetPath) + } + } + out := make([]string, 0, len(set)) + for p := range set { + out = append(out, p) + } + sort.Strings(out) + return out +} + func (s *Service) refreshLoop(rt *repoRuntime) { backoff := rt.cfg.RefreshInterval const maxBackoff = 10 * time.Minute diff --git a/internal/daemon/status_test.go b/internal/daemon/status_test.go index efb3d7b..6ed9d44 100644 --- a/internal/daemon/status_test.go +++ b/internal/daemon/status_test.go @@ -1,12 +1,19 @@ package daemon import ( + "bytes" "context" + "io" + "log/slog" "os" "path/filepath" + "reflect" + "strings" "testing" + "time" "github.com/cloudflare/artifact-fs/internal/model" + "github.com/cloudflare/artifact-fs/internal/overlay" ) func TestReadPersistedStatusIncludesHydrationStats(t *testing.T) { @@ -46,3 +53,73 @@ func TestReadPersistedStatusIncludesHydrationStats(t *testing.T) { t.Fatalf("HydratedBlobBytes = %d, want 8", st.HydratedBlobBytes) } } + +func TestFSMonitorDirtyPathsIncludesChangedPathsAndRenameEndpoints(t *testing.T) { + paths := fsMonitorDirtyPaths([]model.OverlayEntry{ + {Path: "src/pkg/file.go", Kind: model.OverlayKindModify}, + {Path: "new/name.go", Kind: model.OverlayKindRename, TargetPath: "old/name.go"}, + {Path: ".", Kind: model.OverlayKindMkdir}, + }) + want := []string{ + "new/name.go", + "old/name.go", + "src/pkg/file.go", + } + if !reflect.DeepEqual(paths, want) { + t.Fatalf("dirty paths = %#v, want %#v", paths, want) + } +} + +func TestFSMonitorHookOutputsDirtyOverlayPaths(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + svc, err := New(ctx, root, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatal(err) + } + defer svc.Close() + cfg := model.RepoConfig{ + Name: "repo", + ID: "repo", + Branch: "main", + RefreshInterval: time.Minute, + Enabled: true, + } + svc.fillPaths(&cfg) + if err := svc.registry.AddRepo(ctx, cfg); err != nil { + t.Fatal(err) + } + ov, err := overlay.New(ctx, cfg) + if err != nil { + t.Fatal(err) + } + if _, err := ov.CreateFile(ctx, "new/file.txt", 0o644); err != nil { + t.Fatal(err) + } + if err := ov.Remove(ctx, "deleted.txt"); err != nil { + t.Fatal(err) + } + if err := ov.Close(); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + if err := svc.FSMonitorHook(ctx, "repo", &out); err != nil { + t.Fatal(err) + } + parts := strings.Split(out.String(), "\x00") + if !strings.HasPrefix(parts[0], "artifact-fs:repo:") { + t.Fatalf("token = %q", parts[0]) + } + got := map[string]bool{} + for _, p := range parts[1:] { + if p != "" { + got[p] = true + } + } + for _, want := range []string{"deleted.txt", "new/file.txt"} { + if !got[want] { + t.Fatalf("fsmonitor paths missing %q: %v", want, got) + } + } +} diff --git a/internal/fusefs/fuse_unix.go b/internal/fusefs/fuse_unix.go index 58b8ba7..af1e920 100644 --- a/internal/fusefs/fuse_unix.go +++ b/internal/fusefs/fuse_unix.go @@ -48,14 +48,17 @@ type InodeRef struct { Path string Type string // file, dir, symlink Mode uint32 + Gen int64 Refcnt int64 IsRoot bool Overlay bool } type DirHandle struct { - inode *InodeRef - entries []ReaddirEntry + inode *InodeRef + gen int64 + commitTime int64 + entries []ReaddirEntry } type FileHandle struct { @@ -69,11 +72,26 @@ type FileHandle struct { // ReaddirEntry holds child metadata, avoiding per-child Getattr or snapshot lookups. type ReaddirEntry struct { - Name string - Type string // file, dir, symlink - ObjectOID string - SizeState string - SizeBytes int64 + Name string + Type string // file, dir, symlink + Mode uint32 + ObjectOID string + SizeState string + SizeBytes int64 + FromOverlay bool + MtimeUnixNs int64 + CtimeUnixNs int64 +} + +func (e ReaddirEntry) direntType() fuseutil.DirentType { + switch e.Type { + case "dir": + return fuseutil.DT_Directory + case "symlink": + return fuseutil.DT_Link + default: + return fuseutil.DT_File + } } func NewArtifactFuse(repo model.RepoConfig, resolver *Resolver, engine *Engine) *ArtifactFuse { @@ -95,7 +113,7 @@ func NewArtifactFuse(repo model.RepoConfig, resolver *Resolver, engine *Engine) return fs } -func (fs *ArtifactFuse) allocInode(path, typ string, mode uint32) *InodeRef { +func (fs *ArtifactFuse) allocInode(path, typ string, mode uint32, gen int64) *InodeRef { // Caller must hold fs.mu write lock. if id, ok := fs.pathToInode[path]; ok { if ref, ok := fs.inodes[id]; ok { @@ -105,7 +123,7 @@ func (fs *ArtifactFuse) allocInode(path, typ string, mode uint32) *InodeRef { } id := fs.nextInodeID fs.nextInodeID++ - ref := &InodeRef{ID: id, Path: path, Type: typ, Mode: mode, Refcnt: 1} + ref := &InodeRef{ID: id, Path: path, Type: typ, Mode: mode, Gen: gen, Refcnt: 1} fs.inodes[id] = ref fs.pathToInode[path] = id return ref @@ -126,6 +144,19 @@ func (fs *ArtifactFuse) requireInode(id fuseops.InodeID, missing error) (*InodeR return ref, nil } +func (fs *ArtifactFuse) dropInodeLookup(id fuseops.InodeID) { + fs.mu.Lock() + ref, ok := fs.inodes[id] + if ok { + ref.Refcnt-- + if ref.Refcnt <= 0 && !ref.IsRoot { + delete(fs.inodes, id) + delete(fs.pathToInode, ref.Path) + } + } + fs.mu.Unlock() +} + func (fs *ArtifactFuse) childPath(parentID fuseops.InodeID, name string) (*InodeRef, string, error) { parent, err := fs.requireInode(parentID, syscall.ENOENT) if err != nil { @@ -258,7 +289,7 @@ func (fs *ArtifactFuse) LookUpInode(ctx context.Context, op *fuseops.LookUpInode // Synthesize .git gitfile in root if parent.IsRoot && op.Name == ".git" { fs.mu.Lock() - ref := fs.allocInode(".git", "file", 0o644) + ref := fs.allocInode(".git", "file", 0o644, fs.resolver.Generation()) fs.mu.Unlock() op.Entry.Child = ref.ID op.Entry.Attributes = fs.gitFileAttrs() @@ -275,7 +306,7 @@ func (fs *ArtifactFuse) LookUpInode(ctx context.Context, op *fuseops.LookUpInode } fs.mu.Lock() - ref := fs.allocInode(childPath, typ, mode) + ref := fs.allocInode(childPath, typ, mode, fs.resolver.Generation()) fs.mu.Unlock() op.Entry.Child = ref.ID @@ -405,20 +436,18 @@ func (fs *ArtifactFuse) OpenDir(ctx context.Context, op *fuseops.OpenDirOp) erro if err != nil { return err } + gen := fs.resolver.Generation() + commitTime := fs.resolver.CommitTime() // Eagerly load children at open time to avoid races on concurrent ReadDir. - entries, err := fs.resolver.ReaddirTyped(ctx, ref.Path) + entries, err := fs.resolver.ReaddirTypedAt(ctx, ref.Path, gen) if err != nil { return syscall.EIO } if ref.IsRoot { - entries = append([]ReaddirEntry{{Name: ".git", Type: "file"}}, entries...) + entries = append([]ReaddirEntry{{Name: ".git", Type: "file", Mode: 0o644, SizeBytes: int64(len(fs.gitfileContent)), SizeState: "known"}}, entries...) } - // Speculative prefetch: enqueue file children for hydration at a lower - // priority so they're warmed in the cache before the user opens them. - go fs.engine.PrefetchDir(ref.Path, entries) - - dh := &DirHandle{inode: ref, entries: entries} + dh := &DirHandle{inode: ref, gen: gen, commitTime: commitTime, entries: entries} fs.mu.Lock() handle := fs.nextHandleID fs.nextHandleID++ @@ -437,18 +466,11 @@ func (fs *ArtifactFuse) ReadDir(_ context.Context, op *fuseops.ReadDirOp) error offset := int(op.Offset) for i := offset; i < len(dh.entries); i++ { e := dh.entries[i] - dt := fuseutil.DT_File - switch e.Type { - case "dir": - dt = fuseutil.DT_Directory - case "symlink": - dt = fuseutil.DT_Link - } dirent := fuseutil.Dirent{ Offset: fuseops.DirOffset(i + 1), Inode: fuseops.RootInodeID + 1, // placeholder; kernel re-looks-up via LookUpInode Name: e.Name, - Type: dt, + Type: e.direntType(), } n := fuseutil.WriteDirent(op.Dst[op.BytesRead:], dirent) if n == 0 { @@ -459,6 +481,78 @@ func (fs *ArtifactFuse) ReadDir(_ context.Context, op *fuseops.ReadDirOp) error return nil } +func (fs *ArtifactFuse) ReadDirPlus(_ context.Context, op *fuseops.ReadDirPlusOp) error { + dh, err := fs.dirHandle(op.Handle) + if err != nil { + return err + } + + offset := int(op.Offset) + for i := offset; i < len(dh.entries); i++ { + e := dh.entries[i] + childPath := cleanChildPath(dh.inode.Path, e.Name) + entry, err := fs.childEntryFromReaddir(childPath, dh.gen, dh.commitTime, e) + if err != nil { + if errors.Is(err, iofs.ErrNotExist) { + return syscall.ENOENT + } + return syscall.EIO + } + dirent := fuseutil.DirentPlus{ + Dirent: fuseutil.Dirent{ + Offset: fuseops.DirOffset(i + 1), + Inode: entry.Child, + Name: e.Name, + Type: e.direntType(), + }, + Entry: entry, + } + n := fuseutil.WriteDirentPlus(op.Dst[op.BytesRead:], dirent) + if n == 0 { + fs.dropInodeLookup(entry.Child) + break + } + op.BytesRead += n + } + return nil +} + +func (fs *ArtifactFuse) childEntryFromReaddir(path string, gen, commitTime int64, e ReaddirEntry) (fuseops.ChildInodeEntry, error) { + if path == ".git" { + fs.mu.Lock() + ref := fs.allocInode(path, "file", 0o644, gen) + fs.mu.Unlock() + entry := fuseops.ChildInodeEntry{Child: ref.ID, Attributes: fs.gitFileAttrs()} + setChildEntryExpiry(&entry, time.Minute) + return entry, nil + } + mode, size, typ, mtime, ctime := readdirAttrs(e, gen, commitTime) + fs.mu.Lock() + ref := fs.allocInode(path, typ, mode, gen) + fs.mu.Unlock() + entry := fuseops.ChildInodeEntry{ + Child: ref.ID, + Attributes: inodeAttrs(mode, uint64(size), typ, mtime, ctime), + } + setChildEntryExpiry(&entry, time.Second) + return entry, nil +} + +func readdirAttrs(e ReaddirEntry, gen, commitTime int64) (mode uint32, size int64, typ string, mtime time.Time, ctime time.Time) { + typ = e.Type + mode = normalizeMode(e.Mode, typ) + size = e.SizeBytes + if e.FromOverlay { + return mode, size, typ, time.Unix(0, e.MtimeUnixNs), time.Unix(0, e.CtimeUnixNs) + } + ct := commitTime + if ct == 0 { + ct = gen + } + mt := time.Unix(ct, 0) + return mode, size, typ, mt, mt +} + func (fs *ArtifactFuse) ReleaseDirHandle(_ context.Context, op *fuseops.ReleaseDirHandleOp) error { fs.mu.Lock() delete(fs.dirHandles, op.Handle) @@ -535,7 +629,7 @@ func (fs *ArtifactFuse) CreateFile(ctx context.Context, op *fuseops.CreateFileOp return syscall.EIO } fs.mu.Lock() - ref := fs.allocInode(childPath, "file", uint32(op.Mode)) + ref := fs.allocInode(childPath, "file", uint32(op.Mode), fs.resolver.Generation()) fh := &FileHandle{inode: ref, path: childPath} handle := fs.nextHandleID fs.nextHandleID++ @@ -559,7 +653,7 @@ func (fs *ArtifactFuse) MkDir(ctx context.Context, op *fuseops.MkDirOp) error { return syscall.EIO } fs.mu.Lock() - ref := fs.allocInode(childPath, "dir", uint32(op.Mode)) + ref := fs.allocInode(childPath, "dir", uint32(op.Mode), fs.resolver.Generation()) fs.mu.Unlock() op.Entry.Child = ref.ID @@ -720,9 +814,8 @@ func MountRepoWithGate(repo model.RepoConfig, resolver *Resolver, engine *Engine Subtype: "artifact-fs", DisableWritebackCaching: true, UseVectoredRead: true, - // UseReadDirPlus intentionally not set -- the ReadDir implementation - // uses WriteDirent (plain format). Enable only after implementing - // WriteDirentPlus with full ChildInodeEntry. + EnableReaddirplus: true, + EnableAutoReaddirplus: true, } platformMountConfig(mountCfg) diff --git a/internal/fusefs/fuse_unix_test.go b/internal/fusefs/fuse_unix_test.go index 9b17caa..6b7692b 100644 --- a/internal/fusefs/fuse_unix_test.go +++ b/internal/fusefs/fuse_unix_test.go @@ -245,6 +245,122 @@ func TestLookUpInodeDoesNotHydrateKnownOverlayDirOrSymlinkAttributes(t *testing. } } +func TestReadDirPlusUsesReaddirMetadataWithoutHydration(t *testing.T) { + repo := model.RepoConfig{ID: "repo", GitDir: "/tmp/repo.git"} + r := newResolver( + &fakeSnapshot{kids: map[string][]model.BaseNode{ + ".": {{Path: "file.txt", Type: "file", Mode: 0o100644, ObjectOID: "blob", SizeState: "known", SizeBytes: 42}}, + }}, + &fakeOverlay{entries: map[string]model.OverlayEntry{}}, + ) + h := &fakeLookupHydrator{size: 12} + fs := NewArtifactFuse(repo, r, &Engine{Resolver: r, Repo: repo, Hydrator: h}) + open := &fuseops.OpenDirOp{Inode: fuseops.RootInodeID} + if err := fs.OpenDir(context.Background(), open); err != nil { + t.Fatalf("OpenDir: %v", err) + } + op := &fuseops.ReadDirPlusOp{} + op.Handle = open.Handle + op.Dst = make([]byte, 4096) + if err := fs.ReadDirPlus(context.Background(), op); err != nil { + t.Fatalf("ReadDirPlus: %v", err) + } + if op.BytesRead == 0 { + t.Fatal("ReadDirPlus wrote no entries") + } + if h.calls != 0 { + t.Fatalf("EnsureHydrated calls = %d, want 0", h.calls) + } + if fs.pathToInode["file.txt"] == 0 { + t.Fatal("ReadDirPlus did not allocate file inode") + } +} + +func TestReadDirPlusDoesNotHydrateUnknownSizeBaseFile(t *testing.T) { + repo := model.RepoConfig{ID: "repo", GitDir: "/tmp/repo.git"} + r := newResolver( + &fakeSnapshot{kids: map[string][]model.BaseNode{ + ".": {{Path: "file.txt", Type: "file", Mode: 0o100644, ObjectOID: "blob", SizeState: "unknown"}}, + }}, + &fakeOverlay{entries: map[string]model.OverlayEntry{}}, + ) + h := &fakeLookupHydrator{size: 12} + fs := NewArtifactFuse(repo, r, &Engine{Resolver: r, Repo: repo, Hydrator: h}) + open := &fuseops.OpenDirOp{Inode: fuseops.RootInodeID} + if err := fs.OpenDir(context.Background(), open); err != nil { + t.Fatalf("OpenDir: %v", err) + } + op := &fuseops.ReadDirPlusOp{} + op.Handle = open.Handle + op.Dst = make([]byte, 4096) + + if err := fs.ReadDirPlus(context.Background(), op); err != nil { + t.Fatalf("ReadDirPlus: %v", err) + } + if op.BytesRead == 0 { + t.Fatal("ReadDirPlus wrote no entries") + } + if h.calls != 0 { + t.Fatalf("EnsureHydrated calls = %d, want 0", h.calls) + } +} + +func TestReadDirPlusUsesOpenDirGeneration(t *testing.T) { + repo := model.RepoConfig{ID: "repo", GitDir: "/tmp/repo.git"} + snap := &generationSnapshot{nodes: map[int64]map[string]model.BaseNode{}, kids: map[int64]map[string][]model.BaseNode{ + 1: {".": {{Path: "old.txt", Type: "file", Mode: 0o100644, SizeState: "known", SizeBytes: 1}}}, + 2: {".": {{Path: "new.txt", Type: "file", Mode: 0o100644, SizeState: "known", SizeBytes: 2}}}, + }} + r := &Resolver{Snapshot: snap, Overlay: &fakeOverlay{entries: map[string]model.OverlayEntry{}}} + r.SetGeneration(1) + fs := NewArtifactFuse(repo, r, &Engine{Resolver: r, Repo: repo, Hydrator: &fakeLookupHydrator{}}) + open := &fuseops.OpenDirOp{Inode: fuseops.RootInodeID} + if err := fs.OpenDir(context.Background(), open); err != nil { + t.Fatalf("OpenDir: %v", err) + } + r.SetGeneration(2) + op := &fuseops.ReadDirPlusOp{} + op.Handle = open.Handle + op.Dst = make([]byte, 4096) + + if err := fs.ReadDirPlus(context.Background(), op); err != nil { + t.Fatalf("ReadDirPlus: %v", err) + } + if fs.pathToInode["old.txt"] == 0 { + t.Fatal("ReadDirPlus did not use OpenDir generation entry") + } + if fs.pathToInode["new.txt"] != 0 { + t.Fatal("ReadDirPlus used live resolver generation entry") + } +} + +func TestReadDirPlusDropsLookupWhenEntryDoesNotFit(t *testing.T) { + repo := model.RepoConfig{ID: "repo", GitDir: "/tmp/repo.git"} + r := newResolver( + &fakeSnapshot{kids: map[string][]model.BaseNode{ + ".": {{Path: "file.txt", Type: "file", Mode: 0o100644, ObjectOID: "blob", SizeState: "known", SizeBytes: 42}}, + }}, + &fakeOverlay{entries: map[string]model.OverlayEntry{}}, + ) + fs := NewArtifactFuse(repo, r, &Engine{Resolver: r, Repo: repo, Hydrator: &fakeLookupHydrator{}}) + open := &fuseops.OpenDirOp{Inode: fuseops.RootInodeID} + if err := fs.OpenDir(context.Background(), open); err != nil { + t.Fatalf("OpenDir: %v", err) + } + op := &fuseops.ReadDirPlusOp{} + op.Handle = open.Handle + op.Dst = make([]byte, 1) + if err := fs.ReadDirPlus(context.Background(), op); err != nil { + t.Fatalf("ReadDirPlus: %v", err) + } + if op.BytesRead != 0 { + t.Fatalf("BytesRead = %d, want 0", op.BytesRead) + } + if fs.pathToInode[".git"] != 0 { + t.Fatal("inode lookup leaked for entry that did not fit") + } +} + type fakeLookupHydrator struct { size int64 calls int diff --git a/internal/fusefs/merged.go b/internal/fusefs/merged.go index 3fa8869..92c8262 100644 --- a/internal/fusefs/merged.go +++ b/internal/fusefs/merged.go @@ -123,15 +123,21 @@ func (r *Resolver) Readdir(ctx context.Context, path string) ([]string, error) { // ReaddirTyped returns directory entries with name and type, so the FUSE // adapter doesn't need to call Getattr per child. func (r *Resolver) ReaddirTyped(ctx context.Context, path string) ([]ReaddirEntry, error) { + return r.ReaddirTypedAt(ctx, path, r.Generation()) +} + +func (r *Resolver) ReaddirTypedAt(ctx context.Context, path string, generation int64) ([]ReaddirEntry, error) { path = model.CleanPath(path) - children, err := r.Snapshot.ListChildren(r.Generation(), path) + children, err := r.Snapshot.ListChildren(generation, path) if err != nil && !errors.Is(err, fs.ErrNotExist) { return nil, err } type entry struct { - name string - typ string - base model.BaseNode + name string + typ string + base model.BaseNode + overlay model.OverlayEntry + fromOverlay bool } set := map[string]entry{} for _, c := range children { @@ -158,13 +164,30 @@ func (r *Resolver) ReaddirTyped(ctx context.Context, path string) ([]ReaddirEntr if ov, ok := r.Overlay.Get(childPath); ok && ov.IsDeleted() { continue } - typ := e.NodeType() - set[name] = entry{name: name, typ: typ} + if ov, ok := r.Overlay.Get(childPath); ok { + set[name] = entry{name: name, typ: ov.NodeType(), overlay: ov, fromOverlay: true} + continue + } + if _, ok := set[name]; !ok { + set[name] = entry{name: name, typ: "dir", base: model.BaseNode{Type: "dir", Mode: 0o755, SizeState: "known"}} + } } } out := make([]ReaddirEntry, 0, len(set)) for _, e := range set { - out = append(out, ReaddirEntry{Name: e.name, Type: e.typ, ObjectOID: e.base.ObjectOID, SizeState: e.base.SizeState, SizeBytes: e.base.SizeBytes}) + if e.fromOverlay { + out = append(out, ReaddirEntry{ + Name: e.name, + Type: e.typ, + Mode: e.overlay.Mode, + SizeBytes: e.overlay.SizeBytes, + FromOverlay: true, + MtimeUnixNs: e.overlay.MtimeUnixNs, + CtimeUnixNs: e.overlay.CtimeUnixNs, + }) + continue + } + out = append(out, ReaddirEntry{Name: e.name, Type: e.typ, Mode: e.base.Mode, ObjectOID: e.base.ObjectOID, SizeState: e.base.SizeState, SizeBytes: e.base.SizeBytes}) } sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out, nil diff --git a/internal/fusefs/ops_test.go b/internal/fusefs/ops_test.go index c9f90e8..18135d8 100644 --- a/internal/fusefs/ops_test.go +++ b/internal/fusefs/ops_test.go @@ -21,6 +21,7 @@ type fakeBatchHydrator struct { type generationSnapshot struct { nodes map[int64]map[string]model.BaseNode + kids map[int64]map[string][]model.BaseNode } type blockingCopyOverlay struct { @@ -61,8 +62,8 @@ func (g *generationSnapshot) GetNode(gen int64, path string) (model.BaseNode, bo return n, ok } -func (g *generationSnapshot) ListChildren(int64, string) ([]model.BaseNode, error) { - return nil, nil +func (g *generationSnapshot) ListChildren(gen int64, path string) ([]model.BaseNode, error) { + return g.kids[gen][path], nil } func (o *blockingCopyOverlay) EnsureCopyOnWrite(_ context.Context, _ model.RepoConfig, path string, base model.BaseNode) (model.OverlayEntry, error) { diff --git a/internal/fusefs/readsymlink_unix_test.go b/internal/fusefs/readsymlink_unix_test.go index 453697b..2ebad2c 100644 --- a/internal/fusefs/readsymlink_unix_test.go +++ b/internal/fusefs/readsymlink_unix_test.go @@ -58,7 +58,7 @@ func TestReadSymlinkRejectsKnownOversizedBlobBeforeHydration(t *testing.T) { fs := NewArtifactFuse(model.RepoConfig{ID: repoID}, resolver, &Engine{Hydrator: hydrator}) fs.mu.Lock() - ref := fs.allocInode("link", "symlink", 0o120000) + ref := fs.allocInode("link", "symlink", 0o120000, fs.resolver.Generation()) fs.mu.Unlock() op := &fuseops.ReadSymlinkOp{Inode: ref.ID} @@ -97,7 +97,7 @@ func TestReadSymlinkRejectsNegativeKnownBlobBeforeHydration(t *testing.T) { fs := NewArtifactFuse(model.RepoConfig{ID: repoID}, resolver, &Engine{Hydrator: hydrator}) fs.mu.Lock() - ref := fs.allocInode("link", "symlink", 0o120000) + ref := fs.allocInode("link", "symlink", 0o120000, fs.resolver.Generation()) fs.mu.Unlock() op := &fuseops.ReadSymlinkOp{Inode: ref.ID} @@ -135,7 +135,7 @@ func TestReadSymlinkRejectsUnknownOversizedBlobWithoutHydration(t *testing.T) { fs := NewArtifactFuse(model.RepoConfig{ID: repoID}, resolver, &Engine{Hydrator: hydrator}) fs.mu.Lock() - ref := fs.allocInode("link", "symlink", 0o120000) + ref := fs.allocInode("link", "symlink", 0o120000, fs.resolver.Generation()) fs.mu.Unlock() op := &fuseops.ReadSymlinkOp{Inode: ref.ID} @@ -173,7 +173,7 @@ func TestReadSymlinkReadsUnknownBlobThroughBoundedRead(t *testing.T) { fs := NewArtifactFuse(model.RepoConfig{ID: repoID}, resolver, &Engine{Hydrator: hydrator}) fs.mu.Lock() - ref := fs.allocInode("link", "symlink", 0o120000) + ref := fs.allocInode("link", "symlink", 0o120000, fs.resolver.Generation()) fs.mu.Unlock() op := &fuseops.ReadSymlinkOp{Inode: ref.ID} diff --git a/internal/gitstore/gitstore.go b/internal/gitstore/gitstore.go index b5ddc6b..ba97d6c 100644 --- a/internal/gitstore/gitstore.go +++ b/internal/gitstore/gitstore.go @@ -825,6 +825,83 @@ func (s *Store) ReadTreeHEAD(ctx context.Context, repo model.RepoConfig) error { return err } +func (s *Store) ConfigureStatusOptimization(ctx context.Context, repo model.RepoConfig, stateRoot string) error { + exe, err := os.Executable() + if err != nil { + return err + } + hookDir := filepath.Join(repo.GitDir, "hooks") + if err := os.MkdirAll(hookDir, 0o755); err != nil { + return err + } + hookPath := filepath.Join(hookDir, "artifact-fs-fsmonitor") + script := fsmonitorHookScript(stateRoot, exe, repo.Name) + if err := os.WriteFile(hookPath, []byte(script), 0o755); err != nil { + return err + } + if _, err := runGit(ctx, repo.GitDir, "config", "core.fsmonitor", hookPath); err != nil { + return err + } + if _, err := runGit(ctx, repo.GitDir, "config", "fsmonitor.allowRemote", "true"); err != nil { + return err + } + workTreeEnv := gitWorkTreeEnv(repo.MountPath) + if _, err := runGitWithEnv(ctx, repo.GitDir, workTreeEnv, "update-index", "--fsmonitor"); err != nil { + return err + } + return markIndexFSMonitorValid(ctx, repo.GitDir, repo.MountPath) +} + +func gitWorkTreeEnv(workTree string) []string { + if strings.TrimSpace(workTree) == "" { + return nil + } + return []string{"GIT_WORK_TREE=" + workTree} +} + +func markIndexFSMonitorValid(ctx context.Context, gitDir, workTree string) error { + env := append(os.Environ(), "GIT_DIR="+gitDir) + env = append(env, gitWorkTreeEnv(workTree)...) + ls := exec.CommandContext(ctx, "git", "ls-files", "-z") + ls.Env = env + stdout, err := ls.StdoutPipe() + if err != nil { + return err + } + lsErr := &bytes.Buffer{} + ls.Stderr = lsErr + update := exec.CommandContext(ctx, "git", "update-index", "--fsmonitor-valid", "-z", "--stdin") + update.Env = env + update.Stdin = stdout + updateErr := &bytes.Buffer{} + update.Stderr = updateErr + if err := ls.Start(); err != nil { + return err + } + if err := update.Start(); err != nil { + _ = ls.Process.Kill() + _ = ls.Wait() + return err + } + upErr := update.Wait() + lsWaitErr := ls.Wait() + if lsWaitErr != nil { + msg := auth.RedactString(strings.TrimSpace(lsErr.String())) + if msg == "" { + msg = auth.RedactString(lsWaitErr.Error()) + } + return errors.New(msg) + } + if upErr != nil { + msg := auth.RedactString(strings.TrimSpace(updateErr.String())) + if msg == "" { + msg = auth.RedactString(upErr.Error()) + } + return errors.New(msg) + } + return nil +} + func (s *Store) ComputeAheadBehind(ctx context.Context, repo model.RepoConfig) (ahead int, behind int, diverged bool, err error) { rangeSpec := fmt.Sprintf("HEAD...origin/%s", repo.Branch) out, err := runGit(ctx, repo.GitDir, "rev-list", "--left-right", "--count", rangeSpec) @@ -1166,6 +1243,20 @@ func shellQuote(s string) string { return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" } +func fsmonitorHookScript(stateRoot, exe, repoName string) string { + return fmt.Sprintf("#!/bin/sh\nARTIFACT_FS_ROOT=%s exec %s fsmonitor-hook --name %s \"$@\"\n", shellScriptQuote(stateRoot), shellScriptQuote(exe), shellScriptQuote(repoName)) +} + +func shellScriptQuote(s string) string { + if s == "" { + return "''" + } + if isShellSafeScriptValue(s) { + return s + } + return "'" + strings.ReplaceAll(s, "'", "'\\''") + "'" +} + func doubleQuote(s string) string { var b strings.Builder b.WriteByte('"') @@ -1198,6 +1289,19 @@ func isShellSafe(s string) bool { return true } +func isShellSafeScriptValue(s string) bool { + for _, r := range s { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') { + continue + } + if strings.ContainsRune("@%_+=:,./-~", r) { + continue + } + return false + } + return true +} + func fetchRefTarget(repo model.RepoConfig, ref string) (fetchRefInfo, error) { ref = strings.TrimSpace(ref) if ref == "" { diff --git a/internal/gitstore/gitstore_test.go b/internal/gitstore/gitstore_test.go index c281ec4..a4802c6 100644 --- a/internal/gitstore/gitstore_test.go +++ b/internal/gitstore/gitstore_test.go @@ -52,6 +52,51 @@ func TestResolveHEADAndBuildTreeIndex(t *testing.T) { } } +func TestFSMonitorHookScriptQuotesArgs(t *testing.T) { + t.Parallel() + tmp := t.TempDir() + pwned := filepath.Join(tmp, "pwned") + hookPath := filepath.Join(tmp, "artifact-fs-fsmonitor") + script := fsmonitorHookScript(tmp, "/bin/false", "repo$(touch "+pwned+")") + if err := os.WriteFile(hookPath, []byte(script), 0o755); err != nil { + t.Fatalf("write hook: %v", err) + } + _ = exec.Command("sh", hookPath).Run() + if _, err := os.Stat(pwned); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("generated hook allowed shell command substitution; stat err = %v", err) + } +} + +func TestMarkIndexFSMonitorValidUsesWorkTree(t *testing.T) { + t.Parallel() + tmp := t.TempDir() + repo := filepath.Join(tmp, "repo") + run(t, "git", "init", repo) + os.WriteFile(filepath.Join(repo, "README.md"), []byte("hello"), 0o644) + run(t, "git", "-C", repo, "add", "README.md") + run(t, "git", "-C", repo, "-c", "user.name=test", "-c", "user.email=test@example.com", "commit", "-m", "init") + gitDir := filepath.Join(repo, ".git") + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if _, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "config", "core.fsmonitor", "true"); err != nil { + t.Fatalf("config core.fsmonitor: %v", err) + } + if _, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "update-index", "--fsmonitor"); err != nil { + t.Fatalf("update-index --fsmonitor: %v", err) + } + + if err := markIndexFSMonitorValid(ctx, gitDir, repo); err != nil { + t.Fatalf("markIndexFSMonitorValid: %v", err) + } + out, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "ls-files", "-f", "README.md") + if err != nil { + t.Fatalf("ls-files -f: %v", err) + } + if !strings.HasPrefix(out, "h ") { + t.Fatalf("ls-files -f = %q, want fsmonitor-clean lowercase h", out) + } +} + func TestBlobToCacheBinarySafe(t *testing.T) { t.Parallel() tmp := t.TempDir() diff --git a/internal/overlay/store.go b/internal/overlay/store.go index 75c014b..391ec19 100644 --- a/internal/overlay/store.go +++ b/internal/overlay/store.go @@ -542,6 +542,10 @@ func (s *Store) DirtyCount(ctx context.Context) (int64, error) { return c, err } +func (s *Store) ListAll(ctx context.Context) ([]model.OverlayEntry, error) { + return s.queryEntries(ctx, `SELECT `+overlayCols+` FROM overlay_entries ORDER BY path`) +} + // ListByPrefix returns overlay entries that are direct children of the given // directory path. Uses path + "/" prefix to avoid matching sibling directories. func (s *Store) ListByPrefix(ctx context.Context, prefix string) ([]model.OverlayEntry, error) { From c17950b6e3876df4fa65aca2fcd92823558986fe Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Mon, 29 Jun 2026 12:53:47 -0400 Subject: [PATCH 2/3] fix fsmonitor test portability --- internal/gitstore/gitstore_test.go | 68 ++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/internal/gitstore/gitstore_test.go b/internal/gitstore/gitstore_test.go index a4802c6..2ad2c16 100644 --- a/internal/gitstore/gitstore_test.go +++ b/internal/gitstore/gitstore_test.go @@ -68,32 +68,64 @@ func TestFSMonitorHookScriptQuotesArgs(t *testing.T) { } func TestMarkIndexFSMonitorValidUsesWorkTree(t *testing.T) { - t.Parallel() tmp := t.TempDir() - repo := filepath.Join(tmp, "repo") - run(t, "git", "init", repo) - os.WriteFile(filepath.Join(repo, "README.md"), []byte("hello"), 0o644) - run(t, "git", "-C", repo, "add", "README.md") - run(t, "git", "-C", repo, "-c", "user.name=test", "-c", "user.email=test@example.com", "commit", "-m", "init") - gitDir := filepath.Join(repo, ".git") + binDir := filepath.Join(tmp, "bin") + if err := os.MkdirAll(binDir, 0o755); err != nil { + t.Fatal(err) + } + recordPath := filepath.Join(tmp, "env.log") + stdinPath := filepath.Join(tmp, "stdin.bin") + expectedStdinPath := filepath.Join(tmp, "expected-stdin.bin") + if err := os.WriteFile(expectedStdinPath, []byte("README.md\x00"), 0o644); err != nil { + t.Fatalf("write expected stdin: %v", err) + } + fakeGit := filepath.Join(binDir, "git") + fakeGitScript := "#!/bin/sh\n" + + "if [ \"$GIT_DIR\" != \"$AFS_TEST_GIT_DIR\" ]; then printf 'bad GIT_DIR: %s\\n' \"$GIT_DIR\" >&2; exit 2; fi\n" + + "if [ \"$GIT_WORK_TREE\" != \"$AFS_TEST_WORK_TREE\" ]; then printf 'bad GIT_WORK_TREE: %s\\n' \"$GIT_WORK_TREE\" >&2; exit 2; fi\n" + + "case \"$1\" in\n" + + " ls-files)\n" + + " if [ \"$#\" -ne 2 ] || [ \"$2\" != \"-z\" ]; then printf 'bad ls-files args: %s\\n' \"$*\" >&2; exit 2; fi\n" + + " printf 'README.md\\0'\n" + + " printf 'ls-files\\n' >>\"$AFS_TEST_RECORD\"\n" + + " exit 0\n" + + " ;;\n" + + " update-index)\n" + + " if [ \"$#\" -ne 4 ] || [ \"$2\" != \"--fsmonitor-valid\" ] || [ \"$3\" != \"-z\" ] || [ \"$4\" != \"--stdin\" ]; then printf 'bad update-index args: %s\\n' \"$*\" >&2; exit 2; fi\n" + + " cat >\"$AFS_TEST_STDIN\"\n" + + " if ! cmp -s \"$AFS_TEST_STDIN\" \"$AFS_TEST_EXPECTED_STDIN\"; then printf 'bad update-index stdin\\n' >&2; exit 2; fi\n" + + " printf 'update-index\\n' >>\"$AFS_TEST_RECORD\"\n" + + " exit 0\n" + + " ;;\n" + + "esac\n" + + "printf 'unexpected git command: %s\\n' \"$*\" >&2\n" + + "exit 2\n" + if err := os.WriteFile(fakeGit, []byte(fakeGitScript), 0o755); err != nil { + t.Fatalf("write fake git: %v", err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("AFS_TEST_RECORD", recordPath) + t.Setenv("AFS_TEST_STDIN", stdinPath) + t.Setenv("AFS_TEST_EXPECTED_STDIN", expectedStdinPath) ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if _, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "config", "core.fsmonitor", "true"); err != nil { - t.Fatalf("config core.fsmonitor: %v", err) - } - if _, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "update-index", "--fsmonitor"); err != nil { - t.Fatalf("update-index --fsmonitor: %v", err) - } - if err := markIndexFSMonitorValid(ctx, gitDir, repo); err != nil { + gitDir := filepath.Join(tmp, "git") + workTree := filepath.Join(tmp, "worktree") + t.Setenv("AFS_TEST_GIT_DIR", gitDir) + t.Setenv("AFS_TEST_WORK_TREE", workTree) + if err := markIndexFSMonitorValid(ctx, gitDir, workTree); err != nil { t.Fatalf("markIndexFSMonitorValid: %v", err) } - out, err := runGitWithEnv(ctx, gitDir, gitWorkTreeEnv(repo), "ls-files", "-f", "README.md") + recorded, err := os.ReadFile(recordPath) if err != nil { - t.Fatalf("ls-files -f: %v", err) + t.Fatalf("read env log: %v", err) } - if !strings.HasPrefix(out, "h ") { - t.Fatalf("ls-files -f = %q, want fsmonitor-clean lowercase h", out) + got := strings.Fields(string(recorded)) + slices.Sort(got) + want := []string{"ls-files", "update-index"} + if !slices.Equal(got, want) { + t.Fatalf("recorded commands = %#v, want %#v", got, want) } } From 19fc66cc631b735c2579586a2b8918e39e51fd51 Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Tue, 30 Jun 2026 12:06:45 -0400 Subject: [PATCH 3/3] mark fsmonitor clean after head update --- internal/daemon/daemon.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 6201d0b..7bf0913 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -977,7 +977,6 @@ func (s *Service) onHEADChanged(ctx context.Context, rt *repoRuntime) { if err := s.git.ReadTreeHEAD(ctx, rt.cfg); err != nil { s.logger.Warn("read-tree HEAD failed", "repo", rt.cfg.Name, "error", err) } - s.configureStatusOptimization(ctx, rt.cfg) s.refreshCommitTime(ctx, rt.cfg, oid, rt.resolver, "commit timestamp unavailable") // Atomically update the resolver's generation so FUSE ops see the new snapshot @@ -985,6 +984,7 @@ func (s *Service) onHEADChanged(ctx context.Context, rt *repoRuntime) { s.mu.Lock() setHeadState(&rt.state, oid, ref, gen) s.mu.Unlock() + s.configureStatusOptimization(ctx, rt.cfg) } func (s *Service) configureStatusOptimization(ctx context.Context, cfg model.RepoConfig) {