feat(accounts1): support autologin configuration for DDM - #1224
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideThe accounts module now detects DDM, reads and writes autologin settings in /etc/ddm.conf, and forces the default user session to Treeland while DDM is active; accompanying fixtures and tests cover DDM configuration parsing and service detection. Sequence diagram for DDM autologin configurationsequenceDiagram
participant Accounts as Accounts module
participant DM as Display manager detection
participant Config as /etc/ddm.conf
Accounts->>DM: GetDefaultDM()
DM-->>Accounts: ddm
Accounts->>Config: GetAutoLoginUser()
Config-->>Accounts: Autologin.User
Accounts->>Config: SetAutoLoginUser(username, session)
Config-->>Accounts: updated Autologin.User and Session
Flow diagram for DDM default session selectionflowchart TD
Start[Load user configuration] --> Detect["GetDefaultDM()"]
Detect --> IsDDM{Display manager is ddm?}
IsDDM -->|Yes| Force[Set XSession to treeland]
IsDDM -->|No| Existing{XSession is empty?}
Existing -->|Yes| Default["GetDefaultXSession()"]
Existing -->|No| Keep[Keep configured session]
Force --> Save[Persist user configuration]
Default --> Save
Keep --> End[Finish]
Save --> End
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}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="accounts1/users/display_manager.go" line_range="123-130" />
<code_context>
}
return setIniKeys(sddmConfig, kfGroupSDDMAutologin,
keys, values)
+ case "ddm":
+ keys := []string{kfKeyDDMUser}
+ values := []string{username}
+ if session != "" {
+ keys = append(keys, kfKeyDDMSession)
+ values = append(values, session)
+ }
+ return setIniKeys(ddmConfig, kfGroupDDMAutologin,
+ keys, values)
case "lxdm":
</code_context>
<issue_to_address>
**issue (bug_risk):** SetAutoLoginUser returns before the new DDM branch whenever the configured autologin username already equals `username`, so changing only the session leaves the old `Session` value in `/etc/ddm.conf`. The DDM session setting is therefore not persisted for an already-configured user.
**Triggers:** When autologin remains enabled for the same user but the user changes the session.
**Suggested fix:** Compare the existing session as well as the username before returning, or move the unchanged-username fast path after applying session changes.
</issue_to_address>
### Comment 2
<location path="accounts1/users/display_manager.go" line_range="126-130" />
<code_context>
+ case "ddm":
+ keys := []string{kfKeyDDMUser}
+ values := []string{username}
+ if session != "" {
+ keys = append(keys, kfKeyDDMSession)
+ values = append(values, session)
+ }
+ return setIniKeys(ddmConfig, kfGroupDDMAutologin,
+ keys, values)
case "lxdm":
</code_context>
<issue_to_address>
**issue (bug_risk):** The DDM configuration fixture uses `Session=treeland.desktop`, but the account's forced session value is `treeland` and SetAutoLoginUser writes the supplied value unchanged. Enabling DDM autologin therefore writes `Session=treeland`, which does not match the DDM session identifier used by the configuration and prevents the requested Treeland session from being selected.
**Triggers:** When DDM autologin is enabled through the accounts module.
**Suggested fix:** Use the DDM desktop-entry identifier (`treeland.desktop`) when writing the DDM `Session` key, while retaining the account module's internal `XSession` representation if needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| case "ddm": | ||
| keys := []string{kfKeyDDMUser} | ||
| values := []string{username} | ||
| if session != "" { | ||
| keys = append(keys, kfKeyDDMSession) | ||
| values = append(values, session) | ||
| } | ||
| return setIniKeys(ddmConfig, kfGroupDDMAutologin, |
There was a problem hiding this comment.
issue (bug_risk): SetAutoLoginUser returns before the new DDM branch whenever the configured autologin username already equals username, so changing only the session leaves the old Session value in /etc/ddm.conf. The DDM session setting is therefore not persisted for an already-configured user.
Triggers: When autologin remains enabled for the same user but the user changes the session.
Suggested fix: Compare the existing session as well as the username before returning, or move the unchanged-username fast path after applying session changes.
Add DDM support to the accounts module, so the autologin setting is persisted to /etc/ddm.conf and the default session is forced to treeland when DDM is active. 为账户模块增加DDM适配，自动登录配置写入 /etc/ddm.conf，并 在DDM下强制默认会话为treeland。 Log: 适配DDM自动登录配置 PMS: BUG-294419 Influence: DDM环境下自动登录配置生效，默认会话固定为treeland。
deepin pr auto review★ 总体评分：93分 (大于70分通过)■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 建议1：记录 GetDefaultDM 错误日志（accounts1/user.go） // 修改前:
if dm, _ := users.GetDefaultDM(); dm == "ddm" {
if u.XSession != "treeland" {
u.XSession = "treeland"
isSave = true
}
} else if u.XSession == "" {
// 修改后:
dm, err := users.GetDefaultDM()
if err != nil {
logger.Warning("failed to get default display manager:", err)
}
if dm == "ddm" {
if u.XSession != ddmDefaultSession {
u.XSession = ddmDefaultSession
isSave = true
}
} else if u.XSession == "" {建议2：提取 treeland 为常量（accounts1/users/display_manager.go） // 在 const 块中添加:
const ddmDefaultSession = "treeland"
// GetDefaultXSession 中使用常量:
case "ddm":
return ddmDefaultSession, nil
// user.go 中使用常量:
u.XSession = ddmDefaultSession建议3：补充 GetDefaultDM 函数注释（accounts1/users/display_manager.go） // GetDefaultDM return the current display manager name.
// It first reads /etc/X11/default-display-manager file,
// and falls back to checking systemd display-manager.service symlink on failure.
func GetDefaultDM() (string, error) { |
2 participants
Add DDM support to the accounts module, so the autologin setting is persisted to /etc/ddm.conf and the default session is forced to treeland when DDM is active.
为账户模块增加DDM适配，自动登录配置写入 /etc/ddm.conf，并
在DDM下强制默认会话为treeland。
Log: 适配DDM自动登录配置
PMS: BUG-294419
Influence: DDM环境下自动登录配置生效，默认会话固定为treeland。
Summary by Sourcery
Support DDM account integration by persisting autologin settings and enforcing treeland as the default session.
New Features:
Bug Fixes:
Enhancements:
Tests: