From 91e2f227d25ae077af2f0f5d8f018dbe3f98f348 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Sun, 11 Oct 2026 12:15:28 +0100 Subject: [PATCH] Fix config upgrade failures, save races, lock confirmation and release validation (#23) * Coordinate config writes and gate releases on desktop validation * Use toolkit rc.2 with painting and session lock fixes * Ignore retired desktop widget settings in existing configs --- .github/workflows/release-checks.yml | 71 +++++++- Cargo.lock | 36 ++-- Cargo.toml | 3 +- crates/ferese-config/Cargo.toml | 2 - crates/ferese-config/src/file.rs | 182 ++++++++++++++++++++ crates/ferese-config/src/lib.rs | 16 +- crates/ferese-theme-client/src/service.rs | 1 + crates/ferese-theme/Cargo.toml | 2 +- portal/src/desktop.rs | 79 +++++++-- scripts/ci/run-native-checks.py | 69 +++++--- scripts/tests/test_lock_isolated.py | 18 +- scripts/tests/test_shell_motion_isolated.py | 9 +- shell/ferese-lock/src/main.rs | 26 ++- shell/ferese-lock/src/runtime.rs | 67 ++++--- shell/ferese-settings/Cargo.toml | 2 +- shell/ferese-settings/src/store.rs | 48 ++---- shell/ferese-settings/src/watch.rs | 58 ++++++- shell/ferese-shell/Cargo.toml | 2 +- 18 files changed, 536 insertions(+), 155 deletions(-) create mode 100644 crates/ferese-config/src/file.rs diff --git a/.github/workflows/release-checks.yml b/.github/workflows/release-checks.yml index 6eda390..92ffecd 100644 --- a/.github/workflows/release-checks.yml +++ b/.github/workflows/release-checks.yml @@ -1,7 +1,10 @@ name: Workspace release checks on: - release: - types: [published] + pull_request: + push: + branches: [main] + tags: ["v*"] + workflow_dispatch: permissions: contents: read concurrency: @@ -28,7 +31,7 @@ jobs: mesa-libgbm-devel mesa-libEGL-devel mesa-dri-drivers mesa-vulkan-drivers libdrm-devel fontconfig-devel \ freetype-devel expat-devel dbus-daemon dbus-tools desktop-file-utils foot pam xorg-x11-xauth \ clang clang-devel pipewire-devel pipewire wireplumber polkit polkit-libs libei \ - xdg-desktop-portal xdg-desktop-portal-gtk \ + xdg-desktop-portal xdg-desktop-portal-gtk weston \ gstreamer1-devel gstreamer1-plugins-base gstreamer1-plugins-good pipewire-gstreamer - name: Create private test runtime directory run: install -d -m 0700 "$XDG_RUNTIME_DIR" @@ -46,5 +49,67 @@ jobs: run: | cargo build --locked --workspace cargo test --locked --workspace --no-fail-fast + - name: Check release tag matches the workspace version + if: startsWith(github.ref, 'refs/tags/') + env: + RELEASE_TAG: ${{ github.ref_name }} + run: | + python3 - <<'PYTHON' + import os, tomllib + with open("Cargo.toml", "rb") as source: + version = tomllib.load(source)["workspace"]["package"]["version"] + assert os.environ["RELEASE_TAG"] == "v" + version, "Tag must match workspace.package.version" + PYTHON + - name: Test capture privacy and failure recovery + env: + LIBGL_ALWAYS_SOFTWARE: "1" + EGL_PLATFORM: surfaceless + run: | + cargo test --locked -p ferese state::capture_privacy::tests -- --ignored + cargo test --locked -p ferese backends::direct::capture::tests::capture_recomposes_cursor_without_touching_the_display_target -- --ignored + cargo test --locked -p ferese winit::tests::capture_failure_restores_display_target_and_subsequent_frames_render -- --ignored + - name: Test native shell, visibility and resize behavior + run: python3 scripts/ci/run-native-checks.py --suite desktop --log-dir native-logs + - name: Upload native test logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: native-test-logs + path: native-logs + if-no-files-found: ignore - name: Test installer, session recovery and private portal contracts run: python3 -m unittest discover -s scripts/tests -v + + publish: + name: Publish validated tag + needs: workspace + if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags/v') + runs-on: ubuntu-24.04 + permissions: + contents: write + steps: + - uses: actions/github-script@v8 + with: + script: | + const tag = context.ref.slice('refs/tags/'.length); + const repo = context.repo; + const commit = await github.rest.repos.getCommit({...repo, ref: tag}); + if (commit.data.sha !== context.sha) { + throw new Error('Release tag moved after validation'); + } + let release; + try { + release = (await github.rest.repos.getReleaseByTag({...repo, tag})).data; + } catch (error) { + if (error.status !== 404) throw error; + } + if (release?.draft) { + await github.rest.repos.updateRelease({...repo, release_id: release.id, draft: false}); + } else if (!release) { + await github.rest.repos.createRelease({ + ...repo, tag_name: tag, name: tag, + generate_release_notes: true, prerelease: tag.includes('-') + }); + } else { + core.notice(`Release ${tag} is already published`); + } diff --git a/Cargo.lock b/Cargo.lock index 87362d7..c3e2093 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -705,7 +705,7 @@ dependencies = [ [[package]] name = "build_helpers" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "cfg_aliases", ] @@ -1124,7 +1124,7 @@ dependencies = [ [[package]] name = "cosmic-config" version = "1.0.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "atomicwrites", "cosmic-config-derive", @@ -1145,7 +1145,7 @@ dependencies = [ [[package]] name = "cosmic-config-derive" version = "1.0.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "quote", "syn 2.0.119", @@ -1214,7 +1214,7 @@ dependencies = [ [[package]] name = "cosmic-theme" version = "1.0.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "almost", "configparser", @@ -2740,7 +2740,7 @@ dependencies = [ [[package]] name = "iced" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "build_helpers", "dnd", @@ -2762,7 +2762,7 @@ dependencies = [ [[package]] name = "iced_accessibility" version = "0.1.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "accesskit", "accesskit_winit", @@ -2771,7 +2771,7 @@ dependencies = [ [[package]] name = "iced_core" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "bitflags 2.13.2", "build_helpers", @@ -2799,7 +2799,7 @@ dependencies = [ [[package]] name = "iced_debug" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "iced_core", "iced_futures", @@ -2809,7 +2809,7 @@ dependencies = [ [[package]] name = "iced_futures" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "futures", "iced_core", @@ -2823,7 +2823,7 @@ dependencies = [ [[package]] name = "iced_graphics" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "bitflags 2.13.2", "bytemuck", @@ -2844,7 +2844,7 @@ dependencies = [ [[package]] name = "iced_program" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "iced_graphics", "iced_runtime", @@ -2853,7 +2853,7 @@ dependencies = [ [[package]] name = "iced_renderer" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "iced_graphics", "iced_tiny_skia", @@ -2865,7 +2865,7 @@ dependencies = [ [[package]] name = "iced_runtime" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "build_helpers", "bytes", @@ -2882,7 +2882,7 @@ dependencies = [ [[package]] name = "iced_tiny_skia" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "bytemuck", "cosmic-text", @@ -2899,7 +2899,7 @@ dependencies = [ [[package]] name = "iced_wgpu" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "as-raw-xcb-connection", "bitflags 2.13.2", @@ -2931,7 +2931,7 @@ dependencies = [ [[package]] name = "iced_widget" version = "0.14.2" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "build_helpers", "cosmic-client-toolkit", @@ -2951,7 +2951,7 @@ dependencies = [ [[package]] name = "iced_winit" version = "0.14.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "build_helpers", "cosmic-client-toolkit", @@ -3488,7 +3488,7 @@ checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" [[package]] name = "libcosmic" version = "1.0.0" -source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.1#170ddfe42ef7c58730685392fdc7185e8511938f" +source = "git+https://github.com/ferese-wm/libcosmic?tag=ferese-v0.1.3-rc.2#50ff247f14653b3d3a01affef3853a1ef5d8b349" dependencies = [ "apply", "ashpd 0.12.3", diff --git a/Cargo.toml b/Cargo.toml index 26d0e83..98932ac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,8 @@ bitflags = "2.9.4" calloop = { version = "0.14.4", features = ["signals"] } jiff = "0.2.37" libc = "0.2.176" -libcosmic = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.1", default-features = false, features = ["multi-window", "tokio", "wayland"] } +iced_accessibility = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.2" } +libcosmic = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.2", default-features = false, features = ["multi-window", "tokio", "wayland"] } smithay = { version = "0.7.0", default-features = false, features = [ "backend_drm", "backend_gbm", diff --git a/crates/ferese-config/Cargo.toml b/crates/ferese-config/Cargo.toml index 584e6fd..5380ca6 100644 --- a/crates/ferese-config/Cargo.toml +++ b/crates/ferese-config/Cargo.toml @@ -11,6 +11,4 @@ jiff.workspace = true kdl = "6.7.1" serde.workspace = true serde_json.workspace = true - -[dev-dependencies] tempfile.workspace = true diff --git a/crates/ferese-config/src/file.rs b/crates/ferese-config/src/file.rs new file mode 100644 index 0000000..384c793 --- /dev/null +++ b/crates/ferese-config/src/file.rs @@ -0,0 +1,182 @@ +use std::fs::{self, File, OpenOptions}; +use std::io::{Read, Write}; +use std::os::unix::fs::OpenOptionsExt; +use std::path::{Path, PathBuf}; + +pub const LIMIT: usize = 60 * 1024; + +pub struct ConfigFile { + path: PathBuf, + source: String, + // Keep the sidecar inode: deleting it would let another writer lock a new inode. + _lock: File, +} + +impl ConfigFile { + pub fn open(path: &Path) -> Result { + let parent = path + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + fs::create_dir_all(parent).map_err(|e| e.to_string())?; + + let path = match path.canonicalize() { + Ok(path) => path, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => parent + .canonicalize() + .map_err(|e| e.to_string())? + .join(path.file_name().ok_or("Invalid configuration path")?), + Err(error) => return Err(error.to_string()), + }; + let mut lock_name = path.file_name().ok_or("Invalid configuration path")?.to_os_string(); + lock_name.push(".lock"); + + let lock = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .mode(0o600) + .open(path.with_file_name(lock_name)) + .map_err(|e| e.to_string())?; + lock.try_lock().map_err(|error| match error { + fs::TryLockError::WouldBlock => "Configuration is being saved by another app. Please try again.".into(), + fs::TryLockError::Error(error) => error.to_string(), + })?; + + let source = read(&path)?; + + Ok(Self { + path, + source, + _lock: lock, + }) + } + + pub fn source(&self) -> &str { + &self.source + } + + pub fn replace( + self, + desired: &str, + backup: Option<&Path>, + validate: impl FnOnce(&Path) -> Result<(), String>, + ) -> Result<(), String> { + if desired.len() > LIMIT { + return Err("Configuration exceeds the 60 KiB limit.".into()); + } + + let parent = self.path.parent().ok_or("Invalid configuration path")?; + let mut staged = tempfile::NamedTempFile::new_in(parent).map_err(|e| e.to_string())?; + + if let Ok(metadata) = fs::metadata(&self.path) { + staged + .as_file() + .set_permissions(metadata.permissions()) + .map_err(|e| e.to_string())?; + } + + staged.write_all(desired.as_bytes()).map_err(|e| e.to_string())?; + staged.as_file().sync_all().map_err(|e| e.to_string())?; + validate(staged.path())?; + + if read(&self.path)? != self.source { + return Err("Configuration changed while saving. Please reload and try again.".into()); + } + + if let Some(path) = backup { + let mut previous = tempfile::NamedTempFile::new_in(parent).map_err(|e| e.to_string())?; + previous.write_all(self.source.as_bytes()).map_err(|e| e.to_string())?; + previous.as_file().sync_all().map_err(|e| e.to_string())?; + previous.persist(path).map_err(|e| e.to_string())?; + } + + staged.persist(&self.path).map_err(|e| e.to_string())?; + + File::open(parent) + .and_then(|directory| directory.sync_all()) + .map_err(|e| e.to_string())?; + + Ok(()) + } +} + +fn read(path: &Path) -> Result { + let file = match File::open(path) { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(String::new()), + Err(error) => return Err(error.to_string()), + }; + let mut source = String::new(); + file.take((LIMIT + 1) as u64) + .read_to_string(&mut source) + .map_err(|e| e.to_string())?; + if source.len() > LIMIT { + return Err("Configuration exceeds the 60 KiB limit.".into()); + } + Ok(source) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lock_excludes_other_processes_and_survives_replacement() { + const CHILD: &str = "FERESE_CONFIG_LOCK_TEST_PATH"; + if let Some(path) = std::env::var_os(CHILD) { + assert!( + ConfigFile::open(Path::new(&path)) + .err() + .unwrap() + .contains("another app") + ); + return; + } + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + for source in ["// first\n", "// second\n"] { + let file = ConfigFile::open(&path).unwrap(); + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "file::tests::lock_excludes_other_processes_and_survives_replacement", + ]) + .env(CHILD, &path) + .status() + .unwrap(); + assert!(status.success()); + file.replace(source, None, |_| Ok(())).unwrap(); + assert_eq!(ConfigFile::open(&path).unwrap().source(), source); + } + } + + #[test] + fn symlink_aliases_share_a_lock_and_preserve_the_link() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + let alias = directory.path().join("alias.kdl"); + fs::write(&path, "// initial").unwrap(); + std::os::unix::fs::symlink(&path, &alias).unwrap(); + let file = ConfigFile::open(&path).unwrap(); + assert!(ConfigFile::open(&alias).is_err()); + drop(file); + ConfigFile::open(&alias) + .unwrap() + .replace("// updated", None, |_| Ok(())) + .unwrap(); + assert!(alias.is_symlink()); + assert_eq!(fs::read_to_string(path).unwrap(), "// updated"); + } + + #[test] + fn failed_validation_releases_lock_and_preserves_contents() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + fs::write(&path, "// initial").unwrap(); + let file = ConfigFile::open(&path).unwrap(); + assert!(file.replace("// rejected", None, |_| Err("invalid".into())).is_err()); + assert_eq!(ConfigFile::open(&path).unwrap().source(), "// initial"); + } +} diff --git a/crates/ferese-config/src/lib.rs b/crates/ferese-config/src/lib.rs index 2babc96..f6d3863 100644 --- a/crates/ferese-config/src/lib.rs +++ b/crates/ferese-config/src/lib.rs @@ -1,4 +1,5 @@ pub mod families; +pub mod file; pub mod notifications; pub mod panel; pub mod presets; @@ -147,7 +148,8 @@ fn object(doc: &KdlDocument, parent: &str) -> Result, Error> for node in doc.nodes() { let key = field(node.name().value(), parent); if parent.is_empty() && key == "desktop_widgets" { - return Err(Error("Unknown configuration section: desktop-widgets".into())); + // Retired widgets stay in the source for edits but have no runtime effect. + continue; } let value = node_value(node, &key, parent)?; if is_records(&key, parent) { @@ -962,12 +964,16 @@ autostart "program" "two words" enabled=#false } #[test] - fn removed_desktop_widgets_are_rejected_without_affecting_authored_metadata() { + fn removed_desktop_widgets_are_ignored_without_affecting_authored_metadata() { for section in ["desktop-widgets", "desktop_widgets"] { let source = format!("{section} {{ clock {{ enabled #true; }}; }}"); - let error = Document::parse(&source).unwrap_err().to_string(); - assert!(error.contains("Unknown configuration section"), "{error}"); - assert!(crate::from_str::(&source).is_err()); + let doc = Document::parse(&source).unwrap(); + assert!(doc.get("desktop_widgets").is_none()); + assert_eq!(doc.to_string(), source); + assert_eq!( + crate::from_str::(&source).unwrap(), + serde_json::json!({}) + ); } assert!(Document::parse("panel main { metadata { desktop-widgets \"custom\"; }; }").is_ok()); } diff --git a/crates/ferese-theme-client/src/service.rs b/crates/ferese-theme-client/src/service.rs index 34ec09d..268d04b 100644 --- a/crates/ferese-theme-client/src/service.rs +++ b/crates/ferese-theme-client/src/service.rs @@ -90,6 +90,7 @@ mod tests { ]) .env(CHILD, "1") .env("XDG_RUNTIME_DIR", root.path()) + .env_remove("FERESE_SOCKET") .output() .unwrap(); assert!( diff --git a/crates/ferese-theme/Cargo.toml b/crates/ferese-theme/Cargo.toml index ba89904..6168a4c 100644 --- a/crates/ferese-theme/Cargo.toml +++ b/crates/ferese-theme/Cargo.toml @@ -11,7 +11,7 @@ ferese-theme-model = { path = "../ferese-theme-model" } ferese-config = { path = "../ferese-config" } libcosmic = { workspace = true, features = ["a11y"] } -iced_accessibility = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.1" } +iced_accessibility.workspace = true [dev-dependencies] tokio = { version = "1", features = ["rt"] } diff --git a/portal/src/desktop.rs b/portal/src/desktop.rs index d710c6a..2cb7a1a 100644 --- a/portal/src/desktop.rs +++ b/portal/src/desktop.rs @@ -251,8 +251,15 @@ pub(crate) fn ipc_sync(command: &'static str, args: serde_json::Value) -> Result command: command.into(), args, }; - ferese_ipc::write_frame(&mut stream, &request).map_err(|error| error.to_string())?; - let response: ferese_ipc::Response = ferese_ipc::read_frame(&mut stream).map_err(|error| error.to_string())?; + exchange(&mut stream, &request) +} + +fn exchange(stream: &mut UnixStream, request: &ferese_ipc::Request) -> Result { + ferese_ipc::write_frame(stream, request).map_err(|error| error.to_string())?; + let response: ferese_ipc::Response = ferese_ipc::read_frame(stream).map_err(|error| error.to_string())?; + if response.id != request.id || response.version != ferese_ipc::VERSION { + return Err("Unexpected compositor response ID or protocol version".into()); + } if let Some(error) = response.error { return Err(error.message); } @@ -751,12 +758,9 @@ fn edit_wallpaper( ) -> Result<(), String> { use ferese_config::theme::Appearance; - let source = match std::fs::read_to_string(path) { - Ok(source) => source, - Err(error) if error.kind() == std::io::ErrorKind::NotFound => String::new(), - Err(error) => return Err(error.to_string()), - }; - let mut document = ferese_config::Document::parse(&source).map_err(|error| error.to_string())?; + let file = ferese_config::file::ConfigFile::open(path)?; + let source = file.source(); + let mut document = ferese_config::Document::parse(source).map_err(|error| error.to_string())?; let directory = path.parent().ok_or("Invalid config path")?; let before = wallpaper_theme(&document, directory, appearance)?.tokens.background; let (variant, other) = match appearance { @@ -797,16 +801,7 @@ fn edit_wallpaper( return Err("Wallpaper change did not resolve to the requested image".into()); } - let parent = path.parent().ok_or("Invalid config path")?; - std::fs::create_dir_all(parent).map_err(|error| error.to_string())?; - let mut temp = tempfile::NamedTempFile::new_in(parent).map_err(|error| error.to_string())?; - std::io::Write::write_all(&mut temp, document.to_string().as_bytes()).map_err(|error| error.to_string())?; - temp.as_file().sync_all().map_err(|error| error.to_string())?; - if std::fs::read_to_string(path).unwrap_or_default() != source { - return Err("Configuration changed; please try again".into()); - } - temp.persist(path).map_err(|error| error.to_string())?; - Ok(()) + file.replace(&document.to_string(), None, |_| Ok(())) } #[cfg(test)] @@ -827,6 +822,37 @@ mod tests { use super::*; + #[test] + fn desktop_ipc_rejects_responses_for_another_request_or_protocol() { + for (version, id, accepted) in [ + (ferese_ipc::VERSION, 7, true), + (999, 7, false), + (ferese_ipc::VERSION, 8, false), + ] { + let (mut client, mut server) = UnixStream::pair().unwrap(); + client.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + server.set_read_timeout(Some(Duration::from_secs(2))).unwrap(); + let peer = std::thread::spawn(move || { + let _: ferese_ipc::Request = ferese_ipc::read_frame(&mut server).unwrap(); + let mut response = ferese_ipc::Response::success(id, serde_json::json!({"path": "/tmp/capture.png"})); + response.version = version; + ferese_ipc::write_frame(&mut server, &response).unwrap(); + }); + let result = exchange( + &mut client, + &ferese_ipc::Request { + version: ferese_ipc::VERSION, + id: 7, + kind: "command".into(), + command: "screenshot".into(), + args: serde_json::json!({}), + }, + ); + assert_eq!(result.is_ok(), accepted); + peer.join().unwrap(); + } + } + #[test] fn local_uris_preserve_special_characters_and_reject_network_paths() { let path = Path::new("/tmp/wallpaper #1_日本.png"); @@ -1003,6 +1029,23 @@ mod tests { ); } + #[test] + fn wallpaper_save_respects_the_shared_writer_lock() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + std::fs::write(&path, "// initial").unwrap(); + let _settings_save = ferese_config::file::ConfigFile::open(&path).unwrap(); + let error = edit_wallpaper( + &path, + Path::new("/wallpaper.png"), + "background", + ferese_config::theme::Appearance::Dark, + ) + .unwrap_err(); + assert!(error.contains("another app")); + assert_eq!(std::fs::read_to_string(path).unwrap(), "// initial"); + } + #[test] fn wallpaper_validation_failure_preserves_config() { let dir = tempfile::tempdir().unwrap(); diff --git a/scripts/ci/run-native-checks.py b/scripts/ci/run-native-checks.py index 1ccb46f..e77c19d 100644 --- a/scripts/ci/run-native-checks.py +++ b/scripts/ci/run-native-checks.py @@ -1,4 +1,4 @@ -"""Run native regressions against a private, headless Weston instance.""" +"""Run native regressions on a private software-rendered Weston host.""" import argparse import os from pathlib import Path @@ -9,22 +9,34 @@ import tempfile import time +def stop(process): + if process.poll() is None: + os.killpg(process.pid, signal.SIGINT) + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + os.killpg(process.pid, signal.SIGKILL) + process.wait(timeout=5) + + def main(): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--log-dir", type=Path, required=True) + parser.add_argument("--suite", choices=("panel-xwayland", "desktop"), default="panel-xwayland") args = parser.parse_args() logs = args.log_dir.resolve() logs.mkdir(parents=True, exist_ok=True) repo = Path(__file__).resolve().parents[2] - - # No inherited session endpoints or activation settings reach the test host. env = {key: value for key, value in os.environ.items() if not key.startswith("FERESE_") or key.startswith("FERESE_TEST_")} for key in ("DISPLAY", "WAYLAND_DISPLAY", "WAYLAND_SOCKET", "XAUTHORITY", "DBUS_SESSION_BUS_ADDRESS", "DBUS_SESSION_BUS_PID", "EGL_PLATFORM"): env.pop(key, None) env.update(LIBGL_ALWAYS_SOFTWARE="1", FERESE_TEST_LOG_DIR=str(logs)) - + for key, value in {"FERESE_TEST_BINARY": "target/debug/ferese", + "FERESE_TEST_CTL": "target/debug/feresectl", + "FERESE_TEST_SHELL": "target/debug/ferese-shell"}.items(): + env.setdefault(key, value) with tempfile.TemporaryDirectory(prefix="ferese-native-host-") as temporary: root = Path(temporary) runtime = root / "runtime" @@ -34,9 +46,10 @@ def main(): XDG_CACHE_HOME=str(root / "cache")) with (logs / "weston.log").open("w") as log: host = subprocess.Popen([ - env.get("FERESE_TEST_WESTON", "weston"), "--backend=headless-backend.so", - "--renderer=pixman", "--width=1600", "--height=1000", - "--socket=ferese-test-host", "--idle-time=0", "--no-config", + env.get("FERESE_TEST_WESTON", "weston"), + "--backend=headless-backend.so", "--renderer=pixman", + "--width=1600", "--height=1000", "--socket=ferese-test-host", + "--idle-time=0", "--no-config", ], env=env, stdout=log, stderr=log, start_new_session=True) try: deadline = time.monotonic() + 15 @@ -44,22 +57,34 @@ def main(): if host.poll() is not None or time.monotonic() >= deadline: raise RuntimeError((logs / "weston.log").read_text()) time.sleep(.05) - for layout in ("continuous", "islands"): - subprocess.run([ - sys.executable, "scripts/tests/test_shell_motion_isolated.py", "-v", - ], cwd=repo, env=dict(env, FERESE_TEST_SHELL_MOTION="1", - FERESE_TEST_BAR_LAYOUT=layout), check=True) - subprocess.run([ - sys.executable, "scripts/tests/test_xwayland_isolated.py", "-v", - ], cwd=repo, env=dict(env, FERESE_TEST_XWAYLAND="1"), check=True) + cases = [ + ("shell-continuous", "shell_motion", {"FERESE_TEST_SHELL_MOTION": "1", "FERESE_TEST_BAR_LAYOUT": "continuous"}), + ("shell-islands", "shell_motion", {"FERESE_TEST_SHELL_MOTION": "1", "FERESE_TEST_BAR_LAYOUT": "islands"}), + ] + if args.suite == "desktop": + cases.insert(0, ("lock", "lock", {"FERESE_TEST_LOCK": "1", "FERESE_TEST_BIN_DIR": "target/debug"})) + cases.extend([ + ("presentation", "presentation", {"FERESE_TEST_PRESENTATION": "1"}), + ("resize", "resize_dependencies", {"FERESE_TEST_RESIZE": "1"}), + ]) + else: + cases.append(("xwayland", "xwayland", {"FERESE_TEST_XWAYLAND": "1"})) + for name, script, options in cases: + print(f"Running {name}", flush=True) + with (logs / f"{name}-test.log").open("w") as output: + test = subprocess.Popen([ + sys.executable, f"scripts/tests/test_{script}_isolated.py", "-v", + ], cwd=repo, env=dict(env, **options), stdout=output, stderr=output, + start_new_session=True) + try: + status = test.wait(timeout=180) + if status: + raise RuntimeError((logs / f"{name}-test.log").read_text()) + finally: + stop(test) + print(f"Passed {name}", flush=True) finally: - if host.poll() is None: - os.killpg(host.pid, signal.SIGTERM) - try: - host.wait(timeout=5) - except subprocess.TimeoutExpired: - os.killpg(host.pid, signal.SIGKILL) - host.wait(timeout=5) + stop(host) if __name__ == "__main__": diff --git a/scripts/tests/test_lock_isolated.py b/scripts/tests/test_lock_isolated.py index 1e4b172..1fc9cee 100644 --- a/scripts/tests/test_lock_isolated.py +++ b/scripts/tests/test_lock_isolated.py @@ -1,6 +1,7 @@ """Opt-in lock protocol smoke test. Opens a temporary nested compositor. Run: FERESE_TEST_LOCK=1 python3 scripts/tests/test_lock_isolated.py -Requires built binaries and bwrap. FERESE_TEST_BIN_DIR selects debug/release. Host PAM is never modified. +Requires built binaries. FERESE_TEST_BIN_DIR selects debug/release. +No credentials are submitted; this test exercises protocol ownership, not PAM. """ import os import unittest @@ -16,12 +17,12 @@ class NativeLockTest(unittest.TestCase): runtime=root/'runtime';runtime.mkdir(mode=0o700) config=root/'config'/'ferese';config.mkdir(parents=True) (config/'config.kdl').write_text('') - pam=root/'pam';pam.mkdir() - # Deny-only policy confined to a mount namespace; never changes host PAM. - (pam/'ferese-lock').write_text('auth required pam_deny.so\naccount required pam_deny.so\n') host_display=os.environ.get('WAYLAND_DISPLAY','wayland-1') if not host_display.startswith('/'):host_display=str(pathlib.Path(os.environ['XDG_RUNTIME_DIR'])/host_display) - env=dict(os.environ,XDG_RUNTIME_DIR=str(runtime),XDG_CONFIG_HOME=str(root/'config'),WAYLAND_DISPLAY=host_display,FERESE_ENABLE_SCREENCOPY='1') + env=dict(os.environ,XDG_RUNTIME_DIR=str(runtime),XDG_CONFIG_HOME=str(root/'config'), + XDG_STATE_HOME=str(root/'state'),XDG_CACHE_HOME=str(root/'cache'), + WAYLAND_DISPLAY=host_display,FERESE_ENABLE_SCREENCOPY='1') + env.pop('WAYLAND_SOCKET', None) binary_dir = Path(os.environ.get('FERESE_TEST_BIN_DIR', 'target/release')) log=(root/'compositor.log').open('w') compositor=subprocess.Popen([str(binary_dir / 'ferese'),'--backend','nested'],env=ipc_environment(env),stdout=log,stderr=subprocess.STDOUT) @@ -35,7 +36,7 @@ class NativeLockTest(unittest.TestCase): sockets=[p for p in runtime.glob('wayland-*') if not p.name.endswith('.lock')] if sockets:break time.sleep(.1) - childenv=dict(env,WAYLAND_DISPLAY=str(sockets[0]),FERESE_LOCK_READY='1') + childenv=dict(env,WAYLAND_DISPLAY=str(sockets[0]),FERESE_LOCK_READY='1',WAYLAND_DEBUG='1') def capture(destination, timeout=10): with open(destination,'wb') as png: return subprocess.run([str(ctl),'screenshot'],env=ipc_environment(childenv),stdout=png,stderr=subprocess.PIPE,timeout=timeout) @@ -44,8 +45,9 @@ class NativeLockTest(unittest.TestCase): assert result.returncode == 0, f'Capture failed while unlocked: {result.stderr.decode()}' assert (root/'unlocked.png').stat().st_size > 0, 'Capture produced an empty file' print('Native capture succeeded while unlocked',flush=True) - locker=subprocess.Popen(['bwrap','--bind','/','/','--dev-bind','/dev','/dev','--ro-bind',str(pam),'/etc/pam.d','--unshare-user','--',str(binary_dir / 'ferese-lock')],env=ipc_environment(childenv),stdout=subprocess.DEVNULL,stderr=locker_log,start_new_session=True) + locker=subprocess.Popen([str(binary_dir / 'ferese-lock')],env=ipc_environment(childenv),stdout=subprocess.DEVNULL,stderr=locker_log,start_new_session=True) assert locker.wait(timeout=20) == 0, (root/'locker.log').read_text() + self.assertRegex((root/'locker.log').read_text(), r'ext_session_lock_v1[@#]\d+\.locked\(') print('Native locker received compositor confirmation',flush=True) result=capture(root/'locked.png') assert result.returncode != 0, 'Capture unexpectedly succeeded during lock' @@ -55,7 +57,7 @@ class NativeLockTest(unittest.TestCase): result=capture(root/'after-crash.png') assert result.returncode != 0, 'Capture unexpectedly succeeded after locker crash' print('Capture remains denied after locker crash',flush=True) - locker=subprocess.Popen(['bwrap','--bind','/','/','--dev-bind','/dev','/dev','--ro-bind',str(pam),'/etc/pam.d','--unshare-user','--',str(binary_dir / 'ferese-lock')],env=ipc_environment(childenv),stdout=subprocess.DEVNULL,stderr=locker_log,start_new_session=True) + locker=subprocess.Popen([str(binary_dir / 'ferese-lock')],env=ipc_environment(childenv),stdout=subprocess.DEVNULL,stderr=locker_log,start_new_session=True) assert locker.wait(timeout=20) == 0, (root/'locker.log').read_text() result=capture(root/'replacement.png') assert result.returncode != 0, 'Capture unexpectedly succeeded after locker takeover' diff --git a/scripts/tests/test_shell_motion_isolated.py b/scripts/tests/test_shell_motion_isolated.py index 911e8ef..ddd8ffd 100644 --- a/scripts/tests/test_shell_motion_isolated.py +++ b/scripts/tests/test_shell_motion_isolated.py @@ -174,7 +174,9 @@ class ShellMotionTest(unittest.TestCase): surface = wait_for(lambda: layer_surface("ferese-shell-notifications")) time.sleep(1.5) opening_frames = frames(surface) - self.assertGreater(opening_frames, 10, log_path.read_text()[-8000:]) + # Require multiple animation frames, without imposing a + # hardware-dependent frame rate on software-rendered CI. + self.assertGreaterEqual(opening_frames, 4, log_path.read_text()[-8000:]) time.sleep(1) for attempt in range(3): trace_start = len(log_path.read_text()) @@ -191,7 +193,7 @@ class ShellMotionTest(unittest.TestCase): material_updates = log_path.read_text().count('.set_presentation(') dbus("CloseNotification", notice) wait_for(lambda: destroyed(surface), timeout=5) - self.assertGreater(log_path.read_text().count('.set_presentation(') - material_updates, 5, + self.assertGreater(log_path.read_text().count('.set_presentation(') - material_updates, 1, "notification materials did not follow their fade") print(f"Toast: {opening_frames} opening callback requests, {idle_frames / 2:g} settled commits/s; destroyed", flush=True) @@ -199,9 +201,10 @@ class ShellMotionTest(unittest.TestCase): modal = wait_for(lambda: layer_surface("ferese-system-modal")) wait_for(lambda: frames(modal) >= 4) time.sleep(.3) + before_close = frames(modal) subprocess.run([str(ctl), "toggle-keybinding-guide"], env=ipc_environment(env), check=True, capture_output=True, timeout=5) wait_for(lambda: destroyed(modal), timeout=5) - self.assertGreater(frames(modal), 10, "modal close did not follow frames") + self.assertGreater(frames(modal), before_close, "modal close did not follow frames") self.assertIsNone(compositor.poll(), log_path.read_text()[-8000:]) self.assertNotIn("panicked at", log_path.read_text()) print(f"Modal: {frames(modal)} callbacks; interrupted opening closed and destroyed", flush=True) diff --git a/shell/ferese-lock/src/main.rs b/shell/ferese-lock/src/main.rs index 50f20a8..3fac325 100644 --- a/shell/ferese-lock/src/main.rs +++ b/shell/ferese-lock/src/main.rs @@ -354,22 +354,20 @@ impl Locker { } Event::PlatformSpecific(PlatformSpecific::Wayland(wayland::Event::SessionLock(event))) if !self.preview => { match event { - wayland::SessionLockEvent::Locked => { - if self.confirmation.observe() { - use std::io::Write; - if std::env::var_os("FERESE_LOCK_READY").is_some() { - let _ = writeln!(std::io::stdout().lock(), "FERESE_LOCKED"); - } - } - if !self.started { - self.started = true; - return Task::batch( - self.outputs - .iter() - .map(|(output, id)| session_lock::get_lock_surface(*id, output.clone())), - ); + wayland::SessionLockEvent::Locked if self.confirmation.observe(&event) => { + use std::io::Write; + if std::env::var_os("FERESE_LOCK_READY").is_some() { + let _ = writeln!(std::io::stdout().lock(), "FERESE_LOCKED"); } } + wayland::SessionLockEvent::Requested if !self.started => { + self.started = true; + return Task::batch( + self.outputs + .iter() + .map(|(output, id)| session_lock::get_lock_surface(*id, output.clone())), + ); + } wayland::SessionLockEvent::Focused(..) => return focus(), wayland::SessionLockEvent::Unlocked if self.state == AuthState::Unlocking => { return iced::exit(); diff --git a/shell/ferese-lock/src/runtime.rs b/shell/ferese-lock/src/runtime.rs index da8fa1d..8460e57 100644 --- a/shell/ferese-lock/src/runtime.rs +++ b/shell/ferese-lock/src/runtime.rs @@ -1,7 +1,7 @@ //! Startup handshake: the invoking command exits successfully only after the //! compositor's confirmation. The UI child remains alive for authentication. use std::io::{BufRead, BufReader}; -use std::process::{Command, Stdio}; +use std::process::{Child, Command, Stdio}; use std::time::Duration; use wayland_client::globals::{GlobalListContents, registry_queue_init}; @@ -43,6 +43,10 @@ pub fn daemonize() -> Result<(), String> { .stderr(Stdio::inherit()) .spawn() .map_err(|e| format!("Starting locker {}: {e}", executable.display()))?; + confirm(&mut child) +} + +fn confirm(child: &mut Child) -> Result<(), String> { let stdout = child.stdout.take().unwrap(); let (tx, rx) = std::sync::mpsc::sync_channel(1); std::thread::spawn(move || { @@ -53,44 +57,65 @@ pub fn daemonize() -> Result<(), String> { }); match rx.recv_timeout(Duration::from_secs(15)) { Ok(true) => Ok(()), - Ok(false) => { - let _ = child.wait(); - Err("locker exited without confirming protection".into()) - } + Ok(false) => match child.try_wait() { + Ok(Some(_)) => Err("locker exited without confirming protection".into()), + _ => Err("locker did not confirm protection; the locker was left running".into()), + }, Err(_) => Err("lock confirmation timed out; the locker was left running".into()), } } -/// libcosmic revision 03d7dcb emits one synthetic Locked on the request and -/// another from SCTK's actual protocol callback. Requiring both is essential: -/// never release swayidle's delay inhibitor on the synthetic notification. -/// Re-audit iced/winit's session_lock Action::Lock and SessionLockHandler when -/// updating libcosmic. A missing callback times out closed, never signals ready. +/// Readiness follows the compositor's protocol confirmation, never the request. #[derive(Default)] pub struct Confirmation { - events: u8, + confirmed: bool, } impl Confirmation { - pub fn observe(&mut self) -> bool { - self.events = self.events.saturating_add(1); - self.events == 2 + pub fn observe(&mut self, event: &cosmic::iced::event::wayland::SessionLockEvent) -> bool { + if !matches!(event, cosmic::iced::event::wayland::SessionLockEvent::Locked) || self.confirmed { + return false; + } + self.confirmed = true; + true } pub fn confirmed(&self) -> bool { - self.events >= 2 + self.confirmed } } #[cfg(test)] mod tests { use super::*; + #[test] - fn synthetic_event_does_not_confirm_a_lock() { + fn invalid_confirmation_returns_without_waiting_for_or_killing_the_locker() { + for message in ["unexpected", "FERESE_LOCKED"] { + let mut child = Command::new("sh") + .args(["-c", "printf '%s\\n' \"$1\"; exec sleep 3", "lock-fixture", message]) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + let result = confirm(&mut child); + let running = child.try_wait().unwrap().is_none(); + if running { + child.kill().unwrap(); + } + child.wait().unwrap(); + assert_eq!(result.is_ok(), message == "FERESE_LOCKED"); + assert!(running, "confirmation must leave the locker running"); + } + } + + #[test] + fn only_compositor_confirmation_marks_the_lock_ready_once() { + use cosmic::iced::event::wayland::SessionLockEvent::{Finished, Locked, NotSupported, Requested}; let mut c = Confirmation::default(); - assert!(!c.confirmed()); - assert!(!c.observe()); - assert!(!c.confirmed()); - assert!(c.observe()); + for event in [Requested, Requested, Finished, NotSupported] { + assert!(!c.observe(&event)); + assert!(!c.confirmed()); + } + assert!(c.observe(&Locked)); assert!(c.confirmed()); - assert!(!c.observe()); + assert!(!c.observe(&Locked)); } } diff --git a/shell/ferese-settings/Cargo.toml b/shell/ferese-settings/Cargo.toml index 9708cf1..5825e80 100644 --- a/shell/ferese-settings/Cargo.toml +++ b/shell/ferese-settings/Cargo.toml @@ -10,7 +10,7 @@ ferese-theme-client = { path = "../../crates/ferese-theme-client" } ferese-theme = { path = "../../crates/ferese-theme" } ferese-config = { path = "../../crates/ferese-config" } libcosmic = { workspace = true, features = ["wgpu", "a11y", "xdg-portal"] } -iced_accessibility = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.1" } +iced_accessibility.workspace = true ferese-ipc = { path = "../../crates/ferese-ipc" } serde_json.workspace = true tempfile.workspace = true diff --git a/shell/ferese-settings/src/store.rs b/shell/ferese-settings/src/store.rs index a263fc0..c9372ab 100644 --- a/shell/ferese-settings/src/store.rs +++ b/shell/ferese-settings/src/store.rs @@ -1,5 +1,5 @@ use std::fs; -use std::io::{Read, Write}; +use std::io::Read; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; @@ -7,7 +7,7 @@ use std::time::{Duration, Instant}; use ferese_config::Document; use serde_json::Value; -pub const LIMIT: usize = 60 * 1024; +pub const LIMIT: usize = ferese_config::file::LIMIT; #[derive(Clone, Debug)] pub struct Snapshot { @@ -200,34 +200,11 @@ fn save_with( validate: impl FnOnce(&Path) -> Result<(), String>, ) -> Result { let snapshot = Snapshot::parse(desired.to_owned())?; - let unchanged = || -> Result<(), String> { - if Snapshot::read(path)?.source != expected { - return Err("Your config changed in another app. Reload settings before saving.".into()); - } - Ok(()) - }; - unchanged()?; - let parent = path.parent().ok_or("Invalid configuration path.")?; - fs::create_dir_all(parent).map_err(|e| e.to_string())?; - let mut staged = tempfile::NamedTempFile::new_in(parent).map_err(|e| e.to_string())?; - if let Ok(metadata) = fs::metadata(path) { - staged - .as_file() - .set_permissions(metadata.permissions()) - .map_err(|e| e.to_string())?; + let file = ferese_config::file::ConfigFile::open(path)?; + if file.source() != expected { + return Err("Your config changed in another app. Reload settings before saving.".into()); } - staged.write_all(desired.as_bytes()).map_err(|e| e.to_string())?; - staged.as_file().sync_all().map_err(|e| e.to_string())?; - validate(staged.path())?; - unchanged()?; - // Keep the last working source, including comments and unknown keys. - let mut backup = tempfile::NamedTempFile::new_in(parent).map_err(|e| e.to_string())?; - backup.write_all(expected.as_bytes()).map_err(|e| e.to_string())?; - backup.as_file().sync_all().map_err(|e| e.to_string())?; - backup - .persist(path.with_extension("kdl.settings-backup")) - .map_err(|e| e.to_string())?; - staged.persist(path).map_err(|e| e.to_string())?; + file.replace(desired, Some(&path.with_extension("kdl.settings-backup")), validate)?; Ok(snapshot) } @@ -293,6 +270,19 @@ window-rule app-id="mine" floating=#true assert_eq!(fs::read_to_string(&path).unwrap(), "// external edit\n"); } + #[test] + fn validation_holds_the_shared_writer_lock() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("config.kdl"); + save_with(&path, "", "// settings change", |_| { + let error = ferese_config::file::ConfigFile::open(&path).err().unwrap(); + assert!(error.contains("another app")); + Ok(()) + }) + .unwrap(); + assert_eq!(fs::read_to_string(path).unwrap(), "// settings change"); + } + #[test] fn catches_edits_during_validation_and_keeps_a_backup_on_success() { let dir = tempfile::tempdir().unwrap(); diff --git a/shell/ferese-settings/src/watch.rs b/shell/ferese-settings/src/watch.rs index a745cab..9d97a23 100644 --- a/shell/ferese-settings/src/watch.rs +++ b/shell/ferese-settings/src/watch.rs @@ -12,6 +12,13 @@ pub fn changes(path: &Path) -> impl Stream + use<> { let path = path.to_path_buf(); cosmic::iced::stream::channel(1, async move |mut output| { + let path = match std::path::absolute(path) { + Ok(path) => path, + Err(error) => { + let _ = output.send(Message::ExternalConfig(Err(error.to_string()))).await; + return; + } + }; let (send, mut events) = tokio::sync::mpsc::channel(1); let target = path.clone(); let watcher = notify::recommended_watcher(move |event: notify::Result| { @@ -30,10 +37,16 @@ pub fn changes(path: &Path) -> impl Stream + use<> { } }; - if let Err(error) = watcher.watch( - path.parent().unwrap_or(std::path::Path::new(".")), - RecursiveMode::NonRecursive, - ) { + let directory = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + // On a fresh install the directory may not exist until the first save. + // Install its watch now so later saves do not leave this subscription dead. + let watch = std::fs::create_dir_all(directory) + .map_err(notify::Error::io) + .and_then(|()| watcher.watch(directory, RecursiveMode::NonRecursive)); + if let Err(error) = watch { let _ = output.send(Message::ExternalConfig(Err(error.to_string()))).await; return; } @@ -50,10 +63,13 @@ pub fn changes(path: &Path) -> impl Stream + use<> { if events.recv().await.is_none() { return; } - while tokio::time::timeout(Duration::from_millis(150), events.recv()) - .await - .is_ok() - {} + loop { + match tokio::time::timeout(Duration::from_millis(150), events.recv()).await { + Ok(Some(())) => (), + Ok(None) => return, + Err(_) => break, + } + } } }) } @@ -64,6 +80,32 @@ mod tests { use super::*; + #[test] + fn watches_first_save_when_the_config_directory_does_not_exist() { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap() + .block_on(async { + let directory = tempfile::tempdir_in(".").unwrap(); + let path = directory.path().join("ferese/config.kdl"); + let stream = changes(&path); + pin_mut!(stream); + assert!(matches!(stream.next().await, Some(Message::ExternalConfig(Ok(_))))); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let source = "animations { speed 0.75; }"; + std::fs::write(&path, source).unwrap(); + match tokio::time::timeout(Duration::from_secs(3), stream.next()) + .await + .unwrap() + .unwrap() + { + Message::ExternalConfig(Ok(snapshot)) => assert_eq!(snapshot.source, source), + _ => panic!("first save was not observed"), + } + }); + } + #[test] fn watches_atomic_replacement_and_retains_subscription_after_bad_kdl() { tokio::runtime::Builder::new_current_thread() diff --git a/shell/ferese-shell/Cargo.toml b/shell/ferese-shell/Cargo.toml index 9df6d6a..aafacea 100644 --- a/shell/ferese-shell/Cargo.toml +++ b/shell/ferese-shell/Cargo.toml @@ -18,7 +18,7 @@ libc.workspace = true # tiny-skia to avoid a second GPU initialization during desktop startup. libcosmic = { workspace = true, features = ["wgpu", "a11y"] } freedesktop-desktop-entry = { version = "0.8.3", default-features = false } -iced_accessibility = { git = "https://github.com/ferese-wm/libcosmic", tag = "ferese-v0.1.3-rc.1" } +iced_accessibility.workspace = true serde.workspace = true serde_json.workspace = true tempfile.workspace = true