From ca7a5a683f24c90e81f1dd8e40e2a786e5e2e27f Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Fri, 6 Feb 2026 18:58:53 -0800 Subject: [PATCH] Simplify to single shared BranchManager, remove per-mount isolation Signed-off-by: Cong Wang --- README.md | 49 ++++++------ src/daemon.rs | 181 +++++++++++++----------------------------- src/main.rs | 21 +---- tests/test_helper.sh | 2 +- tests/test_unmount.sh | 41 +++++----- 5 files changed, 102 insertions(+), 192 deletions(-) diff --git a/README.md b/README.md index 94c93b3..b1bce10 100644 --- a/README.md +++ b/README.md @@ -82,8 +82,8 @@ branchfs create experiment /mnt/workspace cd /mnt/workspace echo "new code" > feature.py -# List branches for this mount -branchfs list /mnt/workspace +# List branches +branchfs list # Commit changes to base (switches back to main, stays mounted) branchfs commit /mnt/workspace @@ -145,35 +145,34 @@ cat /mnt/workspace/@feature-a/@child/file.txt This is useful for multi-agent workflows where each agent can bind-mount a different `@branch` path to work on isolated branches in parallel within the same mount. -### Parallel Speculation (Multiple Mount Points) +### Parallel Speculation (Multiple Agents) -Each mount has its own isolated branch namespace: +With `@branch` virtual paths, multiple agents can work in parallel through a single mount: ```bash -# Mount two isolated workspaces from the same base -branchfs mount --base ~/project /mnt/approach-a -branchfs mount --base ~/project /mnt/approach-b +# Mount once +branchfs mount --base ~/project /mnt/workspace -# Create branches in each (isolated from each other) -branchfs create experiment /mnt/approach-a -branchfs create experiment /mnt/approach-b # same name, different mount = OK +# Create branches for each agent +branchfs create agent-a /mnt/workspace +branchfs create agent-b /mnt/workspace -# Work in parallel... -echo "approach a" > /mnt/approach-a/solution.py -echo "approach b" > /mnt/approach-b/solution.py +# Each agent works via its own @branch path (no switching needed) +echo "approach a" > /mnt/workspace/@agent-a/solution.py +echo "approach b" > /mnt/workspace/@agent-b/solution.py -# Commit one approach -branchfs commit /mnt/approach-a +# Commit one agent's work +echo "commit" > /mnt/workspace/@agent-a/.branchfs_ctl -# approach-b is unaffected (isolated mount) -cat /mnt/approach-b/solution.py # still works +# agent-b is unaffected +cat /mnt/workspace/@agent-b/solution.py # still works ``` ## Semantics -### Per-Mount Isolation +### Shared Branch Namespace -Each mount point has its own isolated branch namespace. Branches created in one mount are not visible to other mounts, even if they share the same base directory. This includes `@branch` virtual paths — `/@feature-a` on one mount has no relation to `/@feature-a` on another mount, even if they share the same base. This enables true parallel speculation without interference. +All mounts share a single branch namespace managed by the daemon. Branches created through any mount are visible from all mounts via `@branch` virtual paths. This simplifies multi-agent workflows — each agent accesses its branch via `/@branch-name/` without needing separate mount points. ### Commit @@ -181,7 +180,7 @@ Committing a branch applies the entire chain of changes to the base filesystem: 1. Changes are collected from the current branch up through its ancestors 2. Deletions are applied first, then file modifications -3. Mount's epoch increments, invalidating all branches in this mount +3. Epoch increments, invalidating all branches across all mounts 4. **Mount automatically switches to main branch** (stays mounted) 5. Memory-mapped regions trigger `SIGBUS` on next access @@ -190,15 +189,13 @@ Committing a branch applies the entire chain of changes to the base filesystem: Aborting discards the entire branch chain without affecting the base: 1. The entire branch chain (current branch up to main) is discarded -2. Sibling branches in the same mount continue operating normally +2. Other branches continue operating normally 3. **Mount automatically switches to main branch** (stays mounted) 4. Memory-mapped regions in aborted branches trigger `SIGBUS` ### Unmount -Unmounting removes the mount and cleans up all its branches: +Unmounting removes the FUSE mount: -1. **All branches for this mount are discarded** (full cleanup) -2. Mount-specific storage is deleted -3. The daemon automatically exits when the last mount is removed -4. Other mounts are unaffected (per-mount isolation) +1. The FUSE session is torn down +2. The daemon automatically exits when the last mount is removed diff --git a/src/daemon.rs b/src/daemon.rs index f94da2a..573b940 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1,7 +1,5 @@ -use std::collections::hash_map::DefaultHasher; use std::collections::HashMap; use std::fs; -use std::hash::{Hash, Hasher}; use std::io::{BufRead, BufReader, Write}; use std::os::unix::net::{UnixListener, UnixStream}; use std::path::{Path, PathBuf}; @@ -21,25 +19,11 @@ use crate::fs::BranchFs; #[derive(Debug, Serialize, Deserialize)] #[serde(tag = "cmd", rename_all = "snake_case")] pub enum Request { - Mount { - branch: String, - mountpoint: String, - }, - Unmount { - mountpoint: String, - }, - Create { - name: String, - parent: String, - mountpoint: String, - }, - NotifySwitch { - mountpoint: String, - branch: String, - }, - List { - mountpoint: String, - }, + Mount { branch: String, mountpoint: String }, + Unmount { mountpoint: String }, + Create { name: String, parent: String }, + NotifySwitch { mountpoint: String, branch: String }, + List, Shutdown, } @@ -78,29 +62,19 @@ impl Response { } } -/// Per-mount state including the FUSE session, current branch, and isolated branch manager +/// Per-mount state including the FUSE session and current branch pub struct MountInfo { session: BackgroundSession, current_branch: String, - manager: Arc, - mount_storage: PathBuf, } pub struct Daemon { - base_path: PathBuf, - storage_path: PathBuf, + manager: Arc, mounts: Mutex>, socket_path: PathBuf, shutdown: AtomicBool, } -/// Generate a hash-based directory name for a mountpoint -fn mount_hash(mountpoint: &Path) -> String { - let mut hasher = DefaultHasher::new(); - mountpoint.hash(&mut hasher); - format!("{:016x}", hasher.finish()) -} - impl Daemon { pub fn new( base_path: PathBuf, @@ -109,7 +83,15 @@ impl Daemon { ) -> Result { let socket_path = storage_path.join("daemon.sock"); - // Clean up orphaned mount directories on startup + // Clean up branches from previous daemon run for fresh state + let branches_dir = storage_path.join("branches"); + if branches_dir.exists() { + if let Err(e) = fs::remove_dir_all(&branches_dir) { + log::warn!("Failed to clean up branches directory: {}", e); + } + } + + // Also clean up legacy mounts directory if present let mounts_dir = storage_path.join("mounts"); if mounts_dir.exists() { if let Err(e) = fs::remove_dir_all(&mounts_dir) { @@ -122,9 +104,15 @@ impl Daemon { fs::create_dir_all(&storage_path)?; fs::write(&base_file, base_path.to_string_lossy().as_bytes())?; + // Create the single shared BranchManager + let manager = Arc::new(BranchManager::new( + storage_path.clone(), + base_path.clone(), + base_path.clone(), + )?); + Ok(Self { - base_path, - storage_path, + manager, mounts: Mutex::new(HashMap::new()), socket_path, shutdown: AtomicBool::new(false), @@ -136,31 +124,16 @@ impl Daemon { } pub fn spawn_mount(&self, branch_name: &str, mountpoint: &Path) -> Result<()> { - // Create mount-specific storage directory - let mount_storage = self - .storage_path - .join("mounts") - .join(mount_hash(mountpoint)); - fs::create_dir_all(&mount_storage)?; - - // Create a new BranchManager for this mount - let manager = Arc::new(BranchManager::new( - mount_storage.clone(), - self.base_path.clone(), - mountpoint.to_path_buf(), - )?); - - let fs = BranchFs::new(manager.clone(), branch_name.to_string()); + let fs = BranchFs::new(self.manager.clone(), branch_name.to_string()); let options = vec![ MountOption::FSName("branchfs".to_string()), MountOption::DefaultPermissions, ]; log::info!( - "Spawning mount for branch '{}' at {:?} with storage {:?}", + "Spawning mount for branch '{}' at {:?}", branch_name, mountpoint, - mount_storage ); let session = @@ -168,13 +141,12 @@ impl Daemon { // Get the notifier for cache invalidation and register it with the manager let notifier = Arc::new(session.notifier()); - manager.register_notifier(branch_name, mountpoint.to_path_buf(), notifier); + self.manager + .register_notifier(branch_name, mountpoint.to_path_buf(), notifier); let mount_info = MountInfo { session, current_branch: branch_name.to_string(), - manager, - mount_storage, }; self.mounts @@ -199,22 +171,9 @@ impl Daemon { } }; - // Clean up mount storage directory (full cleanup on unmount) if let Some(info) = mount_info { - info.manager + self.manager .unregister_notifier(&info.current_branch, mountpoint); - // Delete the entire mount storage directory - if info.mount_storage.exists() { - if let Err(e) = fs::remove_dir_all(&info.mount_storage) { - log::warn!( - "Failed to clean up mount storage {:?}: {}", - info.mount_storage, - e - ); - } else { - log::info!("Cleaned up mount storage {:?}", info.mount_storage); - } - } } if should_shutdown { @@ -230,18 +189,9 @@ impl Daemon { let mountpoints: Vec = mounts.keys().cloned().collect(); for mountpoint in &mountpoints { if let Some(info) = mounts.remove(mountpoint) { - info.manager + self.manager .unregister_notifier(&info.current_branch, mountpoint); // BackgroundSession dropped here → FUSE unmount - if info.mount_storage.exists() { - if let Err(e) = fs::remove_dir_all(&info.mount_storage) { - log::warn!( - "Failed to clean up mount storage {:?}: {}", - info.mount_storage, - e - ); - } - } log::info!("Cleaned up mount at {:?}", mountpoint); } } @@ -251,27 +201,16 @@ impl Daemon { self.mounts.lock().len() } - pub fn create_branch(&self, name: &str, parent: &str, mountpoint: &Path) -> Result<()> { - let mounts = self.mounts.lock(); - let mount_info = mounts - .get(mountpoint) - .ok_or_else(|| crate::error::BranchError::MountNotFound(format!("{:?}", mountpoint)))?; - mount_info.manager.create_branch(name, parent) + pub fn create_branch(&self, name: &str, parent: &str) -> Result<()> { + self.manager.create_branch(name, parent) } - pub fn list_branches(&self, mountpoint: &Path) -> Result)>> { - let mounts = self.mounts.lock(); - let mount_info = mounts - .get(mountpoint) - .ok_or_else(|| crate::error::BranchError::MountNotFound(format!("{:?}", mountpoint)))?; - Ok(mount_info.manager.list_branches()) + pub fn list_branches(&self) -> Vec<(String, Option)> { + self.manager.list_branches() } - pub fn get_manager(&self, mountpoint: &Path) -> Option> { - self.mounts - .lock() - .get(mountpoint) - .map(|info| info.manager.clone()) + pub fn get_manager(&self) -> Arc { + self.manager.clone() } pub fn run(&self) -> Result<()> { @@ -361,29 +300,22 @@ impl Daemon { Err(e) => Response::error(&format!("{}", e)), } } - Request::Create { - name, - parent, - mountpoint, - } => { - let path = PathBuf::from(&mountpoint); - match self.create_branch(&name, &parent, &path) { - Ok(()) => Response::success(), - Err(e) => Response::error(&format!("{}", e)), - } - } + Request::Create { name, parent } => match self.create_branch(&name, &parent) { + Ok(()) => Response::success(), + Err(e) => Response::error(&format!("{}", e)), + }, Request::NotifySwitch { mountpoint, branch } => { let path = PathBuf::from(&mountpoint); let mut mounts = self.mounts.lock(); if let Some(ref mut info) = mounts.get_mut(&path) { // Unregister old notifier - info.manager + self.manager .unregister_notifier(&info.current_branch, &path); // Update tracked branch let old_branch = std::mem::replace(&mut info.current_branch, branch.clone()); // Register notifier for new branch let notifier = Arc::new(info.session.notifier()); - info.manager + self.manager .register_notifier(&branch, path.clone(), notifier); log::info!( "Mount {:?} switched from '{}' to '{}'", @@ -396,23 +328,18 @@ impl Daemon { Response::error(&format!("Mount not found: {:?}", path)) } } - Request::List { mountpoint } => { - let path = PathBuf::from(&mountpoint); - match self.list_branches(&path) { - Ok(branches) => { - let branches: Vec<_> = branches - .into_iter() - .map(|(name, parent)| { - serde_json::json!({ - "name": name, - "parent": parent - }) - }) - .collect(); - Response::success_with_data(serde_json::json!(branches)) - } - Err(e) => Response::error(&format!("{}", e)), - } + Request::List => { + let branches: Vec<_> = self + .list_branches() + .into_iter() + .map(|(name, parent)| { + serde_json::json!({ + "name": name, + "parent": parent + }) + }) + .collect(); + Response::success_with_data(serde_json::json!(branches)) } Request::Shutdown => { log::info!("Shutdown requested, cleaning up all mounts"); diff --git a/src/main.rs b/src/main.rs index b60997e..4a4d2a1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,7 +36,7 @@ enum Commands { /// Branch name name: String, - /// Mount point where the branch should be created + /// Mount point to switch to the new branch mountpoint: PathBuf, /// Parent branch name @@ -68,11 +68,8 @@ enum Commands { storage: PathBuf, }, - /// List branches for a mountpoint + /// List branches List { - /// Mount point to list branches for - mountpoint: PathBuf, - /// Storage directory #[arg(long, default_value = "/var/lib/branchfs")] storage: PathBuf, @@ -154,7 +151,6 @@ fn main() -> Result<()> { &Request::Create { name: name.clone(), parent: parent.clone(), - mountpoint: mountpoint.to_string_lossy().to_string(), }, )?; @@ -251,19 +247,10 @@ fn main() -> Result<()> { println!("Aborted branch at {:?}", mountpoint); } - Commands::List { - mountpoint, - storage, - } => { + Commands::List { storage } => { let storage = storage.canonicalize()?; - let mountpoint = mountpoint.canonicalize()?; - let response = send_request( - &storage, - &Request::List { - mountpoint: mountpoint.to_string_lossy().to_string(), - }, - )?; + let response = send_request(&storage, &Request::List)?; if response.ok { println!("{:<20} {:<20}", "BRANCH", "PARENT"); diff --git a/tests/test_helper.sh b/tests/test_helper.sh index ef94fce..90c23c9 100755 --- a/tests/test_helper.sh +++ b/tests/test_helper.sh @@ -123,7 +123,7 @@ do_abort() { # List branches do_list() { - "$BRANCHFS" list "$TEST_MNT" --storage "$TEST_STORAGE" + "$BRANCHFS" list --storage "$TEST_STORAGE" } # Assert that a condition is true diff --git a/tests/test_unmount.sh b/tests/test_unmount.sh index 3a2b872..a94f4c2 100755 --- a/tests/test_unmount.sh +++ b/tests/test_unmount.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Test unmount functionality (per-mount isolation: full cleanup on unmount) +# Test unmount functionality source "$(dirname "$0")/test_helper.sh" @@ -12,9 +12,6 @@ test_unmount_main() { # Should be unmounted assert "! mountpoint -q '$TEST_MNT'" "Mount point unmounted" - - # With per-mount isolation, mount storage is cleaned up on unmount - # Remounting creates a fresh mount with only main branch } test_unmount_discards_single_branch() { @@ -45,16 +42,15 @@ test_unmount_cleans_all_branches() { do_create "child_branch" "parent_branch" echo "child content" > "$TEST_MNT/child_file.txt" - # Unmount (per-mount isolation: cleans up ALL branches for this mount) + # Unmount — daemon exits, branches cleaned up on next startup do_unmount # Should be unmounted assert "! mountpoint -q '$TEST_MNT'" "Mount point unmounted" - # Remount - starts fresh with only main branch + # Remount - daemon restarts fresh, only main branch do_mount - # Per-mount isolation: all branches are cleaned up, only main exists assert_branch_exists "main" "Main branch exists after remount" assert_branch_not_exists "parent_branch" "Parent branch cleaned up on unmount" assert_branch_not_exists "child_branch" "Child branch cleaned up on unmount" @@ -74,29 +70,32 @@ test_unmount_cleanup() { # Create some files echo "test" > "$TEST_MNT/test.txt" - # Check that mount storage exists before unmount - local mounts_dir="$TEST_STORAGE/mounts" - assert "[[ -d '$mounts_dir' ]]" "Mounts directory exists before unmount" + # Check that branches directory exists before unmount + local branches_dir="$TEST_STORAGE/branches" + assert "[[ -d '$branches_dir' ]]" "Branches directory exists before unmount" - # Get mount count before unmount - local mount_count_before - mount_count_before=$(ls "$mounts_dir" 2>/dev/null | wc -l) - assert "[[ $mount_count_before -gt 0 ]]" "Mount storage exists before unmount" + # Should have branches (main + cleanup_test) + local branch_count_before + branch_count_before=$(ls "$branches_dir" 2>/dev/null | wc -l) + assert "[[ $branch_count_before -gt 0 ]]" "Branches exist before unmount" - # Unmount + # Unmount — daemon exits when last mount removed do_unmount - # Mount-specific storage should be cleaned up - # (mounts directory may still exist but should be empty) - local mount_count_after - mount_count_after=$(ls "$mounts_dir" 2>/dev/null | wc -l) - assert "[[ $mount_count_after -eq 0 ]]" "Mount storage cleaned up on unmount" + # After daemon restart, branches dir is cleaned up + # (daemon cleans branches/ on startup for fresh state) + do_mount + local branch_count_after + branch_count_after=$(ls "$branches_dir" 2>/dev/null | wc -l) + # Only "main" branch should exist after fresh start + assert "[[ $branch_count_after -eq 1 ]]" "Only main branch after remount" + do_unmount } # Run tests run_test "Unmount Main" test_unmount_main run_test "Unmount Discards Single Branch" test_unmount_discards_single_branch -run_test "Unmount Cleans All Branches (Per-Mount Isolation)" test_unmount_cleans_all_branches +run_test "Unmount Cleans All Branches" test_unmount_cleans_all_branches run_test "Unmount Cleanup" test_unmount_cleanup print_summary