Fix seat access selection and provisioning

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

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

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

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

archinstall/default_profiles/desktops/utils.py

@@ -1,24 +1,79 @@
 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(
 	install_session: Installer,
 	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,

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 == []