diff --git a/native/src/hlcr/renderer.rs b/native/src/hlcr/renderer.rs index b4f445c96..39fa01d61 100644 --- a/native/src/hlcr/renderer.rs +++ b/native/src/hlcr/renderer.rs @@ -1223,8 +1223,8 @@ mod tests { let second = try_reserve(&mut ledger, &with_d, 10, 0, free); assert_eq!(second.map(|r| r.0), Some(PathBuf::from(r"D:\x"))); assert_eq!(try_reserve(&mut ledger, &with_d, 10, 0, free), None); - assert_eq!(ledger.get(Path::new("c:")), Some(&10)); - assert_eq!(ledger.get(Path::new("d:")), Some(&10)); + assert_eq!(ledger.get(&ledger_key(Path::new(r"C:\a"))), Some(&10)); + assert_eq!(ledger.get(&ledger_key(Path::new(r"D:\x"))), Some(&10)); } /// #120: the first clip of a kind reserves the raw estimate; once one diff --git a/native/src/hlcr/take_cleanup.rs b/native/src/hlcr/take_cleanup.rs index 1c3b6d841..96d193a37 100644 --- a/native/src/hlcr/take_cleanup.rs +++ b/native/src/hlcr/take_cleanup.rs @@ -15,7 +15,7 @@ //! touched: the take would be the only copy left. use std::collections::{BTreeMap, HashSet}; -use std::path::{Component, Path, PathBuf}; +use std::path::{Path, PathBuf}; use crate::hlcr::scanner::{ClipData, scan_folder_background}; use crate::hlcr::take_delete::{TakeDeletion, plan_take_deletion}; @@ -169,16 +169,12 @@ pub fn folder_key(path: &str) -> String { .to_string() } -/// The drive a path is on (`d:`, or `\\server\share`), so two Destinations -/// folders on one drive count its free space once. Empty for a path with no -/// drive, which every such path then shares. +/// The volume a path is on (`sys::disk::volume_key`), so two Destinations +/// folders on one drive count its free space once, and a disk mounted as a +/// folder counts apart (#1041). Empty for a path with no drive, which every +/// such path then shares. pub fn volume_of(path: &Path) -> String { - match path.components().next() { - Some(Component::Prefix(prefix)) => { - prefix.as_os_str().to_string_lossy().to_ascii_lowercase() - } - _ => String::new(), - } + crate::sys::disk::volume_key(path).unwrap_or_default() } /// Free space on the drives a job writes to, and the pieces it writes there. @@ -812,12 +808,15 @@ mod tests { } #[test] - fn a_volume_is_the_drive_letter() { + fn folders_on_one_drive_are_one_volume() { if cfg!(windows) { - let mut path = PathBuf::from("D:"); + let mut path = PathBuf::from("C:"); path.push("captures"); path.push("s1"); - assert_eq!(volume_of(&path), "d:"); + let mut other = PathBuf::from("c:"); + other.push("clips"); + assert_eq!(volume_of(&path), volume_of(&other)); + assert!(!volume_of(&path).is_empty()); } assert_eq!(volume_of(Path::new("relative")), ""); } diff --git a/native/src/sys/disk.rs b/native/src/sys/disk.rs index 17bc22944..01eb0e545 100644 --- a/native/src/sys/disk.rs +++ b/native/src/sys/disk.rs @@ -35,15 +35,63 @@ pub fn calculate_agr_bytes(fps: i32, duration_secs: f32) -> u64 { /// literals that could silently drift apart. pub const MIN_DRIVE_HEADROOM_BYTES: u64 = 15 * 1024 * 1024 * 1024; -/// The drive (`c:`) or share (`\\server\share`) a path is on, lowercased; -/// `None` for a path with no prefix. Folders with the same key share one -/// drive's free space, so planners budget per key, not per folder (#774). +/// The volume a path is on, lowercased: its `\\?\volume{guid}\` name when +/// Windows can tell, so a disk mounted as a folder inside another drive +/// (`D:\Captures`) keys apart from `D:` itself (#1041); else the drive +/// (`c:`) or share (`\\server\share`). `None` for a path with no prefix. +/// Folders with the same key share one drive's free space, so planners budget +/// per key, not per folder (#774). pub fn volume_key(path: &std::path::Path) -> Option { - match path.components().next()? { - std::path::Component::Prefix(prefix) => { - Some(prefix.as_os_str().to_string_lossy().to_lowercase()) + let std::path::Component::Prefix(prefix) = path.components().next()? else { + return None; + }; + // Only a drive letter is asked about: asking about a share that's offline + // waits on the network. + #[cfg(windows)] + if matches!( + prefix.kind(), + std::path::Prefix::Disk(_) | std::path::Prefix::VerbatimDisk(_) + ) && let Some(name) = win_volume::volume_name(path) + { + return Some(name.to_lowercase()); + } + Some(prefix.as_os_str().to_string_lossy().to_lowercase()) +} + +/// `GetVolumePathNameW` (the mount point a path is under, its nearest +/// existing folder deciding) then `GetVolumeNameForVolumeMountPointW`. +/// `None` for a `subst` drive or a drive that isn't there. +#[cfg(windows)] +mod win_volume { + use std::os::windows::ffi::OsStrExt; + + #[link(name = "kernel32")] + unsafe extern "system" { + fn GetVolumePathNameW(file_name: *const u16, volume_path: *mut u16, len: u32) -> i32; + fn GetVolumeNameForVolumeMountPointW( + mount_point: *const u16, + volume_name: *mut u16, + len: u32, + ) -> i32; + } + + pub fn volume_name(path: &std::path::Path) -> Option { + let wide: Vec = path.as_os_str().encode_wide().chain(Some(0)).collect(); + let mut mount = vec![0u16; wide.len().max(4) + 1]; + if unsafe { GetVolumePathNameW(wide.as_ptr(), mount.as_mut_ptr(), mount.len() as u32) } == 0 + { + return None; + } + // A volume name is `\\?\Volume{GUID}\`: 49 characters and the nul. + let mut name = [0u16; 64]; + if unsafe { + GetVolumeNameForVolumeMountPointW(mount.as_ptr(), name.as_mut_ptr(), name.len() as u32) + } == 0 + { + return None; } - _ => None, + let len = name.iter().position(|&c| c == 0).unwrap_or(name.len()); + Some(String::from_utf16_lossy(&name[..len])) } } @@ -301,6 +349,29 @@ mod tests { assert_eq!(volume_key(Path::new("relative")), None); } + /// #1041: a real drive keys by its volume name, which a disk mounted as a + /// folder has of its own; a folder that isn't there yet keys by the + /// volume it will be on. + #[cfg(windows)] + #[test] + fn a_drive_keys_by_its_volume_name() { + use std::path::Path; + let system = std::env::var("SystemDrive").unwrap_or_else(|_| "C:".into()); + let key = volume_key(&Path::new(&system).join("Windows")).unwrap(); + assert!(key.starts_with(r"\\?\volume{"), "{key}"); + assert_eq!( + volume_key(&Path::new(&system).join("no-such-folder-1041").join("x")), + Some(key) + ); + // No such drive: the letter, as before. + let missing = (b'A'..=b'Z') + .map(|c| format!("{}:", c as char)) + .find(|d| !Path::new(&format!("{d}\\")).exists()); + if let Some(d) = missing { + assert_eq!(volume_key(&Path::new(&d).join("x")), Some(d.to_lowercase())); + } + } + /// #774: two folders on C: get one budget, D: its own, and folders with /// no drive never merge. #[cfg(windows)]