diff --git a/archinstall/default_profiles/desktops/hyprland.py b/archinstall/default_profiles/desktops/hyprland.py index 0bf46b96db..c7bc9c325a 100644 --- a/archinstall/default_profiles/desktops/hyprland.py +++ b/archinstall/default_profiles/desktops/hyprland.py @@ -1,6 +1,6 @@ from typing import override -from archinstall.default_profiles.desktops.utils import select_seat_access +from archinstall.default_profiles.desktops.utils import seat_access_of, select_seat_access from archinstall.default_profiles.profile import CustomSetting, DisplayServerType, GreeterType, Profile, ProfileType @@ -18,7 +18,7 @@ def __init__(self) -> None: @property @override def packages(self) -> list[str]: - return [ + packages = [ 'hyprland', 'dunst', 'kitty', @@ -33,6 +33,11 @@ def packages(self) -> list[str]: 'slurp', ] + if seat := seat_access_of(self): + packages += seat.packages + + return packages + @property @override def default_greeter_type(self) -> GreeterType: @@ -41,8 +46,8 @@ def default_greeter_type(self) -> GreeterType: @property @override def services(self) -> list[str]: - if pref := self.custom_settings.get(CustomSetting.SeatAccess, None): - return [pref] + if seat := seat_access_of(self): + return seat.services return [] @override diff --git a/archinstall/default_profiles/desktops/labwc.py b/archinstall/default_profiles/desktops/labwc.py index 48fe344e13..72795eac6b 100644 --- a/archinstall/default_profiles/desktops/labwc.py +++ b/archinstall/default_profiles/desktops/labwc.py @@ -1,6 +1,6 @@ from typing import override -from archinstall.default_profiles.desktops.utils import select_seat_access +from archinstall.default_profiles.desktops.utils import seat_access_of, select_seat_access from archinstall.default_profiles.profile import CustomSetting, DisplayServerType, GreeterType, Profile, ProfileType @@ -18,14 +18,15 @@ def __init__(self) -> None: @property @override def packages(self) -> list[str]: - additional = [] - if seat := self.custom_settings.get(CustomSetting.SeatAccess, None): - additional = [seat] - - return [ + packages = [ 'alacritty', 'labwc', - ] + additional + ] + + if seat := seat_access_of(self): + packages += seat.packages + + return packages @property @override @@ -35,8 +36,8 @@ def default_greeter_type(self) -> GreeterType: @property @override def services(self) -> list[str]: - if pref := self.custom_settings.get(CustomSetting.SeatAccess, None): - return [pref] + if seat := seat_access_of(self): + return seat.services return [] @override diff --git a/archinstall/default_profiles/desktops/niri.py b/archinstall/default_profiles/desktops/niri.py index d8db75f5da..db9f9b6d08 100644 --- a/archinstall/default_profiles/desktops/niri.py +++ b/archinstall/default_profiles/desktops/niri.py @@ -1,6 +1,6 @@ from typing import override -from archinstall.default_profiles.desktops.utils import select_seat_access +from archinstall.default_profiles.desktops.utils import seat_access_of, select_seat_access from archinstall.default_profiles.profile import CustomSetting, DisplayServerType, GreeterType, Profile, ProfileType @@ -18,11 +18,7 @@ def __init__(self) -> None: @property @override def packages(self) -> list[str]: - additional = [] - if seat := self.custom_settings.get(CustomSetting.SeatAccess, None): - additional = [seat] - - return [ + packages = [ 'niri', 'alacritty', 'fuzzel', @@ -33,7 +29,12 @@ def packages(self) -> list[str]: 'swayidle', 'swaylock', 'xdg-desktop-portal-gnome', - ] + additional + ] + + if seat := seat_access_of(self): + packages += seat.packages + + return packages @property @override @@ -43,8 +44,8 @@ def default_greeter_type(self) -> GreeterType: @property @override def services(self) -> list[str]: - if pref := self.custom_settings.get(CustomSetting.SeatAccess, None): - return [pref] + if seat := seat_access_of(self): + return seat.services return [] @override diff --git a/archinstall/default_profiles/desktops/sway.py b/archinstall/default_profiles/desktops/sway.py index 7a038dc0ba..9156f845ad 100644 --- a/archinstall/default_profiles/desktops/sway.py +++ b/archinstall/default_profiles/desktops/sway.py @@ -1,6 +1,6 @@ from typing import override -from archinstall.default_profiles.desktops.utils import select_seat_access +from archinstall.default_profiles.desktops.utils import seat_access_of, select_seat_access from archinstall.default_profiles.profile import CustomSetting, DisplayServerType, GreeterType, Profile, ProfileType @@ -18,11 +18,7 @@ def __init__(self) -> None: @property @override def packages(self) -> list[str]: - additional = [] - if seat := self.custom_settings.get(CustomSetting.SeatAccess, None): - additional = [seat] - - return [ + packages = [ 'sway', 'swaybg', 'swaylock', @@ -35,7 +31,12 @@ def packages(self) -> list[str]: 'pavucontrol', 'foot', 'xorg-xwayland', - ] + additional + ] + + if seat := seat_access_of(self): + packages += seat.packages + + return packages @property @override @@ -45,8 +46,8 @@ def default_greeter_type(self) -> GreeterType: @property @override def services(self) -> list[str]: - if pref := self.custom_settings.get(CustomSetting.SeatAccess, None): - return [pref] + if seat := seat_access_of(self): + return seat.services return [] @override diff --git a/archinstall/default_profiles/desktops/utils.py b/archinstall/default_profiles/desktops/utils.py index b179ad5061..f186a173ab 100644 --- a/archinstall/default_profiles/desktops/utils.py +++ b/archinstall/default_profiles/desktops/utils.py @@ -1,16 +1,69 @@ from enum import Enum +from archinstall.default_profiles.profile import CustomSetting, Profile from archinstall.lib.installer import Installer +from archinstall.lib.log import warn from archinstall.lib.menu.helpers import Selection from archinstall.lib.models.users import User from archinstall.lib.translationhandler import tr from archinstall.tui.menu_item import MenuItem, MenuItemGroup from archinstall.tui.result import ResultType +# Before this option was named after the seat manager it was labelled 'polkit', +# and saved configurations still contain that word. +_LEGACY_LOGIND_SETTING = 'polkit' + class SeatAccess(Enum): - seatd = 'seatd' - polkit = 'polkit' + """How a compositor is given access to the seat: the keyboard, mouse and + screen it is meant to drive. Arch offers two ways of doing that, and each + one needs different things installed and started.""" + + Seatd = 'seatd' + Logind = 'systemd-logind' + + @classmethod + def from_setting(cls, value: str | None) -> SeatAccess | None: + """Read back what was stored in a profile's custom settings. Returns + None when nothing was chosen, or when the value is not one we know.""" + if value is None: + return None + + if value == _LEGACY_LOGIND_SETTING: + return cls.Logind + + try: + return cls(value) + except ValueError: + warn(f'Unknown seat access setting, ignoring it: {value}') + return None + + @property + def packages(self) -> list[str]: + match self: + case SeatAccess.Seatd: + return ['seatd'] + case SeatAccess.Logind: + # logind is part of systemd, so there is nothing to install for + # it. Arch ships polkit as an optional dependency of systemd, and + # logind checks with polkit before letting an unprivileged user + # act, so it has to be installed for this choice to work. + return ['polkit'] + + @property + def services(self) -> list[str]: + match self: + case SeatAccess.Seatd: + return ['seatd'] + case SeatAccess.Logind: + # logind is started on demand and cannot be enabled: its unit + # has no [Install] section. + return [] + + +def seat_access_of(profile: Profile) -> SeatAccess | None: + """The seat access a profile was configured with, if any.""" + return SeatAccess.from_setting(profile.custom_settings.get(CustomSetting.SeatAccess)) def provision_seat_access( @@ -18,7 +71,9 @@ def provision_seat_access( users: list[User], seat_access: str, ) -> None: - if seat_access == SeatAccess.seatd.value: + # seatd decides who may talk to the hardware by group membership, so the + # people logging in have to be in it. logind needs nothing of the sort. + if SeatAccess.from_setting(seat_access) is SeatAccess.Seatd: for user in users: install_session.arch_chroot(f'usermod -a -G seat {user.username}') @@ -31,7 +86,10 @@ async def select_seat_access(profile_name: str, default: str | None) -> SeatAcce items = [MenuItem(s.value, value=s) for s in SeatAccess] group = MenuItemGroup(items, sort_items=True) - group.set_default_by_value(default) + # The menu items hold SeatAccess members while the saved setting is a plain + # string, so it has to be turned back into a member or nothing matches and + # the previous choice is not pre-selected. + group.set_default_by_value(SeatAccess.from_setting(default)) result = await Selection[SeatAccess]( group, diff --git a/tests/test_seat_access.py b/tests/test_seat_access.py new file mode 100644 index 0000000000..dacb6bdeb1 --- /dev/null +++ b/tests/test_seat_access.py @@ -0,0 +1,107 @@ +from collections.abc import Callable +from typing import Any + +import pytest + +from archinstall.default_profiles.desktops.hyprland import HyprlandProfile +from archinstall.default_profiles.desktops.labwc import LabwcProfile +from archinstall.default_profiles.desktops.niri import NiriProfile +from archinstall.default_profiles.desktops.sway import SwayProfile +from archinstall.default_profiles.desktops.utils import SeatAccess, provision_seat_access +from archinstall.default_profiles.profile import CustomSetting, Profile +from archinstall.lib.models.users import Password, User + +# Every profile that asks the user how the compositor should reach the hardware. +SEAT_PROFILES: list[Callable[[], Profile]] = [SwayProfile, HyprlandProfile, NiriProfile, LabwcProfile] + + +class FakeInstaller: + """Records the commands provision_seat_access would run in the target.""" + + def __init__(self) -> None: + self.commands: list[str] = [] + + def arch_chroot(self, cmd: str, *args: Any, **kwargs: Any) -> None: + self.commands.append(cmd) + + +def _profile_with(profile_type: Callable[[], Profile], setting: str | None) -> Profile: + profile = profile_type() + profile.custom_settings[CustomSetting.SeatAccess] = setting + return profile + + +def test_saved_settings_are_read_back() -> None: + assert SeatAccess.from_setting('seatd') is SeatAccess.Seatd + assert SeatAccess.from_setting('systemd-logind') is SeatAccess.Logind + + +def test_the_old_polkit_setting_still_means_logind() -> None: + # Configurations written before this option was renamed say 'polkit', and + # they have to keep working. + assert SeatAccess.from_setting('polkit') is SeatAccess.Logind + + +def test_nothing_chosen_and_nonsense_both_come_back_empty() -> None: + assert SeatAccess.from_setting(None) is None + assert SeatAccess.from_setting('not-a-seat-manager') is None + + +def test_the_menu_offers_only_the_two_seat_managers() -> None: + # The menu is built by iterating the enum, so anything added to it shows up + # as an option. The old 'polkit' value must not reappear as one. + assert [seat.value for seat in SeatAccess] == ['seatd', 'systemd-logind'] + + +def test_seatd_is_installed_and_started() -> None: + assert SeatAccess.Seatd.packages == ['seatd'] + assert SeatAccess.Seatd.services == ['seatd'] + + +def test_logind_installs_polkit_and_starts_nothing() -> None: + # logind ships with systemd and its unit has no [Install] section, so there + # is nothing to install or enable for it. Arch ships polkit as an optional + # dependency of systemd, and logind checks with it before letting an + # unprivileged user act, so that has to be installed. + assert SeatAccess.Logind.packages == ['polkit'] + assert SeatAccess.Logind.services == [] + + +@pytest.mark.parametrize('profile_type', SEAT_PROFILES) +@pytest.mark.parametrize('setting', ['seatd', 'systemd-logind', 'polkit']) +def test_a_profile_installs_whatever_it_starts(profile_type: Callable[[], Profile], setting: str) -> None: + # Hyprland used to enable seatd without installing it, and enabling a unit + # that is not there fails the installation. + profile = _profile_with(profile_type, setting) + + assert set(profile.services) <= set(profile.packages) + + +@pytest.mark.parametrize('profile_type', SEAT_PROFILES) +def test_a_profile_asks_for_nothing_until_a_choice_is_made(profile_type: Callable[[], Profile]) -> None: + profile = _profile_with(profile_type, None) + + assert profile.services == [] + assert 'seatd' not in profile.packages + + +def test_seatd_puts_the_users_in_the_seat_group() -> None: + installer = FakeInstaller() + users = [User('alice', Password(plaintext='pw'), False), User('bob', Password(plaintext='pw'), False)] + + provision_seat_access(installer, users, 'seatd') # type: ignore[arg-type] + + assert installer.commands == [ + 'usermod -a -G seat alice', + 'usermod -a -G seat bob', + ] + + +@pytest.mark.parametrize('setting', ['systemd-logind', 'polkit', None]) +def test_logind_needs_no_group_membership(setting: str | None) -> None: + installer = FakeInstaller() + users = [User('alice', Password(plaintext='pw'), False)] + + provision_seat_access(installer, users, setting) # type: ignore[arg-type] + + assert installer.commands == []