diff --git a/compositor/src/config.rs b/compositor/src/config.rs index f910e38..29cf003 100644 --- a/compositor/src/config.rs +++ b/compositor/src/config.rs @@ -2,6 +2,7 @@ mod appearance; mod bindings; mod input_motion; mod layout; +mod loading; mod outputs; mod xwayland; @@ -10,6 +11,7 @@ use std::error::Error; use std::fmt; use std::path::PathBuf; +pub(crate) use self::loading::{prepare, prepare_document, read_source, startup}; pub(crate) use self::xwayland::{XwaylandConfig, XwaylandStartup}; use appearance::*; pub use appearance::{BorderGradient, FocusEffectSettings, MaterialStyle, ThemeSettings}; @@ -403,7 +405,7 @@ mod tests { .contains("unknown field") ); assert!( - crate::theme::prepare(&source, std::path::Path::new("/tmp")) + crate::config::prepare(&source, std::path::Path::new("/tmp")) .err() .unwrap() .contains(&key.replace('-', "_")) diff --git a/compositor/src/config/loading.rs b/compositor/src/config/loading.rs new file mode 100644 index 0000000..8bba48a --- /dev/null +++ b/compositor/src/config/loading.rs @@ -0,0 +1,140 @@ +use std::io::{self, Read}; +use std::path::Path; + +use ferese_config::Document; +use ferese_config::theme::Candidate; + +use super::Config; +use crate::RuntimeConfig; + +pub(crate) fn read_source(path: &Path) -> Result { + let mut source = String::new(); + std::fs::File::open(path) + .map_err(|e| format!("{}: {e}", path.display()))? + .take(60 * 1024 + 1) + .read_to_string(&mut source) + .map_err(|e| e.to_string())?; + if source.len() > 60 * 1024 { + return Err(format!("{} exceeds 60 KiB", path.display())); + } + Ok(source) +} + +pub(crate) fn prepare(source: &str, directory: &Path) -> Result<(Config, RuntimeConfig, Candidate), String> { + let document = Document::parse(source).map_err(|e| e.to_string())?; + prepare_document(&document, directory) +} + +pub(crate) struct StartupConfig { + pub(crate) source: Option, + pub(crate) runtime: RuntimeConfig, + pub(crate) theme: Candidate, + pub(crate) error: Option, +} + +pub(crate) fn startup(path: Option<&Path>) -> Result { + let directory = path.and_then(Path::parent).unwrap_or(Path::new(".")); + let source = path.map_or(Ok(None), |path| match std::fs::symlink_metadata(path) { + Ok(_) => read_source(path).map(Some), + Err(error) if error.kind() == io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(format!("{}: {error}", path.display())), + }); + let prepared = source.and_then(|source| { + let (_, runtime, theme) = prepare(source.as_deref().unwrap_or(""), directory)?; + Ok(StartupConfig { + source, + runtime, + theme, + error: None, + }) + }); + match prepared { + Ok(config) => Ok(config), + Err(error) => { + let (_, runtime, theme) = prepare("", directory)?; + Ok(StartupConfig { + source: Some(String::new()), + runtime, + theme, + error: Some(error), + }) + } + } +} + +pub(crate) fn prepare_document( + document: &Document, + directory: &Path, +) -> Result<(Config, RuntimeConfig, Candidate), String> { + let candidate = crate::theme::resolve_candidate(document, directory)?; + let config: Config = + serde_json::from_value(document.with_theme(&candidate.theme).value().clone()).map_err(|e| e.to_string())?; + let mut runtime = config.runtime_config().map_err(|e| e.to_string())?; + runtime.theme_settings = Config::resolved_theme_settings(&candidate.theme).map_err(|e| e.to_string())?; + Ok((config, runtime, candidate)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn invalid_startup_uses_defaults_and_keeps_the_authored_file() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + let (_, defaults, default_theme) = prepare("", directory.path()).unwrap(); + for source in [ + "panel main { surface solid; group-surface none; }", + "panel main { geometry { height 1; }; }", + "animations { speed 0; }", + "theme { file \"missing-theme.kdl\"; }", + "panel main {", + ] { + std::fs::write(&path, source).unwrap(); + let config = startup(Some(&path)).unwrap(); + assert!(config.error.is_some(), "{source}"); + assert_eq!(config.source.as_deref(), Some("")); + assert_eq!(config.runtime.panel_style, defaults.panel_style); + assert_eq!(config.runtime.layout_mode, defaults.layout_mode); + assert_eq!(config.theme.theme, default_theme.theme); + assert_eq!(std::fs::read_to_string(&path).unwrap(), source); + assert!( + prepare(source, directory.path()).is_err(), + "strict validation must still reject {source}" + ); + } + } + + #[test] + fn startup_distinguishes_missing_valid_and_unreadable_files() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + let missing = startup(Some(&path)).unwrap(); + assert!(missing.error.is_none()); + assert!(missing.source.is_none()); + assert!(!path.exists()); + + let source = "// keep me\npanel main { style floating; geometry { height 42; }; }"; + std::fs::write(&path, source).unwrap(); + let valid = startup(Some(&path)).unwrap(); + assert!(valid.error.is_none()); + assert_eq!(valid.source.as_deref(), Some(source)); + assert_eq!(valid.runtime.panel_style, ferese_panel::PanelStyle::Floating); + assert_eq!( + Document::parse(valid.source.as_deref().unwrap()) + .unwrap() + .get("panels.0.geometry.height"), + Some(&serde_json::json!(42)) + ); + assert_eq!(std::fs::read_to_string(&path).unwrap(), source); + + std::fs::write(&path, [0xff]).unwrap(); + assert!(startup(Some(&path)).unwrap().error.is_some()); + assert_eq!(std::fs::read(&path).unwrap(), [0xff]); + assert!(startup(Some(directory.path())).unwrap().error.is_some()); + std::fs::remove_file(&path).unwrap(); + std::os::unix::fs::symlink(directory.path().join("missing"), &path).unwrap(); + assert!(startup(Some(&path)).unwrap().error.is_some()); + assert!(path.is_symlink()); + } +} diff --git a/compositor/src/main.rs b/compositor/src/main.rs index c80a9df..83e6b4b 100644 --- a/compositor/src/main.rs +++ b/compositor/src/main.rs @@ -78,14 +78,11 @@ fn main() -> Result<(), Box> { let launch = LaunchConfig::from_environment().map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?; - let initial_source = config::config_path() - .filter(|path| path.exists()) - .map(|path| theme::read_source(&path)) - .transpose()?; - let directory = config::config_path() - .and_then(|path| path.parent().map(ToOwned::to_owned)) - .unwrap_or_else(|| std::path::PathBuf::from(".")); - let (_, runtime, candidate) = theme::prepare(initial_source.as_deref().unwrap_or(""), &directory)?; + let config_path = config::config_path(); + let startup = config::startup(config_path.as_deref())?; + if let Some(error) = &startup.error { + warn!(%error, "configuration could not be loaded at startup; using defaults"); + } let mut event_loop = EventLoop::try_new()?; let signals = Signals::new(&[Signal::SIGINT, Signal::SIGTERM])?; event_loop @@ -99,8 +96,14 @@ fn main() -> Result<(), Box> { backends::BackendKind::Drm => ipc::Endpoint::desktop()?, backends::BackendKind::Nested => ipc::Endpoint::instance()?, }; - let mut state = Ferese::new_with_session(&mut event_loop, display, runtime, endpoint, launch.session_policy)?; - state.config_source = initial_source.filter(|source| source.len() <= 60 * 1024); + let mut state = Ferese::new_with_session( + &mut event_loop, + display, + startup.runtime, + endpoint, + launch.session_policy, + )?; + state.config_source = startup.source; state.config_sections = state .config_source .as_deref() @@ -112,7 +115,10 @@ fn main() -> Result<(), Box> { } value }); - theme::init(&mut event_loop, &mut state, candidate)?; + if let Some(error) = startup.error { + state.theme_engine.startup_failed(config_path.as_deref(), error); + } + theme::init(&mut event_loop, &mut state, startup.theme)?; let _resume_monitor = resume::init(&mut event_loop)?; idle_inhibition::media::init(&mut event_loop)?; overview::init_font_loader(&mut event_loop, &mut state)?; @@ -171,8 +177,8 @@ fn init_logging() { } fn check_config(path: &std::path::Path) -> Result<(), String> { - let source = theme::read_source(path)?; - theme::prepare(&source, path.parent().unwrap_or(std::path::Path::new(".")))?; + let source = config::read_source(path)?; + config::prepare(&source, path.parent().unwrap_or(std::path::Path::new(".")))?; Ok(()) } diff --git a/compositor/src/reload.rs b/compositor/src/reload.rs index dc4a1d2..af2d9fc 100644 --- a/compositor/src/reload.rs +++ b/compositor/src/reload.rs @@ -148,7 +148,7 @@ impl Prepared { .map(|path| ferese_config::theme::theme_path(directory, path)), ); } - let (_, runtime, candidate) = crate::theme::prepare_document(&document, directory)?; + let (_, runtime, candidate) = crate::config::prepare_document(&document, directory)?; let mut sections = document.value().clone(); if let Some(object) = sections.as_object_mut() { object.remove("theme"); @@ -229,7 +229,7 @@ impl Worker { || request.fallback.ok_or_else(|| "No configuration available".into()), |path| { if path.exists() || !request.force { - crate::theme::read_source(path) + crate::config::read_source(path) } else { Ok(String::new()) } @@ -362,7 +362,7 @@ impl crate::Ferese { pub(crate) fn reload_config(&mut self) -> Result<(), String> { let path = crate::config::config_path().ok_or("config directory is unavailable")?; - self.reload_config_source(crate::theme::read_source(&path)?) + self.reload_config_source(crate::config::read_source(&path)?) } } @@ -418,7 +418,7 @@ mod tests { return; } - let (_, runtime, _) = crate::theme::prepare("", std::path::Path::new(".")).unwrap(); + let (_, runtime, _) = crate::config::prepare("", std::path::Path::new(".")).unwrap(); let mut event_loop = calloop::EventLoop::try_new().unwrap(); let display = smithay::reexports::wayland_server::Display::new().unwrap(); let mut state = crate::Ferese::new(&mut event_loop, display, runtime).unwrap(); @@ -498,7 +498,7 @@ mod tests { } let source = "appearance { focus-effect { inactive-opacity 0.8; inactive-dim 0.25; }; }"; let directory = Directory::new(); - let (_, runtime, candidate) = crate::theme::prepare(source, &directory.0).unwrap(); + let (_, runtime, candidate) = crate::config::prepare(source, &directory.0).unwrap(); let mut event_loop = smithay::reexports::calloop::EventLoop::try_new().unwrap(); let display = smithay::reexports::wayland_server::Display::new().unwrap(); let mut state = crate::Ferese::new(&mut event_loop, display, runtime).unwrap(); @@ -566,7 +566,7 @@ mod tests { } let directory = Directory::new(); - let (_, runtime, _) = crate::theme::prepare("xwayland { enabled #false; }", &directory.0).unwrap(); + let (_, runtime, _) = crate::config::prepare("xwayland { enabled #false; }", &directory.0).unwrap(); let mut event_loop = smithay::reexports::calloop::EventLoop::try_new().unwrap(); let display = smithay::reexports::wayland_server::Display::new().unwrap(); let mut state = crate::Ferese::new(&mut event_loop, display, runtime).unwrap(); @@ -615,7 +615,7 @@ mod tests { } let directory = Directory::new(); let source = "output-profile mobile { output eDP-1 scale=1.75; }"; - let (_, runtime, _) = crate::theme::prepare(source, &directory.0).unwrap(); + let (_, runtime, _) = crate::config::prepare(source, &directory.0).unwrap(); let mut event_loop = smithay::reexports::calloop::EventLoop::try_new().unwrap(); let display = smithay::reexports::wayland_server::Display::new().unwrap(); let mut state = crate::Ferese::new(&mut event_loop, display, runtime).unwrap(); diff --git a/compositor/src/render/materials.rs b/compositor/src/render/materials.rs index d657402..e169a43 100644 --- a/compositor/src/render/materials.rs +++ b/compositor/src/render/materials.rs @@ -697,7 +697,8 @@ mod region_geometry_tests { let source = format!( "theme {{ material {{ style translucent; opacity 0.7; }}; accessibility {{ reduce-transparency #{reduce}; }}; }}; panel main {{ style {surface}; background-opacity {opacity}; }}" ); - let (_, runtime, candidate) = crate::theme::prepare(&source, std::path::Path::new("/tmp")).unwrap(); + let (_, runtime, candidate) = + crate::config::prepare(&source, std::path::Path::new("/tmp")).unwrap(); let material = resolve_surface_material( SemanticRole::Panel, runtime.theme_settings, @@ -736,7 +737,7 @@ mod region_geometry_tests { let source = format!( "theme {{ material {{ style translucent; opacity 0.6; blur-radius 20; }}; accessibility {{ reduce-transparency #{reduce}; }}; }}; panel main {{ style morph; }}" ); - let (_, runtime, candidate) = crate::theme::prepare(&source, std::path::Path::new("/tmp")).unwrap(); + let (_, runtime, candidate) = crate::config::prepare(&source, std::path::Path::new("/tmp")).unwrap(); let material = resolve_surface_material(SemanticRole::Panel, runtime.theme_settings, None, runtime.panel_style); assert_eq!(material.shadow, [0.; 3]); diff --git a/compositor/src/startup_tests.rs b/compositor/src/startup_tests.rs index 3f93efe..b3f9dba 100644 --- a/compositor/src/startup_tests.rs +++ b/compositor/src/startup_tests.rs @@ -273,11 +273,11 @@ fn check_config_rejects_an_otherwise_valid_oversized_file() { let valid = format!("//{}\n", "x".repeat(60 * 1024 - 3)); std::fs::write(&path, &valid).unwrap(); crate::check_config(&path).unwrap(); - assert!(crate::theme::prepare(&(valid.clone() + "\n"), directory.path()).is_ok()); + assert!(crate::config::prepare(&(valid.clone() + "\n"), directory.path()).is_ok()); std::fs::write(&path, valid + "\n").unwrap(); let error = crate::check_config(&path).unwrap_err(); assert!(error.contains("exceeds 60 KiB"), "{error}"); - assert_eq!(crate::theme::read_source(&path).unwrap_err(), error); + assert_eq!(crate::config::read_source(&path).unwrap_err(), error); } #[test] diff --git a/compositor/src/theme.rs b/compositor/src/theme.rs index 23ff311..617c318 100644 --- a/compositor/src/theme.rs +++ b/compositor/src/theme.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::io::{self, Read}; +use std::io; use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, mpsc}; @@ -20,9 +20,9 @@ use serde_json::{Value, json}; use smithay::backend::renderer::gles::GlesRenderer; use smithay::output::Output; +use crate::Ferese; use crate::config::Config; use crate::wallpaper::{WallpaperConfig, WallpaperState}; -use crate::{Ferese, RuntimeConfig}; const DEBOUNCE: Duration = Duration::from_millis(150); const WALLPAPER_TIMEOUT: Duration = Duration::from_secs(2); @@ -35,34 +35,14 @@ fn transition_progress(elapsed: Duration, duration: Duration) -> f64 { } } -pub(crate) fn read_source(path: &Path) -> Result { - let mut source = String::new(); - std::fs::File::open(path) - .map_err(|e| format!("{}: {e}", path.display()))? - .take(60 * 1024 + 1) - .read_to_string(&mut source) - .map_err(|e| e.to_string())?; - if source.len() > 60 * 1024 { - return Err(format!("{} exceeds 60 KiB", path.display())); - } - Ok(source) -} - -pub(crate) fn prepare(source: &str, directory: &Path) -> Result<(Config, RuntimeConfig, Candidate), String> { - let document = Document::parse(source).map_err(|e| e.to_string())?; - prepare_document(&document, directory) -} - -pub(crate) fn prepare_document( - document: &Document, - directory: &Path, -) -> Result<(Config, RuntimeConfig, Candidate), String> { - let candidate = resolve_with_context(document, directory, Timestamp::now(), &auto_context(), read_source)?; - let config: Config = - serde_json::from_value(document.with_theme(&candidate.theme).value().clone()).map_err(|e| e.to_string())?; - let mut runtime = config.runtime_config().map_err(|e| e.to_string())?; - runtime.theme_settings = Config::resolved_theme_settings(&candidate.theme).map_err(|e| e.to_string())?; - Ok((config, runtime, candidate)) +pub(crate) fn resolve_candidate(document: &Document, directory: &Path) -> Result { + resolve_with_context( + document, + directory, + Timestamp::now(), + &auto_context(), + crate::config::read_source, + ) } pub(crate) fn preview(args: &Value) -> Result { @@ -76,7 +56,7 @@ pub(crate) fn preview(args: &Value) -> Result { .or_else(|| crate::config::config_path().and_then(|path| path.parent().map(ToOwned::to_owned))) .unwrap_or_else(|| PathBuf::from(".")); let document = Document::parse(source).map_err(|e| e.to_string())?; - let candidate = resolve_with_context(&document, &directory, Timestamp::now(), &auto_context(), read_source)?; + let candidate = resolve_candidate(&document, &directory)?; Ok( json!({"theme": candidate.theme, "warnings": candidate.warnings, "families": candidate.families, "fallback_note": candidate.fallback_note}), ) @@ -165,6 +145,7 @@ pub(crate) struct Engine { pending: Option, transition: Option, last_frame: Option, + startup_rejection: Option, } impl Engine { @@ -207,7 +188,21 @@ impl Engine { } } + pub(crate) fn startup_failed(&mut self, path: Option<&Path>, error: String) { + let path = path + .map(|path| path.display().to_string()) + .unwrap_or_else(|| "configuration".into()); + self.live.error = Some(format!( + "Ferese started with default settings because {path} could not be loaded. Your file has not been changed.\n\n{error}" + )); + self.startup_rejection = Some(error); + } + pub(crate) fn reject(&mut self, error: String) { + if self.startup_rejection.as_ref() == Some(&error) { + return; + } + self.startup_rejection = None; if self.live.error.as_ref() != Some(&error) { tracing::warn!(%error, "theme reload rejected; retaining active theme"); self.live.error = Some(error); @@ -400,6 +395,7 @@ impl Ferese { } pub(crate) fn accept_theme(&mut self, candidate: Candidate, wallpaper: WallpaperConfig) { + self.theme_engine.startup_rejection = None; self.theme_engine.active_files = candidate.files.clone(); self.theme_engine.watch_files(&candidate.files); if let Err(error) = self.theme_engine.arm_clock(candidate.next_transition) { @@ -584,11 +580,11 @@ impl Ferese { pub(crate) fn set_theme_mode(&mut self, mode: Mode) -> Result { let path = crate::config::config_path().ok_or("No configuration directory")?; - let original = read_source(&path)?; + let original = crate::config::read_source(&path)?; let mut document = Document::parse(&original).map_err(|e| e.to_string())?; document.set("theme.mode", json!(mode)).map_err(|e| e.to_string())?; let source = document.to_string(); - prepare(&source, path.parent().unwrap_or(Path::new(".")))?; + crate::config::prepare(&source, path.parent().unwrap_or(Path::new(".")))?; let temporary = path.with_extension(format!("kdl.{}.tmp", std::process::id())); let result = (|| -> Result<(), String> { use std::io::Write; @@ -603,7 +599,7 @@ impl Ferese { } file.write_all(source.as_bytes()).map_err(|e| e.to_string())?; file.sync_all().map_err(|e| e.to_string())?; - if read_source(&path)? != original { + if crate::config::read_source(&path)? != original { return Err("Configuration changed while updating the mode; please try again".into()); } std::fs::rename(&temporary, &path).map_err(|e| e.to_string())?; @@ -622,6 +618,26 @@ impl Ferese { mod tests { use super::*; + #[test] + fn startup_warning_survives_repeated_rejection_without_duplicate_publication() { + let mut engine = Engine::default(); + engine.startup_failed(Some(Path::new("/config.kdl")), "unknown field group_surface".into()); + engine.publish(); + let initial = engine.value(); + assert!(initial["error"].as_str().unwrap().contains("default settings")); + assert!(initial["error"].as_str().unwrap().contains("/config.kdl")); + for _ in 0..3 { + engine.reject("unknown field group_surface".into()); + assert_eq!(engine.value(), initial); + } + engine.reject("different invalid edit".into()); + assert_eq!(engine.live.error.as_deref(), Some("different invalid edit")); + assert_eq!(engine.revision, 2); + engine.reject("unknown field group_surface".into()); + assert_eq!(engine.live.error.as_deref(), Some("unknown field group_surface")); + assert_eq!(engine.revision, 3); + } + #[test] fn publication_projects_live_values_and_preserves_revision_semantics() { let mut engine = Engine::default(); diff --git a/compositor/src/xwayland/integration_tests.rs b/compositor/src/xwayland/integration_tests.rs index 151155b..bcfce10 100644 --- a/compositor/src/xwayland/integration_tests.rs +++ b/compositor/src/xwayland/integration_tests.rs @@ -646,7 +646,7 @@ mod manager_loop { /// A `Ferese` wired to a real event loop, ready to run the manager. fn session(directory: &Path, path: PathBuf) -> (EventLoop<'static, Ferese>, Ferese) { - let (_, runtime, _) = crate::theme::prepare("xwayland { }", directory).expect("prepare configuration"); + let (_, runtime, _) = crate::config::prepare("xwayland { }", directory).expect("prepare configuration"); // Owned inputs, so this test never competes for a real display slot. let allocator = directory.join("x11"); fs::create_dir_all(&allocator).expect("create the allocator directory"); diff --git a/docs/configuration.md b/docs/configuration.md index 21320d3..99a4d8a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -68,9 +68,10 @@ appearance, wallpaper, motion, input, bindings, rules, layouts, displays, and login items take effect live. If a reload is invalid, Ferese reports the error and keeps the last working configuration. -When the shell starts, it loads your configuration or, if no file exists, uses -the defaults. An invalid file stops startup with an error message, leaving your -configuration untouched. +At login, Ferese loads your configuration or uses the built-in defaults if the +file is missing or invalid. For an invalid file, it shows a notification once +the desktop is ready and leaves the file untouched, so you can correct it to +match the current schema and reload your settings. ```sh feresectl reload-config # reload with error feedback diff --git a/scripts/ci/run-native-checks.py b/scripts/ci/run-native-checks.py index 1ca5ffd..30bebdc 100644 --- a/scripts/ci/run-native-checks.py +++ b/scripts/ci/run-native-checks.py @@ -60,6 +60,7 @@ def main(): cases = [ ("shell-flat", "shell_motion", {"FERESE_TEST_SHELL_MOTION": "1", "FERESE_TEST_BAR_LAYOUT": "flat"}), ("shell-floating", "shell_motion", {"FERESE_TEST_SHELL_MOTION": "1", "FERESE_TEST_BAR_LAYOUT": "floating"}), + ("invalid-config", "startup_config", {"FERESE_TEST_STARTUP_CONFIG": "1"}), ] if args.suite == "desktop": cases.insert(0, ("lock", "lock", {"FERESE_TEST_LOCK": "1", "FERESE_TEST_BIN_DIR": "target/debug"})) diff --git a/scripts/tests/test_shell_motion_isolated.py b/scripts/tests/test_shell_motion_isolated.py index 16a7691..f4ed3ac 100644 --- a/scripts/tests/test_shell_motion_isolated.py +++ b/scripts/tests/test_shell_motion_isolated.py @@ -162,6 +162,22 @@ class ShellMotionTest(unittest.TestCase): or other == rectangle and other_index < index for other_index, other in enumerate(rectangles))] + def bar_input_size(surface): + rectangles = bar_input_rectangles(surface) + if not rectangles: + return None + return (max(x + width for x, y, width, height in rectangles) + - min(x for x, y, width, height in rectangles), + max(y + height for x, y, width, height in rectangles) + - min(y for x, y, width, height in rectangles)) + + def bar_motion_settled(surface): + updates = log_path.read_text().count('.set_panel_presentation(') + before = commits(surface) + time.sleep(.25) + return (log_path.read_text().count('.set_panel_presentation(') == updates + and commits(surface) - before <= 2) + with log_path.open("w") as log: try: @@ -317,16 +333,26 @@ class ShellMotionTest(unittest.TestCase): subprocess.run([str(ctl), "reload-config"], env=ipc_environment(env), check=True, capture_output=True, timeout=5) wait_for(lambda: bar_region_count(bar) == regions) self.assertTrue(bar_input_rectangles(bar), style + " lost its native input") - config_file.write_text('animations { speed 0.5; }\nstatus { keybinding-guide #false; }\nnotifications { timeout-ms 1200; }\n' + panel_config("morph")) + config_file.write_text('animations { speed 0.5; }\nstatus { keybinding-guide #false; }\nnotifications { timeout-ms 1200; }\n' + + panel_config("morph").replace('style "morph"', 'style "morph"\n morph { peek-on-hover #false; }')) subprocess.run([str(ctl), "reload-config"], env=ipc_environment(env), check=True, capture_output=True, timeout=5) - wait_for(lambda: bar_size(bar) == (150, 34) and bar_region_count(bar) == 1) + # The visible hub follows its measured content; its larger + # backing surface remains allocated to avoid resize flashes. + wait_for(lambda: (size := bar_input_size(bar)) and 0 < size[0] < 320 + and size[1] == 34 and bar_region_count(bar) == 1) reply = dbus("Notify", "Morph test", "0", "", "Hub notification", "Bounded native surface", "[]", "{}", "0").stdout notice = re.search(r'uint32 (\d+)', reply).group(1) - wait_for(lambda: bar_size(bar) == (320, 48)) + wait_for(lambda: bar_input_size(bar) == (320, 48)) + allocation = bar_size(bar) + self.assertGreaterEqual(allocation[0], 320) + self.assertGreaterEqual(allocation[1], 48) # Removing a notice must collapse its hub immediately; an old timeout cannot reopen it. dbus("CloseNotification", notice) - wait_for(lambda: bar_size(bar) == (150, 34)) - time.sleep(.25) + wait_for(lambda: (size := bar_input_size(bar)) and 0 < size[0] < 320 and size[1] == 34) + self.assertEqual(bar_size(bar), allocation, "closing Morph resized its retained backing surface") + # Rounded protocol geometry can stop changing while subpixel + # spring motion or the content fade still needs frames. + wait_for(lambda: bar_motion_settled(bar)) before = commits(bar) trace_start = len(log_path.read_text()) time.sleep(.5) diff --git a/scripts/tests/test_startup_config_isolated.py b/scripts/tests/test_startup_config_isolated.py new file mode 100644 index 0000000..7a15ecb --- /dev/null +++ b/scripts/tests/test_startup_config_isolated.py @@ -0,0 +1,140 @@ +"""Check invalid-config startup, its notification and recovery on a private desktop. + +FERESE_TEST_STARTUP_CONFIG=1 python3 scripts/tests/test_startup_config_isolated.py +Uses release binaries by default. Requires a Wayland host, dbus-daemon and gdbus. +""" +import json +import os +from pathlib import Path +import re +import signal +import subprocess +import tempfile +import time +import unittest + +from session_socket import ipc_environment, ipc_socket + + +@unittest.skipUnless(os.environ.get("FERESE_TEST_STARTUP_CONFIG") == "1", "opt-in nested startup test") +class StartupConfigTest(unittest.TestCase): + def test_defaults_notification_and_recovery_preserve_strict_validation(self): + repo = Path(__file__).resolve().parents[2] + binary = repo / os.environ.get("FERESE_TEST_BINARY", "target/release/ferese") + shell = repo / os.environ.get("FERESE_TEST_SHELL", "target/release/ferese-shell") + ctl = repo / os.environ.get("FERESE_TEST_CTL", "target/release/feresectl") + with tempfile.TemporaryDirectory(prefix="ferese-startup-config-") as temporary: + root = Path(temporary) + runtime = root / "runtime" + runtime.mkdir(mode=0o700) + directory = root / "config/ferese" + directory.mkdir(parents=True) + path = directory / "config.kdl" + invalid = ('// keep this file\n' + 'notifications { do-not-disturb #true; show-popups #false; }\n' + 'panel main { surface solid; group-surface none; }\n') + path.write_text(invalid) + checked = subprocess.run([str(binary), "--check-config", str(path)], + capture_output=True, text=True, timeout=15) + self.assertNotEqual(checked.returncode, 0) + self.assertRegex(checked.stderr, r"unknown field `(surface|group_surface)`") + + display = Path(os.environ["WAYLAND_DISPLAY"]) + if not display.is_absolute(): + display = Path(os.environ["XDG_RUNTIME_DIR"]) / display + env = dict(os.environ, WAYLAND_DISPLAY=str(display), XDG_RUNTIME_DIR=str(runtime), + XDG_CONFIG_HOME=str(root / "config"), XDG_STATE_HOME=str(root / "state"), + XDG_CACHE_HOME=str(root / "cache"), XDG_DATA_HOME=str(root / "data")) + for name in ("WAYLAND_SOCKET", "FERESE_SHELL_CONTROL_SOCKET", "DISPLAY", "XAUTHORITY"): + env.pop(name, None) + # No activation directories: every notification stays on this private bus. + bus_config = root / "bus.conf" + bus_config.write_text( + 'sessionunix:tmpdir=/tmp' + '' + '') + log_path = root / "session.log" + children = [] + with log_path.open("w") as log: + def launch(command, **options): + child = subprocess.Popen(command, env=ipc_environment(env), start_new_session=True, + stderr=log, **options) + children.append(child) + return child + + def control(*args, check=True): + return subprocess.run([str(ctl), *args], env=ipc_environment(env), capture_output=True, + text=True, timeout=5, check=check) + + def theme(): + result = control("theme", "get", "--json", check=False) + return json.loads(result.stdout) if result.returncode == 0 else None + + def wait_for(predicate): + deadline = time.monotonic() + 30 + while time.monotonic() < deadline: + value = predicate() + if value: + return value + if any(child.poll() is not None for child in children): + self.fail("A test process exited:\n" + log_path.read_text()[-10000:]) + time.sleep(.05) + self.fail("Timed out:\n" + log_path.read_text()[-10000:]) + + def notification_presented(): + trace = log_path.read_text() + surface = re.search( + r'get_layer_surface\([^\n]*wl_surface[@#](\d+)[^\n]*"ferese-shell-notifications"', trace) + if not surface: + return False + identity = surface.group(1) + lifetime = trace[surface.start():] + attached = re.search(r'wl_surface[@#]' + identity + r'\.attach\(wl_buffer[@#]', lifetime) + return attached is not None and re.search( + r'wl_surface[@#]' + identity + r'\.commit\(', lifetime[attached.end():]) is not None + + try: + bus = launch(["dbus-daemon", "--config-file=" + str(bus_config), "--nofork", "--print-address=1"], + stdout=subprocess.PIPE, text=True) + env["DBUS_SESSION_BUS_ADDRESS"] = bus.stdout.readline().strip() + bus.stdout.close() + launch([str(binary), "--backend", "nested", "--grant-effects", "--grant-shell-control", + "--", "env", "WAYLAND_DEBUG=client", "ICED_BACKEND=tiny-skia", str(shell)], stdout=log) + wait_for(lambda: ipc_socket(runtime).is_socket()) + snapshot = wait_for(theme) + self.assertIn("default settings", snapshot["error"]) + self.assertIn(str(path), snapshot["error"]) + wait_for(lambda: '"ferese-shell-top-bar"' in log_path.read_text()) + wait_for(lambda: "ferese-shell: configuration warning:" in log_path.read_text()) + wait_for(notification_presented) + self.assertEqual(path.read_text(), invalid) + + valid = '// corrected by the user\ntheme { accent "#83AA5F"; }\n' + path.write_text(valid) + control("reload-config") + accepted = wait_for(lambda: (value := theme()) and value["error"] is None and value) + self.assertEqual(accepted["theme"]["requested_accent"], "#83AA5F") + self.assertNotEqual(accepted["theme"]["tokens"]["colors"]["accent"], + snapshot["theme"]["tokens"]["colors"]["accent"]) + self.assertEqual(path.read_text(), valid) + self.assertEqual(log_path.read_text().count("ferese-shell: configuration warning:"), 1) + + path.write_text(invalid) + rejected = control("reload-config", check=False) + self.assertNotEqual(rejected.returncode, 0) + retained = wait_for(lambda: (value := theme()) and value["error"] and value) + self.assertEqual(retained["theme"], accepted["theme"]) + self.assertEqual(path.read_text(), invalid) + finally: + for child in reversed(children): + if child.poll() is None: + os.killpg(child.pid, signal.SIGINT) + try: + child.wait(timeout=8) + except subprocess.TimeoutExpired: + os.killpg(child.pid, signal.SIGKILL) + child.wait(timeout=5) + + +if __name__ == "__main__": + unittest.main() diff --git a/shell/ferese-shell/src/config.rs b/shell/ferese-shell/src/config.rs index 9bc5750..87c0d3f 100644 --- a/shell/ferese-shell/src/config.rs +++ b/shell/ferese-shell/src/config.rs @@ -1,4 +1,5 @@ use std::fs; +use std::io::Read; use std::path::PathBuf; use serde::Deserialize; @@ -349,7 +350,16 @@ fn load_path(path: &std::path::Path) -> Result { Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(ShellConfig::default()), Err(error) => return Err(format!("cannot read {}: {error}", path.display())), } - let source = fs::read_to_string(path).map_err(|error| format!("cannot read {}: {error}", path.display()))?; + let mut source = String::new(); + fs::File::open(path) + .and_then(|file| { + file.take((ferese_config::file::LIMIT + 1) as u64) + .read_to_string(&mut source) + }) + .map_err(|error| format!("cannot read {}: {error}", path.display()))?; + if source.len() > ferese_config::file::LIMIT { + return Err(format!("{} exceeds 60 KiB", path.display())); + } parse_source(&source).map_err(|error| format!("invalid configuration in {}: {error}", path.display())) } @@ -781,7 +791,7 @@ theme { } #[test] - fn startup_rejects_invalid_configuration_without_replacing_it() { + fn loader_reports_invalid_configuration_without_replacing_it() { let directory = tempfile::tempdir().unwrap(); let path = directory.path().join("config.kdl"); let source = @@ -793,6 +803,10 @@ theme { assert_eq!(std::fs::read_to_string(&path).unwrap(), source); std::fs::write(&path, "panel main {").unwrap(); assert!(super::load_path(&path).is_err()); + let oversized = " ".repeat(ferese_config::file::LIMIT + 1); + std::fs::write(&path, &oversized).unwrap(); + assert!(super::load_path(&path).unwrap_err().contains("60 KiB")); + assert_eq!(std::fs::read_to_string(&path).unwrap(), oversized); } #[test] diff --git a/shell/ferese-shell/src/main.rs b/shell/ferese-shell/src/main.rs index ddd2f80..82a5690 100644 --- a/shell/ferese-shell/src/main.rs +++ b/shell/ferese-shell/src/main.rs @@ -100,10 +100,16 @@ impl From for BarMetrics { } fn main() -> cosmic::iced::Result { - let mut config = config::load().unwrap_or_else(|error| { - eprintln!("ferese-shell: {error}; shell was not started"); - std::process::exit(2); - }); + let (mut config, startup_error) = match config::load() { + Ok(config) => (config, None), + Err(error) => { + eprintln!("ferese-shell: {error}; starting with defaults"); + let notice = format!( + "The shell started with default settings. Your configuration file has not been changed.\n\n{error}" + ); + (ShellConfig::default(), Some(notice)) + } + }; renderer::configure_shell(); let compositor_wallpaper = std::env::var_os("FERESE_COMPOSITOR_WALLPAPER").is_some(); @@ -133,7 +139,7 @@ fn main() -> cosmic::iced::Result { .default_font(shell_font) .theme(config.theme.palette().native_theme()); - cosmic::app::run::(settings, (config, wallpaper)) + cosmic::app::run::(settings, (config, wallpaper, startup_error)) } type WallpaperLoad = cosmic::iced::futures::channel::oneshot::Receiver>; @@ -158,6 +164,7 @@ struct FereseShell { calendar_offset: i32, status_error: Option, theme_error: Option, + pending_config_error: Option, menu: Option, system_modal: Option, display_mode: display_mode::Model, @@ -267,7 +274,7 @@ enum Message { impl cosmic::Application for FereseShell { type Executor = cosmic::executor::Default; - type Flags = (ShellConfig, Option); + type Flags = (ShellConfig, Option, Option); type Message = Message; const APP_ID: &'static str = APP_ID; @@ -280,7 +287,13 @@ impl cosmic::Application for FereseShell { &mut self.core } - fn init(core: Core, (config, wallpaper): Self::Flags) -> (Self, Task) { + fn init(core: Core, (config, wallpaper, startup_error): Self::Flags) -> (Self, Task) { + let control = ShellControl::connect() + .map_err(|error| eprintln!("ferese-shell: shell control unavailable: {error}")) + .ok(); + // The compositor reports the complete configuration error through its + // accepted-theme snapshot. Use the local warning only when standalone. + let pending_config_error = if control.is_none() { startup_error } else { None }; let app = Self { core, notifications: notifications::Center::new(config.notifications.clone(), config.animations), @@ -292,6 +305,7 @@ impl cosmic::Application for FereseShell { calendar_offset: 0, status_error: None, theme_error: None, + pending_config_error, menu: None, system_modal: None, display_mode: display_mode::Model::default(), @@ -302,11 +316,7 @@ impl cosmic::Application for FereseShell { render: rendering::Rendering::new(&config), config, wallpaper: None, - control: ShellControl::connect() - .map_err(|error| { - eprintln!("ferese-shell: shell control unavailable: {error}"); - }) - .ok(), + control, snapshot: ShellSnapshot::default(), workspace_ui: Default::default(), overview_active: false, @@ -366,7 +376,7 @@ impl cosmic::Application for FereseShell { } fn update(&mut self, message: Self::Message) -> Task { - match message { + let task = match message { Message::HubOpen(id, kind) => self.open_hub(id, kind), Message::HubClose(id) => self.close_hub(id), Message::HubWorkspaceScroll(id, delta) => self.scroll_hub_workspaces(id, delta), @@ -471,9 +481,7 @@ impl cosmic::Application for FereseShell { config.apply_theme(&snapshot.presented); config.theme_mode = snapshot.mode; if snapshot.error != self.theme_error { - if let Some(error) = &snapshot.error { - self.notifications.service_error("Theme could not be loaded", error); - } + self.pending_config_error = snapshot.error.clone(); self.theme_error = snapshot.error; } self.apply_config(config) @@ -875,7 +883,9 @@ impl cosmic::Application for FereseShell { } self.dismiss_workspace_tooltip() } - } + }; + self.flush_config_error(); + task } fn style(&self) -> Option { @@ -894,6 +904,17 @@ impl cosmic::Application for FereseShell { } impl FereseShell { + fn flush_config_error(&mut self) { + if self.notifications.ready + && self.outputs.iter().any(|output| !output.hidden) + && let Some(error) = self.pending_config_error.take() + { + eprintln!("ferese-shell: configuration warning: {error}"); + self.notifications + .service_error("Configuration could not be loaded", &error); + } + } + fn reload_config(&mut self, source: String) -> Task { let config = match config::parse_source(&source) { Ok(config) => config, @@ -1080,6 +1101,7 @@ mod tests { calendar_offset: 0, status_error: None, theme_error: None, + pending_config_error: None, menu: None, system_modal: None, display_mode: Default::default(),