Updating the Plasma #254
Merged
Updating the Plasma #254
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideReplaces SDDM-specific setup with LightDM configuration, adds BIOS/UTC-based time and NTP configuration for live systems, tunes Plasma-related system settings (sysctl, xinit, devfs), and introduces polkit rules for mounting as wheel users while wiring all new setup routines into the plasma image build. Sequence diagram for live session login and privileged mount via LightDM and polkitsequenceDiagram
actor User
participant LiveSystem as Live_system
participant LightDM as LightDM_display_manager
participant Greeter as Lightdm_kde_greeter
participant Plasma as KDE_Plasma_session
participant Polkit as polkitd
participant Udisks as udisks2
participant Kernel as Kernel_devfs
User->>LiveSystem: Power_on_live_image
LiveSystem->>Kernel: Initialize_kernel_and_devfs_rules
Kernel-->>LiveSystem: Devfs_rules_applied_for_da_devices
LiveSystem->>LightDM: Start_display_manager
LightDM->>Greeter: Start_kde_greeter_session
User->>Greeter: Enter_liveuser_credentials
Greeter->>LightDM: Authenticate_liveuser
LightDM-->>LiveSystem: Start_plasma_session_for_liveuser
LiveSystem->>Plasma: Exec_startplasma_wayland_via_xinitrc
User->>Plasma: Open_file_manager_and_click_mount_disk
Plasma->>Udisks: Request_mount_da_device
Udisks->>Polkit: Check_authorization_for_mount_action
Polkit-->>Udisks: Allow_mount_for_wheel_user_per_10_mount_rules
Udisks-->>Kernel: Mount_filesystem
Kernel-->>Plasma: Device_mounted
Plasma-->>User: Mounted_volume_available_in_session
Flow diagram for plasma.sh setup routines in image buildflowchart TD
A[start_plasma_sh] --> B[load_common_finalize_and_setuser]
B --> C[patch_etc_files]
C --> D[patch_loader_conf_d]
D --> E[community_setup_liveuser]
E --> F[community_setup_autologin]
F --> G[configure_devfs]
G --> H[update_rcconf_dm_add_ntpdate_hosts]
H --> I[lightdm_setup_configure_lightdm_conf]
I --> J[lightdm_kde_greeter_conf_enable_kde_greeter]
J --> K[set_localtime_from_bios_link_UTC_and_enable_ntpd]
K --> L[plasma_settings_tune_sysctl_and_usermount]
L --> M[setup_polkit_rules_allow_mount_for_wheel]
M --> N[setup_xinit_disable_lock_screen_set_keyboard_start_plasma_wayland]
N --> O[final_setup]
O --> P[end_plasma_sh]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Several helpers like
update_rcconf_dm,plasma_settings, andconfigure_devfsalways append to config files without checking for existing lines, which can lead to duplicated settings on repeated runs; consider usingsed/grepor idempotent patterns similar to other parts of the script. - In
lightdm_setupyou assume/usr/local/etc/lightdm/lightdm.confexists and contains commentedgreeter-session/user-sessionlines; it would be safer to create the file or append appropriate sections if they are missing to avoidsedfailing or leaving LightDM misconfigured. - The skeleton
.xinitrcpath changed fromdot.xinitrcto.xinitrc; if existing tooling expects the olddot.xinitrcname, this may break user skeleton population, so please confirm the name change is intentional or keep compatibility with both.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several helpers like `update_rcconf_dm`, `plasma_settings`, and `configure_devfs` always append to config files without checking for existing lines, which can lead to duplicated settings on repeated runs; consider using `sed`/`grep` or idempotent patterns similar to other parts of the script.
- In `lightdm_setup` you assume `/usr/local/etc/lightdm/lightdm.conf` exists and contains commented `greeter-session`/`user-session` lines; it would be safer to create the file or append appropriate sections if they are missing to avoid `sed` failing or leaving LightDM misconfigured.
- The skeleton `.xinitrc` path changed from `dot.xinitrc` to `.xinitrc`; if existing tooling expects the old `dot.xinitrc` name, this may break user skeleton population, so please confirm the name change is intentional or keep compatibility with both.
## Individual Comments
### Comment 1
<location> `desktop_config/plasma.sh:18-27` </location>
<code_context>
+
+lightdm_setup() {
+ lightdm_conf="${release}/usr/local/etc/lightdm/lightdm.conf"
+ sed -i '' "s@#greeter-session=.*@greeter-session=slick-greeter@" "${lightdm_conf}"
+ sed -i '' "s@#user-session=default@user-session=plasma@" "${lightdm_conf}"
+}
+
+# ADD: Enable KDE Greeter
+lightdm_kde_greeter_conf() {
+ mkdir -p "${release}/usr/local/etc/lightdm/lightdm.conf.d"
+ cat <<EOF > "${release}/usr/local/etc/lightdm/lightdm.conf.d/50-myconfig.conf"
+[Seat:*]
+greeter-session=lightdm-kde-greeter
EOF
- else
</code_context>
<issue_to_address>
**issue (bug_risk):** The LightDM greeter is configured twice with potentially conflicting values.
`lightdm_setup` sets `greeter-session=slick-greeter` in `lightdm.conf`, while `lightdm_kde_greeter_conf` sets `greeter-session=lightdm-kde-greeter` in a drop-in. This double configuration can be confusing and may behave differently if merge/precedence changes. If the goal is to always use the KDE greeter, either remove the `slick-greeter` change or conditionally apply one of these settings so they can’t conflict.
</issue_to_address>
### Comment 2
<location> `desktop_config/plasma.sh:85-91` </location>
<code_context>
+
+}
+
+configure_devfs() {
+ devfs_rules="${release}/etc/devfs.rules"
+ rc_conf="${release}/etc/rc.conf"
+
+ echo '[localrules=10]' >> "${devfs_rules}"
+ echo "add path 'da*' mode 0666 group operator" >> "${devfs_rules}"
+ echo 'devfs_system_ruleset="localrules"' >> "${rc_conf}"
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** configure_devfs always appends rules, which can lead to duplicate or conflicting devfs configuration.
Because it always appends, rerunning this script or running it on an image that already has these entries can produce duplicate `[localrules=10]` sections and multiple `devfs_system_ruleset` lines, making the final configuration unpredictable. Please guard these writes by checking whether the entries already exist (e.g., `grep -q` before appending) or by updating existing lines with `sed` instead of blindly appending.
</issue_to_address>
### Comment 3
<location> `desktop_config/plasma.sh:55` </location>
<code_context>
+
+ echo 'net.local.stream.recvspace=65536' >> "${sysctl_conf}"
+ echo 'net.local.stream.sendspace=65536' >> "${sysctl_conf}"
+ echo 'vfs.usermount=1' >> "${sysctl_conf}"
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Appending vfs.usermount may create multiple conflicting entries in sysctl.conf.
You’re removing existing `net.local.stream.*` entries but not any existing `vfs.usermount` line before appending a new one. If the base system or a prior run already set `vfs.usermount`, you can end up with conflicting entries whose effect depends on load order. Consider first deleting any existing `^vfs.usermount` line (as you do for `net.local.*`) and then appending the desired value to keep the config deterministic.
Suggested implementation:
```
sed -i '' '/^net.local.stream.recvspace/d' "${sysctl_conf}" 2>/dev/null || true
sed -i '' '/^net.local.stream.sendspace/d' "${sysctl_conf}" 2>/dev/null || true
sed -i '' '/^vfs.usermount/d' "${sysctl_conf}" 2>/dev/null || true
```
```
echo 'net.local.stream.recvspace=65536' >> "${sysctl_conf}"
echo 'net.local.stream.sendspace=65536' >> "${sysctl_conf}"
echo 'vfs.usermount=1' >> "${sysctl_conf}"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+18
to
+27
| sed -i '' "s@#greeter-session=.*@greeter-session=slick-greeter@" "${lightdm_conf}" | ||
| sed -i '' "s@#user-session=default@user-session=plasma@" "${lightdm_conf}" | ||
| } | ||
|
|
||
| # ADD: Enable KDE Greeter | ||
| lightdm_kde_greeter_conf() { | ||
| mkdir -p "${release}/usr/local/etc/lightdm/lightdm.conf.d" | ||
| cat <<EOF > "${release}/usr/local/etc/lightdm/lightdm.conf.d/50-myconfig.conf" | ||
| [Seat:*] | ||
| greeter-session=lightdm-kde-greeter |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The LightDM greeter is configured twice with potentially conflicting values.
lightdm_setup sets greeter-session=slick-greeter in lightdm.conf, while lightdm_kde_greeter_conf sets greeter-session=lightdm-kde-greeter in a drop-in. This double configuration can be confusing and may behave differently if merge/precedence changes. If the goal is to always use the KDE greeter, either remove the slick-greeter change or conditionally apply one of these settings so they can’t conflict.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove the old package and updated the plasma.sh wby adding polkit and bios based time on liveuser mode.
Summary by Sourcery
Update Plasma live environment configuration to use LightDM with a KDE greeter, Wayland sessions, and improved system defaults for time, device access, and resource limits.
Enhancements:
Chores: