From a605431a10298f30ab4c7f06b23a3cede2e19848 Mon Sep 17 00:00:00 2001 From: michel Date: Mon, 20 Jul 2026 20:47:23 +0200 Subject: [PATCH] feat: multiple shared directories Shares::scan_many merges several roots into one index, suffixing duplicate root names so virtual paths cannot collide, and skipping unreadable roots instead of failing. ClientSettings now takes shared_directories: Vec; config gains a shared_dirs list (combined with the single shared_dir and --shared-dir, deduped), with an explicitly empty value still disabling sharing and the share-what-you-download default unchanged. --- soulseek-rs-lib/src/client/connection.rs | 40 +++--- soulseek-rs-lib/src/client/mod.rs | 18 +-- soulseek-rs-lib/src/shares/mod.rs | 171 ++++++++++++++++++----- soulseek-rs-lib/tests/e2e.rs | 8 +- soulseek-rs/src/config.rs | 2 +- soulseek-rs/src/directories.rs | 14 ++ soulseek-rs/src/main.rs | 34 ++--- soulseek-rs/src/persist/config.rs | 96 ++++++++++--- soulseek-rs/src/ui/main_tui/render.rs | 11 +- 9 files changed, 277 insertions(+), 117 deletions(-) diff --git a/soulseek-rs-lib/src/client/connection.rs b/soulseek-rs-lib/src/client/connection.rs index 5c89383..987ae66 100644 --- a/soulseek-rs-lib/src/client/connection.rs +++ b/soulseek-rs-lib/src/client/connection.rs @@ -23,27 +23,25 @@ impl Client { let listen_sender = sender.clone(); - // Scan the shared directory once into the read-only index, and report - // the real folder/file counts to the server on login. - let shares = match self.shared_directory.as_deref() { - Some(dir) if !dir.trim().is_empty() => { - match Shares::scan(std::path::Path::new(dir)) { - Ok(scanned) => { - info!( - "Sharing {} files in {} folders from {}", - scanned.file_count(), - scanned.folder_count(), - dir - ); - Arc::new(scanned) - } - Err(e) => { - warn!("Failed to scan shared directory {}: {}", dir, e); - Arc::new(Shares::empty()) - } - } - } - _ => Arc::new(Shares::empty()), + // Scan the shared directories once into the read-only index, and + // report the real folder/file counts to the server on login. + let roots: Vec = self + .shared_directories + .iter() + .filter(|dir| !dir.trim().is_empty()) + .map(std::path::PathBuf::from) + .collect(); + let shares = if roots.is_empty() { + Arc::new(Shares::empty()) + } else { + let scanned = Shares::scan_many(&roots); + info!( + "Sharing {} files in {} folders from {} directories", + scanned.file_count(), + scanned.folder_count(), + roots.len() + ); + Arc::new(scanned) }; let shared_folder_count = shares.folder_count(); let shared_file_count = shares.file_count(); diff --git a/soulseek-rs-lib/src/client/mod.rs b/soulseek-rs-lib/src/client/mod.rs index 7253656..9e04cf2 100644 --- a/soulseek-rs-lib/src/client/mod.rs +++ b/soulseek-rs-lib/src/client/mod.rs @@ -97,9 +97,9 @@ pub struct ClientSettings { pub server_address: PeerAddress, pub enable_listen: bool, pub listen_port: u16, - /// Directory whose files are shared with (uploaded to) other peers. - /// `None` means nothing is shared. - pub shared_directory: Option, + /// Directories whose files are shared with (uploaded to) other peers. + /// Empty means nothing is shared. + pub shared_directories: Vec, } impl ClientSettings { @@ -126,7 +126,7 @@ impl Default for ClientSettings { ), enable_listen: true, listen_port: DEFAULT_LISTEN_PORT, - shared_directory: None, + shared_directories: Vec::new(), } } } @@ -617,7 +617,7 @@ pub struct Client { address: PeerAddress, username: String, password: String, - shared_directory: Option, + shared_directories: Vec, server_handle: Option>, context: Arc>, } @@ -639,16 +639,16 @@ impl Client { address: settings.server_address, username: settings.username, password: settings.password, - shared_directory: settings.shared_directory, + shared_directories: settings.shared_directories, context: Arc::new(RwLock::new(ClientContext::new())), server_handle: None, } } - /// The directory whose files are shared with other peers, if configured. + /// The directories whose files are shared with other peers. #[must_use] - pub fn shared_directory(&self) -> Option<&str> { - self.shared_directory.as_deref() + pub fn shared_directories(&self) -> &[String] { + &self.shared_directories } } diff --git a/soulseek-rs-lib/src/shares/mod.rs b/soulseek-rs-lib/src/shares/mod.rs index 437a48a..a14d539 100644 --- a/soulseek-rs-lib/src/shares/mod.rs +++ b/soulseek-rs-lib/src/shares/mod.rs @@ -44,55 +44,55 @@ impl Shares { // Fail fast if the root is unreadable; deeper failures are skipped. let _ = std::fs::read_dir(root)?; - let root_name = root.file_name().map_or_else( - || "shared".to_string(), - |n| n.to_string_lossy().into_owned(), - ); - let mut files = Vec::new(); - let mut folders_with_files: HashSet = HashSet::new(); - let mut stack = vec![root.to_path_buf()]; + let mut folder_count = 0; + scan_root(root, &root_display_name(root), &mut files, &mut folder_count); + Ok(Self::from_files(files, folder_count)) + } - while let Some(dir) = stack.pop() { - let Ok(entries) = std::fs::read_dir(&dir) else { + /// Scan several roots into one merged index. Unreadable roots are + /// skipped (with a log line) instead of failing the whole scan, and + /// roots that share a directory name get a ` (2)`, ` (3)`… suffix so + /// their virtual paths cannot collide. + #[must_use] + pub fn scan_many(roots: &[PathBuf]) -> Self { + let mut files = Vec::new(); + let mut folder_count = 0; + let mut name_uses: HashMap = HashMap::new(); + + for root in roots { + if std::fs::read_dir(root).is_err() { + crate::warn!( + "Skipping unreadable shared directory {}", + root.display() + ); continue; - }; - for entry in entries.flatten() { - let Ok(meta) = entry.metadata() else { - continue; - }; - // `entry.metadata()` does not traverse the symlink itself. - if meta.file_type().is_symlink() { - continue; - } - let path = entry.path(); - if meta.is_dir() { - stack.push(path); - } else if meta.is_file() { - let virtual_path = - virtual_path_for(&root_name, root, &path); - files.push(SharedFile { - virtual_path, - real_path: path, - size: meta.len(), - attributes: Vec::new(), - }); - folders_with_files.insert(dir.clone()); - } } + let base = root_display_name(root); + let uses = name_uses.entry(base.clone()).or_insert(0); + *uses += 1; + let name = if *uses == 1 { + base + } else { + format!("{base} ({uses})") + }; + scan_root(root, &name, &mut files, &mut folder_count); } + Self::from_files(files, folder_count) + } + + fn from_files(files: Vec, folder_count: usize) -> Self { let by_virtual = files .iter() .enumerate() .map(|(i, f)| (f.virtual_path.clone(), i)) .collect(); - - Ok(Self { + Self { files, by_virtual, - folder_count: folders_with_files.len(), - }) + folder_count, + } } /// Files whose virtual path contains *every* whitespace-separated term of @@ -159,6 +159,54 @@ impl Shares { } } +fn root_display_name(root: &Path) -> String { + root.file_name().map_or_else( + || "shared".to_string(), + |n| n.to_string_lossy().into_owned(), + ) +} + +/// Recursively walk `root`, appending its files (under the virtual root name +/// `root_name`) to `files` and counting folders that contain files. +fn scan_root( + root: &Path, + root_name: &str, + files: &mut Vec, + folder_count: &mut usize, +) { + let mut folders_with_files: HashSet = HashSet::new(); + let mut stack = vec![root.to_path_buf()]; + + while let Some(dir) = stack.pop() { + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let Ok(meta) = entry.metadata() else { + continue; + }; + // `entry.metadata()` does not traverse the symlink itself. + if meta.file_type().is_symlink() { + continue; + } + let path = entry.path(); + if meta.is_dir() { + stack.push(path); + } else if meta.is_file() { + let virtual_path = virtual_path_for(root_name, root, &path); + files.push(SharedFile { + virtual_path, + real_path: path, + size: meta.len(), + attributes: Vec::new(), + }); + folders_with_files.insert(dir.clone()); + } + } + } + *folder_count += folders_with_files.len(); +} + /// Build the peer-facing virtual path for `path` under `root`: the root's own /// name followed by the backslash-separated components relative to it. fn virtual_path_for(root_name: &str, root: &Path, path: &Path) -> String { @@ -226,6 +274,57 @@ mod tests { let _ = std::fs::remove_dir_all(root); } + #[test] + fn scan_many_merges_roots() { + let root_a = temp_tree(); + let root_b = temp_tree(); + let shares = + Shares::scan_many(&[root_a.clone(), root_b.clone()]); + assert_eq!(shares.file_count(), 6); + assert_eq!(shares.folder_count(), 4); + // Files from both roots are reachable under their root's name. + let name_a = root_a.file_name().unwrap().to_string_lossy(); + let name_b = root_b.file_name().unwrap().to_string_lossy(); + assert!(shares.get(&format!("{name_a}\\top.mp3")).is_some()); + assert!(shares.get(&format!("{name_b}\\top.mp3")).is_some()); + let _ = std::fs::remove_dir_all(root_a); + let _ = std::fs::remove_dir_all(root_b); + } + + #[test] + fn scan_many_disambiguates_equal_root_names() { + // Two different directories both named "music". + let parent_a = temp_tree(); + let parent_b = temp_tree(); + let music_a = parent_a.join("music"); + let music_b = parent_b.join("music"); + std::fs::create_dir_all(&music_a).unwrap(); + std::fs::create_dir_all(&music_b).unwrap(); + std::fs::write(music_a.join("a.mp3"), b"a").unwrap(); + std::fs::write(music_b.join("b.mp3"), b"b").unwrap(); + + let shares = + Shares::scan_many(&[music_a.clone(), music_b.clone()]); + assert!(shares.get("music\\a.mp3").is_some()); + assert!( + shares.get("music (2)\\b.mp3").is_some(), + "second root with the same name gets a numbered suffix" + ); + let _ = std::fs::remove_dir_all(parent_a); + let _ = std::fs::remove_dir_all(parent_b); + } + + #[test] + fn scan_many_skips_unreadable_roots() { + let root = temp_tree(); + let shares = Shares::scan_many(&[ + std::path::PathBuf::from("/does/not/exist"), + root.clone(), + ]); + assert_eq!(shares.file_count(), 3); + let _ = std::fs::remove_dir_all(root); + } + #[test] fn empty_shares_have_no_files_or_folders() { let shares = Shares::empty(); diff --git a/soulseek-rs-lib/tests/e2e.rs b/soulseek-rs-lib/tests/e2e.rs index 63c89ce..6be0b81 100644 --- a/soulseek-rs-lib/tests/e2e.rs +++ b/soulseek-rs-lib/tests/e2e.rs @@ -96,7 +96,7 @@ impl TestServer { server_address: PeerAddress::new(self.host.clone(), self.port), enable_listen: false, listen_port: 0, - shared_directory: None, + shared_directories: Vec::new(), } } @@ -1164,7 +1164,7 @@ fn two_real_clients_search_and_download() { let sharer_port = free_port().expect("sharer port"); let mut sharer = Client::with_settings(ClientSettings { - shared_directory: Some(share_dir.display().to_string()), + shared_directories: vec![share_dir.display().to_string()], ..server.listening_settings("e2e_sharer", "pw", sharer_port) }); sharer.connect().expect("sharer connect"); @@ -1260,7 +1260,7 @@ fn browse_a_peers_shared_files() { let sharer_port = free_port().expect("sharer port"); let mut sharer = Client::with_settings(ClientSettings { - shared_directory: Some(share_dir.display().to_string()), + shared_directories: vec![share_dir.display().to_string()], ..server.listening_settings("e2e_browsee", "pw", sharer_port) }); sharer.connect().expect("sharer connect"); @@ -1316,7 +1316,7 @@ fn browse_a_firewalled_peer_via_broker() { // Sharer does NOT listen (firewalled): the browser's direct dial will fail, // forcing the server-brokered connect-back. let mut sharer = Client::with_settings(ClientSettings { - shared_directory: Some(share_dir.display().to_string()), + shared_directories: vec![share_dir.display().to_string()], ..server.settings("e2e_fw_sharer", "pw") }); sharer.connect().expect("sharer connect"); diff --git a/soulseek-rs/src/config.rs b/soulseek-rs/src/config.rs index 6bc0c3a..28f720b 100644 --- a/soulseek-rs/src/config.rs +++ b/soulseek-rs/src/config.rs @@ -10,5 +10,5 @@ pub struct SearchConfig { pub download_dir: String, pub verbose: u8, pub max_concurrent_downloads: usize, - pub shared_directory: Option, + pub shared_directories: Vec, } diff --git a/soulseek-rs/src/directories.rs b/soulseek-rs/src/directories.rs index b90dd11..e048d19 100644 --- a/soulseek-rs/src/directories.rs +++ b/soulseek-rs/src/directories.rs @@ -35,6 +35,20 @@ pub fn resolve_shared_directory( Ok(Some(path)) } +/// Validate several configured share paths; invalid ones are skipped with a +/// warning instead of aborting (the client just shares less). +pub fn resolve_shared_directories(dirs: &[String]) -> Vec { + let mut resolved = Vec::new(); + for dir in dirs { + match resolve_shared_directory(Some(dir)) { + Ok(Some(path)) => resolved.push(path.display().to_string()), + Ok(None) => {} + Err(e) => eprintln!("⚠️ Ignoring shared directory: {e}"), + } + } + resolved +} + #[cfg(test)] mod tests { use super::resolve_shared_directory; diff --git a/soulseek-rs/src/main.rs b/soulseek-rs/src/main.rs index 87646d1..6272ff2 100644 --- a/soulseek-rs/src/main.rs +++ b/soulseek-rs/src/main.rs @@ -72,17 +72,10 @@ fn main() -> Result<()> { let (server_host, server_port) = parse_server_address(&resolved.server)?; - // Resolve the optional shared/upload directory up front; a misconfigured - // one is a warning, not a fatal error (the client just shares nothing). - let shared_directory = match directories::resolve_shared_directory( - resolved.shared_dir.as_deref(), - ) { - Ok(dir) => dir.map(|path| path.display().to_string()), - Err(e) => { - eprintln!("⚠️ Ignoring shared directory: {e}"); - None - } - }; + // Resolve the configured shared/upload directories up front; a + // misconfigured one is a warning, not a fatal error. + let shared_directories = + directories::resolve_shared_directories(&resolved.shared_dirs); let settings = ClientSettings { username: username.clone(), @@ -90,7 +83,7 @@ fn main() -> Result<()> { server_address: PeerAddress::new(server_host.clone(), server_port), enable_listen: !resolved.disable_listener, listen_port: resolved.listener_port, - shared_directory: shared_directory.clone(), + shared_directories: shared_directories.clone(), }; match cli.command { @@ -112,7 +105,7 @@ fn main() -> Result<()> { download_dir, verbose: cli.verbose, max_concurrent_downloads, - shared_directory, + shared_directories, }; search_and_download(config) } @@ -182,15 +175,8 @@ fn run_default_tui( ); } - let shared_directory = match directories::resolve_shared_directory( - resolved.shared_dir.as_deref(), - ) { - Ok(dir) => dir.map(|path| path.display().to_string()), - Err(e) => { - eprintln!("⚠️ Ignoring shared directory: {e}"); - None - } - }; + let shared_directories = + directories::resolve_shared_directories(&resolved.shared_dirs); let secret_store = persist::secret::KeyringStore; let initial_password = persist::secret::resolve_password( @@ -217,7 +203,7 @@ fn run_default_tui( server_address: PeerAddress::new(server_host.clone(), server_port), enable_listen, listen_port, - shared_directory: shared_directory.clone(), + shared_directories: shared_directories.clone(), }; // Clear screen and enable mouse capture before initializing TUI @@ -475,7 +461,7 @@ fn search_and_download(config: SearchConfig) -> Result<()> { ), enable_listen: config.enable_listener, listen_port: config.listener_port, - shared_directory: config.shared_directory.clone(), + shared_directories: config.shared_directories.clone(), }; let _port_mapper = settings diff --git a/soulseek-rs/src/persist/config.rs b/soulseek-rs/src/persist/config.rs index da5d91a..356feaa 100644 --- a/soulseek-rs/src/persist/config.rs +++ b/soulseek-rs/src/persist/config.rs @@ -13,7 +13,11 @@ pub struct FileConfig { pub listener_port: Option, pub disable_listener: Option, pub download_dir: Option, + /// Single shared folder (also what `--shared-dir` sets). Prefer + /// `shared_dirs` for multiple; both may be combined. pub shared_dir: Option, + /// Multiple shared folders. An explicitly empty list disables sharing. + pub shared_dirs: Option>, pub max_concurrent_downloads: Option, pub search_timeout: Option, /// Command whose stdout is the password (headless fallback, like mutt's @@ -61,7 +65,7 @@ pub struct Resolved { pub listener_port: u16, pub disable_listener: bool, pub download_dir: String, - pub shared_dir: Option, + pub shared_dirs: Vec, pub max_concurrent_downloads: usize, pub search_timeout: u64, pub password_cmd: Option, @@ -98,14 +102,7 @@ pub fn resolve(cli: &crate::cli::Cli, file: &FileConfig) -> Resolved { disable_listener: cli.disable_listener || file.disable_listener.unwrap_or(false), download_dir: download_dir.clone(), - // Soulseek convention: share what you download. An explicitly empty - // shared_dir ("") opts out of sharing entirely. - shared_dir: cli - .shared_dir - .clone() - .or_else(|| file.shared_dir.clone()) - .or(Some(download_dir)) - .filter(|dir| !dir.trim().is_empty()), + shared_dirs: resolve_shared_dirs(cli, file, &download_dir), max_concurrent_downloads: cli .max_concurrent_downloads .or(file.max_concurrent_downloads) @@ -118,6 +115,33 @@ pub fn resolve(cli: &crate::cli::Cli, file: &FileConfig) -> Resolved { } } +/// Sharing follows the Soulseek convention of sharing what you download: +/// with nothing configured, the download folder is shared. Configuring any +/// of `--shared-dir` / `shared_dir` / `shared_dirs` replaces that default +/// (their non-empty values are combined), and an explicitly empty value +/// (`shared_dir = ""` or `shared_dirs = []`) disables sharing entirely. +fn resolve_shared_dirs( + cli: &crate::cli::Cli, + file: &FileConfig, + download_dir: &str, +) -> Vec { + if cli.shared_dir.is_none() + && file.shared_dir.is_none() + && file.shared_dirs.is_none() + { + return vec![download_dir.to_string()]; + } + let mut dirs: Vec = Vec::new(); + let singles = cli.shared_dir.iter().chain(file.shared_dir.iter()); + for dir in singles.chain(file.shared_dirs.iter().flatten()) { + let dir = dir.trim(); + if !dir.is_empty() && !dirs.iter().any(|d| d == dir) { + dirs.push(dir.to_string()); + } + } + dirs +} + #[cfg(test)] mod tests { use super::*; @@ -169,22 +193,19 @@ mod tests { } #[test] - fn shared_dir_defaults_to_the_download_dir() { + fn sharing_defaults_to_the_download_dir() { let resolved = resolve(&bare_cli(), &FileConfig::default()); - assert_eq!( - resolved.shared_dir.as_deref(), - Some(resolved.download_dir.as_str()) - ); + assert_eq!(resolved.shared_dirs, vec![resolved.download_dir.clone()]); } #[test] - fn shared_dir_follows_a_customized_download_dir() { + fn sharing_follows_a_customized_download_dir() { let file = FileConfig { download_dir: Some("/music".into()), ..FileConfig::default() }; let resolved = resolve(&bare_cli(), &file); - assert_eq!(resolved.shared_dir.as_deref(), Some("/music")); + assert_eq!(resolved.shared_dirs, vec!["/music".to_string()]); } #[test] @@ -194,7 +215,45 @@ mod tests { ..FileConfig::default() }; let resolved = resolve(&bare_cli(), &file); - assert_eq!(resolved.shared_dir, None); + assert!(resolved.shared_dirs.is_empty()); + } + + #[test] + fn empty_shared_dirs_list_disables_sharing() { + let file = FileConfig { + shared_dirs: Some(Vec::new()), + ..FileConfig::default() + }; + let resolved = resolve(&bare_cli(), &file); + assert!(resolved.shared_dirs.is_empty()); + } + + #[test] + fn shared_dirs_list_replaces_the_default_share() { + let file = FileConfig { + shared_dirs: Some(vec!["/a".into(), "/b".into()]), + ..FileConfig::default() + }; + let resolved = resolve(&bare_cli(), &file); + assert_eq!( + resolved.shared_dirs, + vec!["/a".to_string(), "/b".to_string()] + ); + } + + #[test] + fn cli_shared_dir_combines_with_file_list_and_dedupes() { + let mut cli = bare_cli(); + cli.shared_dir = Some("/a".into()); + let file = FileConfig { + shared_dirs: Some(vec!["/a".into(), "/b".into()]), + ..FileConfig::default() + }; + let resolved = resolve(&cli, &file); + assert_eq!( + resolved.shared_dirs, + vec!["/a".to_string(), "/b".to_string()] + ); } #[test] @@ -206,6 +265,7 @@ mod tests { disable_listener: Some(true), download_dir: Some("/music".into()), shared_dir: Some("/shared".into()), + shared_dirs: None, max_concurrent_downloads: Some(2), search_timeout: Some(30), password_cmd: Some("pass show slsk".into()), @@ -216,7 +276,7 @@ mod tests { assert_eq!(resolved.listener_port, 4321); assert!(resolved.disable_listener); assert_eq!(resolved.download_dir, "/music"); - assert_eq!(resolved.shared_dir.as_deref(), Some("/shared")); + assert_eq!(resolved.shared_dirs, vec!["/shared".to_string()]); assert_eq!(resolved.max_concurrent_downloads, 2); assert_eq!(resolved.search_timeout, 30); assert_eq!(resolved.password_cmd.as_deref(), Some("pass show slsk")); diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index b190ce9..a041e87 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -353,10 +353,13 @@ impl MainTui { let shortcuts_line = format_shortcuts_styled(&shortcuts); // Surface our own sharing status in the block title. - let title = format!( - "Shortcuts · Sharing: {}", - self.client.shared_directory().unwrap_or("off") - ); + let shared = self.client.shared_directories(); + let sharing = match shared { + [] => "off".to_string(), + [only] => only.clone(), + more => format!("{} folders", more.len()), + }; + let title = format!("Shortcuts · Sharing: {sharing}"); let shortcuts_widget = Paragraph::new(shortcuts_line).block( Block::default() .borders(Borders::ALL)