From dcbcec6ba4420856ccb499162669472e2f00c277 Mon Sep 17 00:00:00 2001 From: Samuel Onoja Date: Wed, 7 Oct 2026 09:30:25 +0100 Subject: [PATCH] Preserve display assignments and fix CI checks Keep surviving outputs on their applied CRTCs when enabling another display. Reserve nested IPC directories with explicit private permissions and cover session umasks. Separate compositor checks from release-only workspace checks, provide test runtime prerequisites, and correct the reload test's initial revision assumption. --- .github/workflows/compositor-checks.yml | 69 +++++++++ .github/workflows/compositor-lifecycle.yml | 101 ------------- .github/workflows/release-checks.yml | 50 +++++++ compositor/src/backends/direct.rs | 84 ++++++----- .../src/backends/direct/crtc_assignment.rs | 137 ++++++++++++++++++ compositor/src/ipc.rs | 49 +++++++ compositor/src/reload.rs | 4 +- 7 files changed, 355 insertions(+), 139 deletions(-) create mode 100644 .github/workflows/compositor-checks.yml delete mode 100644 .github/workflows/compositor-lifecycle.yml create mode 100644 .github/workflows/release-checks.yml create mode 100644 compositor/src/backends/direct/crtc_assignment.rs diff --git a/.github/workflows/compositor-checks.yml b/.github/workflows/compositor-checks.yml new file mode 100644 index 0000000..75246fd --- /dev/null +++ b/.github/workflows/compositor-checks.yml @@ -0,0 +1,69 @@ +name: Compositor lifecycle and capture tests +on: + pull_request: + paths: + - "compositor/**" + - "crates/**" + - "shell/**" + - "portal/**" + - "feresectl/**" + - "scripts/**" + - "packaging/**" + - "assets/**" + - "LICENSE" + - "rust-toolchain.toml" + - "rustfmt.toml" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/compositor-checks.yml" + push: + branches: [main] + paths: + - "compositor/**" + - "crates/**" + - "shell/**" + - "portal/**" + - "feresectl/**" + - "scripts/**" + - "packaging/**" + - "assets/**" + - "LICENSE" + - "rust-toolchain.toml" + - "rustfmt.toml" + - "Cargo.toml" + - "Cargo.lock" + - ".github/workflows/compositor-checks.yml" + workflow_dispatch: +permissions: + contents: read +concurrency: + group: compositor-checks-${{ github.ref }} + cancel-in-progress: true +jobs: + protocol: + runs-on: ubuntu-24.04 + timeout-minutes: 30 + env: + XDG_RUNTIME_DIR: /tmp/ferese-compositor-runtime + steps: + - uses: actions/checkout@v6 + - name: Install compositor build dependencies + run: | + sudo apt-get update + sudo apt-get install -y build-essential pkg-config dbus-daemon libwayland-dev \ + libxkbcommon-dev xkb-data libinput-dev libudev-dev libseat-dev libgbm-dev \ + libegl-dev libegl-mesa0 libgl1-mesa-dri libdrm-dev libfontconfig1-dev libfreetype-dev libexpat1-dev xauth + - name: Create private test runtime directory + run: install -d -m 0700 "$XDG_RUNTIME_DIR" + - name: Install Rust + run: rustup toolchain install 1.95.0 --profile minimal + - name: Test compositor policies, protocols and worker lifecycles + run: cargo +1.95.0 test --locked -p ferese -- --nocapture + - name: Test capture privacy, resize dependencies and cursor composition + env: + LIBGL_ALWAYS_SOFTWARE: "1" + EGL_PLATFORM: surfaceless + run: | + cargo +1.95.0 test --locked -p ferese state::capture_privacy::tests -- --ignored --nocapture + cargo +1.95.0 test --locked -p ferese backends::direct::capture::tests::capture_recomposes_cursor_without_touching_the_display_target -- --ignored --nocapture + cargo +1.95.0 test --locked -p ferese winit::tests::capture_failure_restores_display_target_and_subsequent_frames_render -- --ignored --nocapture diff --git a/.github/workflows/compositor-lifecycle.yml b/.github/workflows/compositor-lifecycle.yml deleted file mode 100644 index 7d57da6..0000000 --- a/.github/workflows/compositor-lifecycle.yml +++ /dev/null @@ -1,101 +0,0 @@ -name: Workspace release checks -on: - pull_request: - paths: - - "compositor/**" - - "crates/**" - - "shell/**" - - "portal/**" - - "feresectl/**" - - "scripts/**" - - "packaging/**" - - "assets/**" - - "LICENSE" - - "rust-toolchain.toml" - - "rustfmt.toml" - - "Cargo.toml" - - "Cargo.lock" - - ".github/workflows/compositor-lifecycle.yml" - push: - branches: [main] - paths: - - "compositor/**" - - "crates/**" - - "shell/**" - - "portal/**" - - "feresectl/**" - - "scripts/**" - - "packaging/**" - - "assets/**" - - "LICENSE" - - "rust-toolchain.toml" - - "rustfmt.toml" - - "Cargo.toml" - - "Cargo.lock" - - ".github/workflows/compositor-lifecycle.yml" - workflow_dispatch: -permissions: - contents: read -concurrency: - group: compositor-lifecycle-${{ github.ref }} - cancel-in-progress: true -jobs: - protocol: - runs-on: ubuntu-24.04 - timeout-minutes: 30 - steps: - - uses: actions/checkout@v6 - - name: Install compositor build dependencies - run: | - sudo apt-get update - sudo apt-get install -y build-essential pkg-config dbus-daemon libwayland-dev \ - libxkbcommon-dev xkb-data libinput-dev libudev-dev libseat-dev libgbm-dev \ - libegl-dev libegl-mesa0 libgl1-mesa-dri libdrm-dev libfontconfig1-dev libfreetype-dev libexpat1-dev - - name: Install Rust - run: rustup toolchain install 1.95.0 --profile minimal - - name: Test compositor policies, protocols and worker lifecycles - run: cargo +1.95.0 test --locked -p ferese -- --nocapture - - name: Test capture privacy, resize dependencies and cursor composition - env: - LIBGL_ALWAYS_SOFTWARE: "1" - EGL_PLATFORM: surfaceless - run: | - cargo +1.95.0 test --locked -p ferese state::capture_privacy::tests -- --ignored --nocapture - cargo +1.95.0 test --locked -p ferese backends::direct::capture::tests::capture_recomposes_cursor_without_touching_the_display_target -- --ignored --nocapture - cargo +1.95.0 test --locked -p ferese winit::tests::capture_failure_restores_display_target_and_subsequent_frames_render -- --ignored --nocapture - - workspace: - runs-on: ubuntu-24.04 - container: fedora:44 - timeout-minutes: 60 - env: - CARGO_BUILD_JOBS: "2" - CARGO_INCREMENTAL: "0" - CARGO_PROFILE_DEV_DEBUG: "0" - CARGO_PROFILE_TEST_DEBUG: "0" - steps: - - name: Install desktop build and test dependencies - run: | - dnf install -y --allowerasing python3 python3-gobject python3-pillow git curl gcc gcc-c++ make cmake pkgconf-pkg-config \ - wayland-devel wayland-protocols-devel libxkbcommon-devel libinput-devel systemd-devel libseat-devel \ - mesa-libgbm-devel mesa-libEGL-devel mesa-dri-drivers libdrm-devel fontconfig-devel \ - freetype-devel expat-devel dbus-daemon dbus-tools desktop-file-utils foot pam \ - clang clang-devel pipewire-devel pipewire wireplumber polkit polkit-libs libei \ - xdg-desktop-portal xdg-desktop-portal-gtk \ - gstreamer1-devel gstreamer1-plugins-base gstreamer1-plugins-good pipewire-gstreamer - - uses: actions/checkout@v6 - - name: Install pinned Rust and checks - run: | - curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs -o /tmp/rustup.sh - sh /tmp/rustup.sh -y --profile minimal --default-toolchain 1.95.0 --component rustfmt --component clippy - echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" - - name: Check formatting and all workspace lints - run: | - cargo fmt --all --check - cargo clippy --locked --workspace --all-targets -- -D warnings - - name: Build desktop binaries and test the complete workspace - run: | - cargo build --locked --workspace - cargo test --locked --workspace --no-fail-fast - - name: Test installer, session recovery and private portal contracts - run: python3 -m unittest discover -s scripts/tests -v diff --git a/.github/workflows/release-checks.yml b/.github/workflows/release-checks.yml new file mode 100644 index 0000000..65cd1b8 --- /dev/null +++ b/.github/workflows/release-checks.yml @@ -0,0 +1,50 @@ +name: Workspace release checks +on: + release: + types: [published] +permissions: + contents: read +concurrency: + group: release-checks-${{ github.ref }} + cancel-in-progress: false +jobs: + workspace: + runs-on: ubuntu-24.04 + container: + image: fedora:44 + options: --init + timeout-minutes: 60 + env: + CARGO_BUILD_JOBS: "2" + CARGO_INCREMENTAL: "0" + CARGO_PROFILE_DEV_DEBUG: "0" + CARGO_PROFILE_TEST_DEBUG: "0" + XDG_RUNTIME_DIR: /tmp/ferese-release-runtime + steps: + - name: Install desktop build and test dependencies + run: | + dnf install -y --allowerasing python3 python3-gobject python3-pillow git curl gcc gcc-c++ make cmake pkgconf-pkg-config \ + wayland-devel wayland-protocols-devel libxkbcommon-devel libinput-devel systemd-devel libseat-devel \ + mesa-libgbm-devel mesa-libEGL-devel mesa-dri-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 \ + gstreamer1-devel gstreamer1-plugins-base gstreamer1-plugins-good pipewire-gstreamer + - name: Create private test runtime directory + run: install -d -m 0700 "$XDG_RUNTIME_DIR" + - uses: actions/checkout@v6 + - name: Install pinned Rust and checks + run: | + curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs -o /tmp/rustup.sh + sh /tmp/rustup.sh -y --profile minimal --default-toolchain 1.95.0 --component rustfmt --component clippy + echo "$HOME/.cargo/bin" >> "$GITHUB_PATH" + - name: Check formatting and all workspace lints + run: | + cargo fmt --all --check + cargo clippy --locked --workspace --all-targets -- -D warnings + - name: Build desktop binaries and test the complete workspace + run: | + cargo build --locked --workspace + cargo test --locked --workspace --no-fail-fast + - name: Test installer, session recovery and private portal contracts + run: python3 -m unittest discover -s scripts/tests -v diff --git a/compositor/src/backends/direct.rs b/compositor/src/backends/direct.rs index 10c8368..2d1f80d 100644 --- a/compositor/src/backends/direct.rs +++ b/compositor/src/backends/direct.rs @@ -6,6 +6,7 @@ use std::path::Path; use std::time::{Duration, Instant}; pub(crate) mod capture; +mod crtc_assignment; mod lid; mod mirror; mod output_power; @@ -770,8 +771,7 @@ pub(crate) fn validate_live_outputs(state: &Ferese, profiles: &[OutputProfile]) resolve_output_positions(backend, &mut desired)?; let mut usable = 0; for device in backend.devices.values() { - let scan = - select_outputs(&device.drm, &backend.monitors, &desired, backend.low_power).map_err(|e| e.to_string())?; + let scan = select_outputs(device, &backend.monitors, &desired, backend.low_power).map_err(|e| e.to_string())?; transaction::validate_device(device, &scan).map_err(|error| error.to_string())?; usable += scan.selections.len(); } @@ -1471,12 +1471,7 @@ fn rescan_device(state: &mut Ferese, node: DrmNode) -> Result { let Some(mut device) = backend.devices.remove(&node) else { return Err(()); }; - let scan = select_outputs( - &device.drm, - &backend.monitors, - &backend.desired_outputs, - backend.low_power, - ); + let scan = select_outputs(&device, &backend.monitors, &backend.desired_outputs, backend.low_power); let result = scan .map_err(|error| error.to_string()) .and_then(|scan| transaction::apply_device(state, node, &mut device, scan)); @@ -1492,12 +1487,7 @@ fn rescan_device(state: &mut Ferese, node: DrmNode) -> Result { fn refresh_connected_info(state: &mut Ferese) { let backend = state.direct_backend.as_mut().unwrap(); for device in backend.devices.values_mut() { - if let Ok(scan) = select_outputs( - &device.drm, - &backend.monitors, - &backend.desired_outputs, - backend.low_power, - ) { + if let Ok(scan) = select_outputs(device, &backend.monitors, &backend.desired_outputs, backend.low_power) { device.connected_outputs = scan.connected_outputs; } transaction::update_applied_info(device); @@ -1567,13 +1557,8 @@ fn remember_applied_configuration(state: &mut Ferese) { fn validate_desired_outputs(state: &Ferese) -> Result<(), String> { let backend = state.direct_backend.as_ref().ok_or("direct backend unavailable")?; for device in backend.devices.values() { - let scan = select_outputs( - &device.drm, - &backend.monitors, - &backend.desired_outputs, - backend.low_power, - ) - .map_err(|error| error.to_string())?; + let scan = select_outputs(device, &backend.monitors, &backend.desired_outputs, backend.low_power) + .map_err(|error| error.to_string())?; transaction::validate_device(device, &scan).map_err(|error| error.to_string())?; } Ok(()) @@ -1878,11 +1863,12 @@ fn send_frame_callbacks(state: &mut Ferese, output: &Output) { } fn select_outputs( - drm: &DrmDevice, + device: &DirectDevice, monitors: &[Monitor], desired: &DesiredOutputConfiguration, low_power: bool, ) -> io::Result { + let drm = &device.drm; let resources = drm.resource_handles()?; let connected = resources .connectors() @@ -1895,7 +1881,8 @@ fn select_outputs( let device_key = drm_device_key(drm); let mut selections = Vec::new(); let mut connected_outputs = Vec::new(); - let mut used_crtcs = HashSet::new(); + let mut candidates = Vec::new(); + let mut requests = Vec::new(); for connector in connected { let key = format!("{device_key}/{}", connector); @@ -1982,27 +1969,50 @@ fn select_outputs( continue; }; - let current_crtc = connector + let applied = device + .outputs + .iter() + .find(|(_, output)| output.connector == connector.handle()) + .map(|(crtc, _)| *crtc); + let kernel = connector .current_encoder() .and_then(|handle| drm.get_encoder(handle).ok()) - .and_then(|encoder| encoder.crtc()) - .filter(|crtc| !used_crtcs.contains(crtc)); - let compatible_crtc = connector.encoders().iter().find_map(|handle| { - let encoder = drm.get_encoder(*handle).ok()?; - resources - .filter_crtcs(encoder.possible_crtcs()) - .into_iter() - .find(|crtc| !used_crtcs.contains(crtc)) + .and_then(|encoder| encoder.crtc()); + let mut compatible = Vec::new(); + for handle in connector.encoders() { + if let Ok(encoder) = drm.get_encoder(*handle) { + for crtc in resources.filter_crtcs(encoder.possible_crtcs()) { + if !compatible.contains(&crtc) { + compatible.push(crtc); + } + } + } + } + requests.push(crtc_assignment::Candidates { + applied, + kernel, + compatible, }); - if let Some(crtc) = current_crtc.or(compatible_crtc) { - used_crtcs.insert(crtc); + candidates.push(( + connector, + mode, + settings, + identity, + desired_output.mirror_source.clone(), + )); + } + + for ((connector, mode, settings, identity, mirror_source), crtc) in + candidates.into_iter().zip(crtc_assignment::assign(&requests)) + { + if let Some(crtc) = crtc { selections.push(OutputSelection { connector, crtc, mode, settings, identity, - mirror_source: desired_output.mirror_source.clone(), + mirror_source, }); } } @@ -2245,8 +2255,8 @@ fn resolve_output_positions( ) -> Result<(), String> { let mut placements = Vec::new(); for device in backend.devices.values() { - let scan = select_outputs(&device.drm, &backend.monitors, desired, backend.low_power) - .map_err(|error| error.to_string())?; + let scan = + select_outputs(device, &backend.monitors, desired, backend.low_power).map_err(|error| error.to_string())?; for selection in scan.selections { let size = output_transform(selection.settings.transform).transform_size(OutputMode::from(selection.mode).size); diff --git a/compositor/src/backends/direct/crtc_assignment.rs b/compositor/src/backends/direct/crtc_assignment.rs new file mode 100644 index 0000000..7bbfb39 --- /dev/null +++ b/compositor/src/backends/direct/crtc_assignment.rs @@ -0,0 +1,137 @@ +use std::collections::HashSet; + +use smithay::reexports::drm::control::crtc; + +pub(super) struct Candidates { + pub applied: Option, + pub kernel: Option, + pub compatible: Vec, +} + +/// Reserve surviving outputs before assigning newly enabled connectors. Kernel +/// encoder routing can be stale for a disabled connector; applied ownership wins. +pub(super) fn assign(requests: &[Candidates]) -> Vec> { + let mut assigned = vec![None; requests.len()]; + let mut used = HashSet::new(); + + for applied in [true, false] { + for (request, assigned) in requests.iter().zip(&mut assigned) { + let preferred = if applied { request.applied } else { request.kernel }; + + if assigned.is_none() + && let Some(crtc) = preferred.filter(|crtc| request.compatible.contains(crtc)) + && used.insert(crtc) + { + *assigned = Some(crtc); + } + } + } + + // Give connectors with fewer choices the first free CRTC. + let mut remaining = (0..requests.len()) + .filter(|index| assigned[*index].is_none()) + .collect::>(); + + remaining.sort_by_key(|index| requests[*index].compatible.len()); + + for index in remaining { + if let Some(crtc) = requests[index].compatible.iter().find(|crtc| !used.contains(crtc)) { + used.insert(*crtc); + assigned[index] = Some(*crtc); + } + } + + assigned +} + +#[cfg(test)] +mod tests { + use super::*; + + fn crtc(value: u32) -> crtc::Handle { + std::num::NonZeroU32::new(value).unwrap().into() + } + + fn candidates(applied: Option, kernel: Option, compatible: &[u32]) -> Candidates { + Candidates { + applied: applied.map(crtc), + kernel: kernel.map(crtc), + compatible: compatible.iter().copied().map(crtc).collect(), + } + } + + #[test] + fn external_to_two_outputs_keeps_external_crtc_regardless_of_connector_order() { + // Extend and mirror both require two independent scanout assignments. + // The internal panel appears first in the DRM connector inventory. + let internal = candidates(None, None, &[151, 270]); + let external = candidates(Some(151), Some(151), &[151, 270]); + assert_eq!(assign(&[internal, external]), vec![Some(crtc(270)), Some(crtc(151))]); + let internal = candidates(None, None, &[151, 270]); + let external = candidates(Some(151), Some(151), &[151, 270]); + assert_eq!(assign(&[external, internal]), vec![Some(crtc(151)), Some(crtc(270))]); + } + + #[test] + fn stale_disabled_encoder_cannot_steal_applied_external_assignment() { + assert_eq!( + assign(&[ + candidates(None, Some(151), &[151, 270]), + candidates(Some(151), Some(151), &[151, 270]), + ]), + vec![Some(crtc(270)), Some(crtc(151))] + ); + } + + #[test] + fn kernel_assignments_are_preserved_when_no_outputs_have_been_applied() { + assert_eq!( + assign(&[ + candidates(None, None, &[151, 270]), + candidates(None, Some(151), &[151, 270]), + ]), + vec![Some(crtc(270)), Some(crtc(151))] + ); + } + + #[test] + fn returning_to_one_output_and_reconnecting_preserves_surviving_assignment() { + assert_eq!( + assign(&[candidates(Some(270), Some(270), &[151, 270])]), + vec![Some(crtc(270))] + ); + assert_eq!( + assign(&[ + candidates(None, None, &[151, 270]), + candidates(Some(270), Some(270), &[151, 270]), + ]), + vec![Some(crtc(151)), Some(crtc(270))] + ); + } + + #[test] + fn incompatible_previous_assignment_is_not_reserved() { + assert_eq!( + assign(&[candidates(Some(151), Some(151), &[270])]), + vec![Some(crtc(270))] + ); + } + + #[test] + fn restricted_connector_gets_its_only_free_crtc() { + assert_eq!( + assign(&[candidates(None, None, &[151, 270]), candidates(None, None, &[151])]), + vec![Some(crtc(270)), Some(crtc(151))] + ); + } + + #[test] + fn exhausted_crtcs_do_not_displace_surviving_output_or_assign_twice() { + assert_eq!( + assign(&[candidates(None, None, &[151]), candidates(Some(151), Some(151), &[151])]), + vec![None, Some(crtc(151))] + ); + assert!(assign(&[]).is_empty()); + assert_eq!(assign(&[candidates(None, None, &[])]), vec![None]); + } +} diff --git a/compositor/src/ipc.rs b/compositor/src/ipc.rs index 03f3c0c..53cc297 100644 --- a/compositor/src/ipc.rs +++ b/compositor/src/ipc.rs @@ -109,6 +109,7 @@ impl Endpoint { prepare_directory(&instances)?; let instance = tempfile::Builder::new() .prefix(&format!("{}-", std::process::id())) + .permissions(fs::Permissions::from_mode(0o700)) .tempdir_in(instances)?; Ok(Self { path: instance.path().join("control.sock"), @@ -1938,6 +1939,32 @@ mod tests { drop(host); } + #[test] + fn nested_endpoint_permissions_do_not_depend_on_session_umask() { + for mask in [0o000, 0o022, 0o077] { + // Set the mask after exec so the parent and its parallel tests + // retain their umask and socket ownership. + let output = std::process::Command::new("sh") + .arg("-c") + .arg(format!("umask {mask:03o}\nexec \"$@\"")) + .arg("ferese-ipc-umask-test") + .arg(std::env::current_exe().unwrap()) + .args([ + "--exact", + "ipc::tests::nested_endpoints_reserve_private_unique_directories_and_preserve_the_default", + "--nocapture", + ]) + .output() + .unwrap(); + assert!( + output.status.success(), + "umask {mask:03o}:\n{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + } + } + #[test] fn nested_endpoint_rejects_an_instances_symlink() { let runtime = tempfile::tempdir().unwrap(); @@ -1953,6 +1980,28 @@ mod tests { #[test] fn listener_reclaims_only_a_stale_owned_socket() { + // A concurrent test can fork while this listener is open, retaining + // its descriptor until exec. Keep the close/rebind check in a process + // that does not spawn other children during the assertion. + const CHILD: &str = "FERESE_STALE_SOCKET_TEST_CHILD"; + if std::env::var_os(CHILD).is_none() { + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "ipc::tests::listener_reclaims_only_a_stale_owned_socket", + "--nocapture", + ]) + .env(CHILD, "1") + .output() + .unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + return; + } let root = unique_test_directory("stale-socket"); fs::create_dir(&root).unwrap(); let socket = root.join("control.sock"); diff --git a/compositor/src/reload.rs b/compositor/src/reload.rs index 3cdd263..7bc50b4 100644 --- a/compositor/src/reload.rs +++ b/compositor/src/reload.rs @@ -435,6 +435,7 @@ mod tests { } else { std::fs::remove_file(&path).unwrap(); } + let already_applied = state.config_source.as_deref() == Some(valid); let before = state.theme_engine.revision; let (reply, received) = std::sync::mpsc::sync_channel(1); state.queue_config_reload(index == 1, Some((42, reply))).unwrap(); @@ -453,7 +454,8 @@ mod tests { Duration::from_millis(200) ); assert_eq!(state.config_source.as_deref(), Some(valid)); - if source == Some(valid) && before > 0 { + // An initial revision can describe defaults rather than this source. + if source == Some(valid) && already_applied { assert_eq!(state.theme_engine.revision, before); } }