mount: fix iPod automount (lsblk flags + udisks suffixed paths)
Two bugs kept the connected iPod from being auto-mounted: - `lsblk -o PATH,LABEL -n -l -P` is invalid (--pairs and --list are mutually exclusive), so device lookup by label always failed with "no block device found" — a regression from the earlier parse fix. Drop -l. - udisks mounted the iPod volume at /media/sebastian/IPOD1 because the configured directory /media/sebastian/IPOD already exists, so even a resolved device failed with AlreadyMounted. The configured mount point is now treated as a hint: mounted_path_for() resolves an exact mount or the lowest-numbered udisks-suffixed sibling (IPOD → IPOD1) from /proc/self/mountinfo, and the sync destination, storage stats, settings display and fatsort resolution all use the real mount. Also harden fatsort: resolve_fatsort_target now refuses to run when the path is not a mount point, so findmnt can never fall back to the root filesystem's block device. Verified on the connected iPod: mounted_path_for → IPOD1, resolve_block_device → /dev/sdb2, mount_device → "already mounted at /media/sebastian/IPOD1", UI shows ● mounted with 9.7 GiB free · 92% and mount point ✓ /media/sebastian/IPOD1. Bump to 0.3.3.
This commit is contained in:
Generated
+1
-1
@@ -253,7 +253,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "dap-tui"
|
||||
version = "0.3.2"
|
||||
version = "0.3.3"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"chrono",
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "dap-tui"
|
||||
version = "0.3.2"
|
||||
version = "0.3.3"
|
||||
edition = "2021"
|
||||
description = "Device sync for portable players — TUI edition (ratatui)"
|
||||
license = "MIT"
|
||||
|
||||
@@ -107,9 +107,12 @@ mounts the target first (resolved by mount-point label via `lsblk`). Only the
|
||||
mlove `fatsort` step needs root (raw block device).
|
||||
|
||||
Mount status and sync targets are validated against the kernel mount table
|
||||
(`/proc/self/mountinfo`), not just directory existence: if the configured
|
||||
`mount_point` exists but is not actually mounted, `dap-tui` refuses to sync to
|
||||
it rather than writing the library onto the root filesystem.
|
||||
(`/proc/self/mountinfo`), not just directory existence. The configured
|
||||
`mount_point` is a hint: if udisks mounted the volume at a suffixed sibling
|
||||
because the configured directory already existed (e.g. `IPOD` → `IPOD1`),
|
||||
dap-tui follows the actual mount and uses it as the sync destination. If the
|
||||
path exists but is not a mount at all, syncing into it is refused rather than
|
||||
writing the library onto the root filesystem.
|
||||
|
||||
## Development
|
||||
|
||||
|
||||
+3
-1
@@ -122,12 +122,14 @@ pub struct Config {
|
||||
|
||||
impl Config {
|
||||
/// Refresh `Device::mounted` from the kernel mount table (no subprocess).
|
||||
/// A device counts as mounted when its volume is mounted at the configured
|
||||
/// path or at a udisks-suffixed sibling (e.g. `IPOD` → `IPOD1`).
|
||||
pub fn refresh_mounts(&mut self) {
|
||||
for d in self.devices.iter_mut() {
|
||||
d.mounted = d
|
||||
.mount_point
|
||||
.as_deref()
|
||||
.map(crate::disk::is_mounted)
|
||||
.map(|p| crate::disk::mounted_path_for(p).is_some())
|
||||
.unwrap_or(false);
|
||||
}
|
||||
}
|
||||
|
||||
+76
-2
@@ -1,4 +1,4 @@
|
||||
use std::path::Path;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// Whether `path` is currently a mount point.
|
||||
///
|
||||
@@ -26,6 +26,56 @@ pub fn is_mounted(path: &Path) -> bool {
|
||||
path.exists()
|
||||
}
|
||||
|
||||
/// Mount points currently listed in the kernel mount table.
|
||||
fn mount_points() -> Vec<PathBuf> {
|
||||
#[cfg(target_os = "linux")]
|
||||
if let Ok(text) = std::fs::read_to_string("/proc/self/mountinfo") {
|
||||
return text
|
||||
.lines()
|
||||
.filter_map(|line| line.split(' ').nth(4))
|
||||
.map(|field| PathBuf::from(unescape_octal(field)))
|
||||
.collect();
|
||||
}
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
/// Where the volume configured at `path` is actually mounted.
|
||||
///
|
||||
/// udisks mounts a volume at `/media/$USER/$LABEL`; if that directory already
|
||||
/// exists (e.g. a leftover mount dir after an unclean eject) it appends a
|
||||
/// number, so `/media/sebastian/IPOD` becomes `/media/sebastian/IPOD1`. This
|
||||
/// resolves the exact path or the lowest-numbered suffixed sibling, so the
|
||||
/// configured path is treated as a hint rather than a hard requirement.
|
||||
pub fn mounted_path_for(path: &Path) -> Option<PathBuf> {
|
||||
if is_mounted(path) {
|
||||
return Some(path.to_path_buf());
|
||||
}
|
||||
let parent = path.parent()?;
|
||||
let base = path.file_name()?.to_string_lossy().to_string();
|
||||
mount_points()
|
||||
.into_iter()
|
||||
.filter_map(|mp| mount_suffix_rank(parent, &base, &mp).map(|rank| (rank, mp)))
|
||||
.min_by_key(|(rank, _)| *rank)
|
||||
.map(|(_, mp)| mp)
|
||||
}
|
||||
|
||||
/// Rank of a mount point relative to a configured path: 0 for an exact match,
|
||||
/// `n` for `name{n}` (udisks' uniquifying suffix), `None` otherwise.
|
||||
fn mount_suffix_rank(parent: &Path, base: &str, candidate: &Path) -> Option<u32> {
|
||||
if candidate.parent() != Some(parent) {
|
||||
return None;
|
||||
}
|
||||
let name = candidate.file_name()?.to_string_lossy();
|
||||
let rest = name.strip_prefix(base)?;
|
||||
if rest.is_empty() {
|
||||
return Some(0);
|
||||
}
|
||||
if rest.chars().all(|c| c.is_ascii_digit()) {
|
||||
return rest.parse().ok();
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Undo the octal escaping (`\040` etc.) used by /proc mount tables.
|
||||
fn unescape_octal(s: &str) -> String {
|
||||
let bytes = s.as_bytes();
|
||||
@@ -84,7 +134,31 @@ pub fn fmt_bytes(n: u64) -> String {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{fmt_bytes, unescape_octal};
|
||||
use super::{fmt_bytes, mounted_path_for, mount_suffix_rank, unescape_octal};
|
||||
|
||||
#[test]
|
||||
fn ranks_udisks_suffixed_mounts() {
|
||||
let parent = std::path::Path::new("/media/sebastian");
|
||||
let rank = |c: &str| mount_suffix_rank(parent, "IPOD", std::path::Path::new(c));
|
||||
assert_eq!(rank("/media/sebastian/IPOD"), Some(0));
|
||||
assert_eq!(rank("/media/sebastian/IPOD1"), Some(1));
|
||||
assert_eq!(rank("/media/sebastian/IPOD12"), Some(12));
|
||||
// non-numeric suffixes and other directories must not match
|
||||
assert_eq!(rank("/media/sebastian/IPODX"), None);
|
||||
assert_eq!(rank("/media/sebastian/IPOD-2"), None);
|
||||
assert_eq!(rank("/media/other/IPOD1"), None);
|
||||
assert_eq!(rank("/media/sebastian/IPOD1/extra"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(target_os = "linux")]
|
||||
fn resolves_exact_mounts() {
|
||||
// `/` is always mounted; mounted_path_for must return it unchanged.
|
||||
assert_eq!(
|
||||
mounted_path_for(std::path::Path::new("/")),
|
||||
Some(std::path::PathBuf::from("/"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unescapes_mountinfo_paths() {
|
||||
|
||||
+42
-20
@@ -542,29 +542,43 @@ fn scan_music_dir(base: &Path) -> HashMap<String, (u64, i64)> {
|
||||
}
|
||||
|
||||
fn run_rsync_device(config: &Config, device: &Device, ctx: &WorkerCtx) -> Result<(), String> {
|
||||
let mount = device
|
||||
let configured = device
|
||||
.mount_point
|
||||
.as_ref()
|
||||
.ok_or_else(|| "mount_point is not set for this device".to_string())?;
|
||||
|
||||
if !crate::disk::is_mounted(mount) {
|
||||
ctx.log(
|
||||
LogLevel::Info,
|
||||
format!("device not mounted at {} — trying udisksctl…", mount.display()),
|
||||
);
|
||||
match crate::udisks::mount_device(mount) {
|
||||
Ok(msg) => ctx.log(LogLevel::Info, format!("auto-mounted: {msg}")),
|
||||
Err(e) => {
|
||||
return Err(format!(
|
||||
"device not mounted at {} — connect & mount it (m) and retry. {e}",
|
||||
mount.display()
|
||||
));
|
||||
// Trust the kernel mount table over the configured path: udisks may have
|
||||
// mounted the volume at a suffixed path (e.g. /media/.../IPOD1) because the
|
||||
// configured directory already existed.
|
||||
let mount = match crate::disk::mounted_path_for(configured) {
|
||||
Some(p) => p,
|
||||
None => {
|
||||
ctx.log(
|
||||
LogLevel::Info,
|
||||
format!(
|
||||
"device not mounted at {} — trying udisksctl…",
|
||||
configured.display()
|
||||
),
|
||||
);
|
||||
match crate::udisks::mount_device(configured) {
|
||||
Ok(msg) => ctx.log(LogLevel::Info, format!("auto-mounted: {msg}")),
|
||||
Err(e) => {
|
||||
return Err(format!(
|
||||
"device not mounted at {} — connect & mount it (m) and retry. {e}",
|
||||
configured.display()
|
||||
));
|
||||
}
|
||||
}
|
||||
crate::disk::mounted_path_for(configured).ok_or_else(|| {
|
||||
format!(
|
||||
"auto-mount of {} did not yield a mount point",
|
||||
configured.display()
|
||||
)
|
||||
})?
|
||||
}
|
||||
}
|
||||
// A leftover (empty) mount-point directory must not be mistaken for a
|
||||
// mounted device — rsync would otherwise happily fill the root filesystem.
|
||||
if !crate::disk::is_mounted(mount) {
|
||||
};
|
||||
// Paranoia: never write into a directory that is not actually a mount.
|
||||
if !crate::disk::is_mounted(&mount) {
|
||||
return Err(format!(
|
||||
"{} exists but is not a mount point — refusing to sync. Connect the device and press 'm'.",
|
||||
mount.display()
|
||||
@@ -732,15 +746,17 @@ fn capture(cmd: &str, args: &[&str]) -> Result<(i32, String), String> {
|
||||
}
|
||||
|
||||
fn finalize_fat(device: &Device, ctx: &WorkerCtx) -> Result<(), String> {
|
||||
let mount = device
|
||||
let configured = device
|
||||
.mount_point
|
||||
.as_ref()
|
||||
.ok_or_else(|| "mount_point is required to run fatsort".to_string())?;
|
||||
// Use the actual mount point in case udisks moved it to a suffixed path.
|
||||
let mount = crate::disk::mounted_path_for(configured).unwrap_or_else(|| configured.clone());
|
||||
|
||||
let target = resolve_fatsort_target(mount)?;
|
||||
let target = resolve_fatsort_target(&mount)?;
|
||||
|
||||
ctx.log(LogLevel::Info, format!("Unmounting {mount_display}", mount_display = mount.display()));
|
||||
match crate::udisks::unmount_device(mount) {
|
||||
match crate::udisks::unmount_device(&mount) {
|
||||
Ok(msg) => ctx.log(LogLevel::Info, format!("unmounted: {msg}")),
|
||||
Err(e) => {
|
||||
// fall back to sudo for setups without udisks
|
||||
@@ -771,6 +787,12 @@ fn resolve_fatsort_target(mount: &Path) -> Result<String, String> {
|
||||
if !mount.is_dir() {
|
||||
return Ok(mount.display().to_string());
|
||||
}
|
||||
// Never let findmnt fall back to the parent filesystem (e.g. the root
|
||||
// disk) when the directory is not actually a mount point — fatsort on the
|
||||
// wrong block device would corrupt it.
|
||||
if !crate::disk::is_mounted(mount) {
|
||||
return Err(format!("{} is not a mount point", mount.display()));
|
||||
}
|
||||
let (code, out) = capture(
|
||||
"findmnt",
|
||||
&["-n", "-o", "SOURCE", "-T", mount.to_str().unwrap_or("")],
|
||||
|
||||
+56
-6
@@ -19,11 +19,10 @@ fn run(cmd: &str, args: &[&str]) -> (bool, String) {
|
||||
}
|
||||
|
||||
fn resolve_block_device(path: &Path) -> Option<String> {
|
||||
// Only trust findmnt when the path really is a mount point; otherwise
|
||||
// findmnt happily reports the containing filesystem (e.g. /), which would
|
||||
// make us "mount" the root device.
|
||||
if crate::disk::is_mounted(path) {
|
||||
let (ok, out) = run("findmnt", &["-n", "-o", "SOURCE", "-T", path.to_str()?]);
|
||||
// If the volume is already mounted (possibly at a udisks-suffixed path
|
||||
// like IPOD1), ask findmnt for its source device.
|
||||
if let Some(mp) = crate::disk::mounted_path_for(path) {
|
||||
let (ok, out) = run("findmnt", &["-n", "-o", "SOURCE", "-T", mp.to_str()?]);
|
||||
if ok {
|
||||
let src = out.trim();
|
||||
if src.starts_with("/dev/") {
|
||||
@@ -31,8 +30,10 @@ fn resolve_block_device(path: &Path) -> Option<String> {
|
||||
}
|
||||
}
|
||||
}
|
||||
// Not mounted: match a partition label to the configured mount point's
|
||||
// basename (e.g. `/media/sebastian/IPOD` → label `IPOD`).
|
||||
let wanted = path.file_name()?.to_string_lossy().to_lowercase();
|
||||
let (ok, out) = run("lsblk", &["-o", "PATH,LABEL", "-n", "-l", "-P"]);
|
||||
let (ok, out) = run("lsblk", &["-o", "PATH,LABEL", "-n", "-P"]);
|
||||
if !ok {
|
||||
return None;
|
||||
}
|
||||
@@ -50,6 +51,16 @@ fn resolve_block_device(path: &Path) -> Option<String> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Current mount target of a block device, if any.
|
||||
fn mounted_target(dev: &str) -> Option<std::path::PathBuf> {
|
||||
let (ok, out) = run("findmnt", &["-n", "-o", "TARGET", "-S", dev]);
|
||||
if ok && !out.trim().is_empty() {
|
||||
Some(std::path::PathBuf::from(out.trim()))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract a `KEY="value"` field from `lsblk -P` output.
|
||||
fn field(line: &str, key: &str) -> Option<String> {
|
||||
let needle = format!("{key}=\"");
|
||||
@@ -60,13 +71,25 @@ fn field(line: &str, key: &str) -> Option<String> {
|
||||
}
|
||||
|
||||
pub fn mount_device(path: &Path) -> Result<String, String> {
|
||||
// Already mounted, possibly at a udisks-suffixed path.
|
||||
if let Some(mp) = crate::disk::mounted_path_for(path) {
|
||||
return Ok(format!("already mounted at {}", mp.display()));
|
||||
}
|
||||
let Some(dev) = resolve_block_device(path) else {
|
||||
return Err(format!("no block device found for {}", path.display()));
|
||||
};
|
||||
if let Some(mp) = mounted_target(&dev) {
|
||||
return Ok(format!("already mounted at {}", mp.display()));
|
||||
}
|
||||
let (ok, detail) = run("udisksctl", &["mount", "-b", dev.as_str()]);
|
||||
if ok {
|
||||
Ok(detail)
|
||||
} else {
|
||||
// A concurrent/late mount makes AlreadyMounted spurious: report
|
||||
// success when the kernel now has the device mounted.
|
||||
if let Some(mp) = mounted_target(&dev) {
|
||||
return Ok(format!("mounted at {}", mp.display()));
|
||||
}
|
||||
Err(format!("udisksctl mount {dev} failed: {detail}"))
|
||||
}
|
||||
}
|
||||
@@ -82,3 +105,30 @@ pub fn unmount_device(path: &Path) -> Result<String, String> {
|
||||
Err(format!("udisksctl unmount {dev} failed: {detail}"))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::field;
|
||||
|
||||
#[test]
|
||||
fn parses_lsblk_pairs() {
|
||||
assert_eq!(
|
||||
field(r#"PATH="/dev/sdb2" LABEL="IPOD""#, "PATH").as_deref(),
|
||||
Some("/dev/sdb2")
|
||||
);
|
||||
assert_eq!(
|
||||
field(r#"PATH="/dev/sdb2" LABEL="IPOD""#, "LABEL").as_deref(),
|
||||
Some("IPOD")
|
||||
);
|
||||
assert_eq!(
|
||||
field(r#"PATH="/dev/sdb1" LABEL="""#, "LABEL").as_deref(),
|
||||
Some("")
|
||||
);
|
||||
assert_eq!(
|
||||
field(r#"LABEL="My Disk" PATH="/dev/sdc1""#, "LABEL").as_deref(),
|
||||
Some("My Disk")
|
||||
);
|
||||
assert_eq!(field(r#"PATH="/dev/sdc1""#, "LABEL"), None);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -147,11 +147,14 @@ fn draw_device_card(f: &mut Frame, area: Rect, app: &App, device: &crate::config
|
||||
line("last sync", last, last_style),
|
||||
];
|
||||
|
||||
// Mounted storage usage. The compact value ("276.8 GiB free · 39%") fits
|
||||
// the card width, unlike the old "459.5 GiB total · 276.8 GiB free" which
|
||||
// was truncated mid-number.
|
||||
// Mounted storage usage. udisks may mount the volume at a suffixed path
|
||||
// (IPOD → IPOD1), so report stats for wherever it actually is mounted.
|
||||
let actual_mount = device
|
||||
.mount_point
|
||||
.as_deref()
|
||||
.and_then(crate::disk::mounted_path_for);
|
||||
let storage = if device.mounted {
|
||||
device.mount_point.as_ref().and_then(|p| disk_stats(p))
|
||||
actual_mount.as_deref().and_then(disk_stats)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -232,7 +235,12 @@ fn draw_settings_card(f: &mut Frame, area: Rect, device: &crate::config::Device)
|
||||
Style::default().fg(theme::FG),
|
||||
));
|
||||
match device.mount_point.as_deref() {
|
||||
Some(p) => lines.push(path_kv(l_mount, p, value_w, lw)),
|
||||
Some(p) => {
|
||||
// Show where it is actually mounted when udisks used a suffixed
|
||||
// path (e.g. the configured IPOD dir existed, so it chose IPOD1).
|
||||
let shown = crate::disk::mounted_path_for(p).unwrap_or_else(|| p.to_path_buf());
|
||||
lines.push(path_kv(l_mount, &shown, value_w, lw));
|
||||
}
|
||||
None => lines.push(line(l_mount, "—".to_string(), Style::default().fg(theme::DIM))),
|
||||
}
|
||||
lines.push(line(
|
||||
|
||||
Reference in New Issue
Block a user