From c971e81eed6480a8e8db6fea5bd96cce9ee299a9 Mon Sep 17 00:00:00 2001 From: Aleksandr Bogomiakov Date: Sun, 2 Aug 2026 03:04:16 +0100 Subject: [PATCH] Many UI changes --- CHANGELOG.md | 12 ++++++++-- devenv.nix | 35 ++++++++++++++++++++-------- flake.lock | 37 ++++++++++++++++++++++++++++++ flake.nix | 3 +++ src/app/mod.rs | 40 ++++++++++++++++++++------------ src/federation/mod.rs | 34 ++++++++------------------- src/library/mod.rs | 25 ++++++++++++++++---- src/library/tests.rs | 8 +++++++ src/media.rs | 53 ++++++++++++++++++++++++++++++++++++++++++- src/ui/federation.rs | 4 ++-- 10 files changed, 192 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f32750..feed566 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,8 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 optional safe migration of existing Furumi-managed files. - Queue reordering for one track or a `Shift+V` selection with `Alt+K` and `Alt+J`. -- Reproducible Nix/devenv tooling with Rust 1.97 and ALSA development files, - plus automatic Rust 1.97 selection through `rust-toolchain.toml`. +- Reproducible Nix/devenv tooling with Rust 1.97 and ALSA development files. ### Changed @@ -28,5 +27,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Federated artist images and release covers are stored beside permanent music in an `Artist/Release` directory tree. +### Fixed + +- Running a development build next to another Furumi instance no longer lets + an MPRIS name collision disable terminal raw mode and freeze keyboard input. +- `nix develop` now keeps devenv state outside the read-only Nix store and + isolates Rust 1.97 build artifacts from other toolchains. +- Music-directory validation and migration now reject overlapping changes, + resolve canonical paths, and produce Windows-portable managed filenames. + [Unreleased]: https://gt.hexor.cy/ab/furumi_tui/compare/v0.2.5...HEAD [0.2.5]: https://gt.hexor.cy/ab/furumi_tui/compare/v0.2.4...v0.2.5 diff --git a/devenv.nix b/devenv.nix index 1bdf4e3..9e2da86 100644 --- a/devenv.nix +++ b/devenv.nix @@ -1,16 +1,22 @@ -{ pkgs, ... }: +{ config, lib, pkgs, ... }: +let + sourceRoot = builtins.toString ./.; + sourceKey = builtins.substring 0 12 (builtins.hashString "sha256" sourceRoot); + transientDotfile = "/tmp/furumi-tui-devenv-${sourceKey}"; + rustToolchain = builtins.fromTOML (builtins.readFile ./rust-toolchain.toml); + rustVersion = rustToolchain.toolchain.channel; +in { + # Pure flakes only expose their read-only store source. Keep devenv's own + # task/profile state writable, then restore the real checkout path in-shell. + devenv.root = sourceRoot; + devenv.dotfile = transientDotfile; + devenv.state = "${transientDotfile}/state"; + languages.rust = { enable = true; - channel = "stable"; - version = "1.97.0"; - components = [ - "cargo" - "clippy" - "rust-src" - "rustfmt" - ]; + toolchainFile = ./rust-toolchain.toml; }; packages = [ @@ -20,4 +26,15 @@ ]; env.RUST_BACKTRACE = "1"; + + enterShell = lib.mkAfter '' + export DEVENV_ROOT="$PWD" + # Keep Nix builds separate from artifacts produced by rustup or another + # devenv generation. Cargo metadata is not compatible across compilers. + export CARGO_TARGET_DIR="$PWD/target/devenv-rust-${rustVersion}" + export PATH="${config.languages.rust.toolchainPackage}/bin:$PATH" + export RUSTC="${config.languages.rust.toolchainPackage}/bin/rustc" + export RUSTDOC="${config.languages.rust.toolchainPackage}/bin/rustdoc" + hash -r + ''; } diff --git a/flake.lock b/flake.lock index 0d513e4..2ab8b57 100644 --- a/flake.lock +++ b/flake.lock @@ -172,6 +172,21 @@ "type": "github" } }, + "mk-shell-bin": { + "locked": { + "lastModified": 1677004959, + "narHash": "sha256-/uEkr1UkJrh11vD02aqufCxtbF5YnhRTIKlx5kyvf+I=", + "owner": "rrbutani", + "repo": "nix-mk-shell-bin", + "rev": "ff5d8bd4d68a347be5042e2f16caee391cd75887", + "type": "github" + }, + "original": { + "owner": "rrbutani", + "repo": "nix-mk-shell-bin", + "type": "github" + } + }, "nix": { "inputs": { "flake-compat": [ @@ -212,6 +227,26 @@ "type": "github" } }, + "nix2container": { + "inputs": { + "nixpkgs": [ + "nixpkgs" + ] + }, + "locked": { + "lastModified": 1775487831, + "narHash": "sha256-2lguQpLPQaxpQCJjXhmEEAfabwsAhkP29Z7fgLzHARA=", + "owner": "nlewo", + "repo": "nix2container", + "rev": "76be9608a7f4d6c985d28b0e7be903ae2547df3e", + "type": "github" + }, + "original": { + "owner": "nlewo", + "repo": "nix2container", + "type": "github" + } + }, "nixd": { "inputs": { "flake-parts": [ @@ -289,6 +324,8 @@ "inputs": { "devenv": "devenv", "flake-parts": "flake-parts_2", + "mk-shell-bin": "mk-shell-bin", + "nix2container": "nix2container", "nixpkgs": "nixpkgs_2", "rust-overlay": "rust-overlay_2" } diff --git a/flake.nix b/flake.nix index 43de5f8..461745c 100644 --- a/flake.nix +++ b/flake.nix @@ -8,6 +8,9 @@ devenv.inputs.nixpkgs.follows = "nixpkgs"; rust-overlay.url = "github:oxalica/rust-overlay"; rust-overlay.inputs.nixpkgs.follows = "nixpkgs"; + nix2container.url = "github:nlewo/nix2container"; + nix2container.inputs.nixpkgs.follows = "nixpkgs"; + mk-shell-bin.url = "github:rrbutani/nix-mk-shell-bin"; }; outputs = inputs@{ flake-parts, ... }: diff --git a/src/app/mod.rs b/src/app/mod.rs index 4e6f2b6..0fbc944 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -110,6 +110,11 @@ fn refresh_local_library_stats(runtime: &Runtime) { } pub(super) fn validate_music_directory(state: &mut AppState, runtime: &Runtime, path: PathBuf) { + if state.music_dir_changing { + state.status_message = Some("music directory change is already running".into()); + return; + } + state.music_dir_changing = true; state.status_message = Some("checking music directory write access…".into()); let tx = runtime.event_tx.clone(); tokio::task::spawn_blocking(move || { @@ -270,6 +275,7 @@ pub async fn run( Arc::clone(&jam), settings.music_dir.clone(), ); + state.music_dir = federation.media_dir(); state.federation.settings = federation.settings(); state.federation.devices = Some(devices.status()); if let Ok((device_id, device_name)) = devices.identity_summary() { @@ -3040,23 +3046,27 @@ fn handle_app_event(state: &mut AppState, runtime: &mut Runtime, event: AppEvent Err(err) => state::Loadable::Failed(err), }); } - AppEvent::MusicDirectoryValidated(result) => match result { - Ok(path) => { - let current = std::fs::canonicalize(&state.music_dir) - .unwrap_or_else(|_| state.music_dir.clone()); - if path == current { - state.status_message = Some("this is already the music save directory".into()); - } else { - state.popup = Some(state::Popup::ConfirmMusicDirectory { path }); - state.status_message = None; + AppEvent::MusicDirectoryValidated(result) => { + state.music_dir_changing = false; + match result { + Ok(path) => { + let current = std::fs::canonicalize(&state.music_dir) + .unwrap_or_else(|_| state.music_dir.clone()); + if path == current { + state.status_message = + Some("this is already the music save directory".into()); + } else { + state.popup = Some(state::Popup::ConfirmMusicDirectory { path }); + state.status_message = None; + } + } + Err(message) => { + state.status_message = Some(format!( + "music directory is not writable; nothing changed: {message}" + )); } } - Err(message) => { - state.status_message = Some(format!( - "music directory is not writable; nothing changed: {message}" - )); - } - }, + } AppEvent::MusicDirectoryChanged(result) => { state.music_dir_changing = false; match result { diff --git a/src/federation/mod.rs b/src/federation/mod.rs index ed13584..17c613a 100644 --- a/src/federation/mod.rs +++ b/src/federation/mod.rs @@ -544,6 +544,7 @@ impl Federation { .err() .map(|err| format!("cannot create {}: {err}", dir.display())) }); + let media_dir = std::fs::canonicalize(&media_dir).unwrap_or(media_dir); Arc::new(Self { library, devices, @@ -1439,8 +1440,8 @@ impl Federation { /// Returns a cached-or-streamed image for the card: the artist image /// (`release: None`) or a release cover. Peers are tried in order until - /// one answers with an image; the result lands in the art cache and its - /// local path is returned. + /// one answers with an image; the result is stored beside the artist or + /// release in the permanent music tree and its local path is returned. pub async fn card_image( &self, owners: &[String], @@ -1983,8 +1984,8 @@ impl Federation { } } -/// Writes a received artist image into the covers directory and attaches it -/// to the artist unless one is already set. +/// Writes a received artist image into the artist's music directory and +/// attaches it to the artist unless one is already set. fn save_artist_image( library: &Library, artist_name: &str, @@ -2599,36 +2600,19 @@ fn collect_specs(library: &Library) -> Result> { Ok(specs) } -fn sanitize_file_stem(value: &str) -> String { - let cleaned: String = value - .chars() - .map(|c| match c { - '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|' => '_', - c if c.is_control() => '_', - c => c, - }) - .collect(); - let trimmed = cleaned.trim().trim_matches('.'); - let mut stem: String = trimmed.chars().take(120).collect(); - if stem.is_empty() { - stem.push_str("track"); - } - stem -} - fn music_artist_dir(root: &Path, artist: &str) -> PathBuf { let artist = if artist.trim().is_empty() { "Unknown Artist" } else { artist }; - root.join(sanitize_file_stem(artist)) + root.join(crate::library::storage_name(artist, "Unknown Artist")) } fn music_art_dir(root: &Path, artist: &str, release: Option<&str>) -> PathBuf { let artist_dir = music_artist_dir(root, artist); match release.filter(|release| !release.trim().is_empty()) { - Some(release) => artist_dir.join(sanitize_file_stem(release)), + Some(release) => artist_dir.join(crate::library::storage_name(release, "Unknown Release")), None => artist_dir, } } @@ -2650,9 +2634,9 @@ fn music_release_dir(root: &Path, fed: &FedTrack) -> PathBuf { fn download_stem(fed: &FedTrack) -> String { let artists = fed.artist_line(); if artists.is_empty() { - sanitize_file_stem(&fed.title) + crate::library::storage_name(&fed.title, "track") } else { - sanitize_file_stem(&format!("{artists} - {}", fed.title)) + crate::library::storage_name(&format!("{artists} - {}", fed.title), "track") } } diff --git a/src/library/mod.rs b/src/library/mod.rs index c506cbc..3664a09 100644 --- a/src/library/mod.rs +++ b/src/library/mod.rs @@ -446,6 +446,7 @@ impl Library { for row in rows { let source = PathBuf::from(&row.track_path); + let source = std::fs::canonicalize(&source).unwrap_or(source); if !source.is_file() || !source.starts_with(&old_root) { continue; } @@ -469,6 +470,7 @@ impl Library { if let Some(cover) = row.cover_path { let cover_source = PathBuf::from(&cover); + let cover_source = std::fs::canonicalize(&cover_source).unwrap_or(cover_source); if cover_source.is_file() && managed_art(&cover_source) && !release_updates.contains_key(&row.release_id) @@ -491,6 +493,7 @@ impl Library { if let (Some(artist_id), Some(image)) = (row.artist_id, row.image_path) { let image_source = PathBuf::from(&image); + let image_source = std::fs::canonicalize(&image_source).unwrap_or(image_source); if image_source.is_file() && managed_art(&image_source) && !artist_updates.contains_key(&artist_id) @@ -2757,7 +2760,7 @@ impl Library { } } -fn storage_name(value: &str, fallback: &str) -> String { +pub(crate) fn storage_name(value: &str, fallback: &str) -> String { let cleaned: String = value .chars() .map(|character| match character { @@ -2767,12 +2770,24 @@ fn storage_name(value: &str, fallback: &str) -> String { }) .collect(); let cleaned = cleaned.trim().trim_matches('.'); - let shortened: String = cleaned.chars().take(120).collect(); + let mut shortened: String = cleaned.chars().take(120).collect(); if shortened.is_empty() { - fallback.to_string() - } else { - shortened + return fallback.to_string(); } + let windows_base = shortened + .split('.') + .next() + .unwrap_or_default() + .to_ascii_uppercase(); + let windows_reserved = matches!(windows_base.as_str(), "CON" | "PRN" | "AUX" | "NUL") + || windows_base + .strip_prefix("COM") + .or_else(|| windows_base.strip_prefix("LPT")) + .is_some_and(|number| number.len() == 1 && matches!(number.as_bytes()[0], b'1'..=b'9')); + if windows_reserved { + shortened.insert(0, '_'); + } + shortened } fn unique_destination(requested: PathBuf, id: i64, reserved: &mut HashSet) -> PathBuf { diff --git a/src/library/tests.rs b/src/library/tests.rs index 573417d..2381d40 100644 --- a/src/library/tests.rs +++ b/src/library/tests.rs @@ -184,6 +184,14 @@ fn music_directory_validation_rejects_a_file_without_touching_it() { std::fs::remove_dir_all(root).unwrap(); } +#[test] +fn managed_music_names_are_portable_to_windows() { + assert_eq!(storage_name("Artist/Name", "fallback"), "Artist_Name"); + assert_eq!(storage_name("CON", "fallback"), "_CON"); + assert_eq!(storage_name("lpt9.live", "fallback"), "_lpt9.live"); + assert_eq!(storage_name("...", "fallback"), "fallback"); +} + #[test] fn local_stats_counts_library_rows_and_audio_bytes() { let lib = test_library(); diff --git a/src/media.rs b/src/media.rs index 8f8fa72..3e7430f 100644 --- a/src/media.rs +++ b/src/media.rs @@ -99,9 +99,10 @@ fn create_controls() -> Option { #[cfg(not(target_os = "windows"))] let hwnd = None; + let dbus_name = platform_dbus_name(); let config = PlatformConfig { display_name: "Furumi", - dbus_name: "cy.hexor.furumi", + dbus_name: &dbus_name, hwnd, }; match MediaControls::new(config) { @@ -113,6 +114,41 @@ fn create_controls() -> Option { } } +/// MPRIS well-known names must be unique on the session bus. Developers often +/// run a checkout next to an installed Furumi, and souvlaki 0.8.3 reports a +/// duplicate name by panicking in its private service thread. Ratatui's global +/// panic hook then restores the terminal even though the app thread is still +/// alive, leaving a frozen UI in canonical/echo mode. +/// +/// Give every Unix MPRIS instance its own valid bus-name component. Other +/// backends ignore `dbus_name`, so retain the stable application identifier +/// there. +fn platform_dbus_name() -> String { + #[cfg(all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) + ))] + { + mpris_dbus_name(std::process::id()) + } + + #[cfg(not(all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) + )))] + { + "cy.hexor.furumi".to_string() + } +} + +#[cfg(all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) +))] +fn mpris_dbus_name(process_id: u32) -> String { + format!("cy.hexor.furumi.instance{process_id}") +} + /// An invisible top-level window owning the SMTC session. Created on the /// main thread, which also pumps its messages in `pump_platform_events`. #[cfg(target_os = "windows")] @@ -217,3 +253,18 @@ fn pump_platform_events() { #[cfg(not(any(target_os = "macos", target_os = "windows")))] fn pump_platform_events() {} + +#[cfg(test)] +mod tests { + #[cfg(all( + unix, + not(any(target_os = "macos", target_os = "ios", target_os = "android")) + ))] + #[test] + fn mpris_names_are_distinct_between_processes() { + let first = super::mpris_dbus_name(41); + let second = super::mpris_dbus_name(42); + assert_eq!(first, "cy.hexor.furumi.instance41"); + assert_ne!(first, second); + } +} diff --git a/src/ui/federation.rs b/src/ui/federation.rs index fa574f8..231b55e 100644 --- a/src/ui/federation.rs +++ b/src/ui/federation.rs @@ -33,7 +33,7 @@ pub fn draw(frame: &mut Frame, area: Rect, state: &AppState) { } let rows_height = - (settings_rows(state).len() + 6 + device_presence_sections(state).len()) as u16; + (settings_rows(state).len() + 8 + device_presence_sections(state).len()) as u16; let [rows_area, _, status_area] = Layout::vertical([ Constraint::Length(rows_height.min(inner.height)), Constraint::Length(1), @@ -79,7 +79,7 @@ fn draw_settings_rows(frame: &mut Frame, area: Rect, state: &AppState) { state.settings_cursor, "Music save directory", if state.music_dir_changing { - format!("{} moving…", state.spinner()) + format!("{} checking/changing…", state.spinner()) } else { state.music_dir.to_string_lossy().into_owned() },