refactor: Remove unused code #58
Merged
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
|
Hi @calsys456. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's GuideRefactor the display/login daemon by removing the Seat abstraction and unused configuration/theme/test code, consolidating seat management into SeatManager/Display/Auth, and simplifying the build and packaging to rely only on systemd (no elogind/XCB/XKB, no greeter PAM or avatar assets). Sequence diagram for updated login flow via DisplaysequenceDiagram
actor User
participant Treeland as TreelandGreeter
participant Socket as QLocalSocket
participant Display
participant Auth
participant Logind as systemd_logind
participant Xorg as XorgDisplayServer
User->>Treeland: Enter credentials
Treeland->>Socket: Send login(user, password, session)
Socket->>Display: login(socket, user, password, session)
alt user is dde
Display-->>Socket: loginFailed(socket, user)
Display->>Display: return
end
rect rgb(235,235,235)
note over Display,Auth: Reuse or create Auth for this user
end
Display->>Display: Find existing Auth in auths
alt found
Display->>Auth: use existing instance
else not found
Display->>Auth: new Auth(Display, user)
end
Display->>Auth: authenticate(password)
alt auth fails
Auth-->>Display: false
Display-->>Socket: loginFailed(socket, user)
Display->>Display: return
else auth ok
Auth-->>Display: true
end
Display->>Display: Prepare session env (XDG_*, etc.)
Display->>Logind: openSession(env)
Logind-->>Auth: XDG_SESSION_ID
Auth->>Auth: xdgSessionId set
alt session.isSingleMode
Display->>Auth: set type = Treeland, tty = new VT
Display->>Treeland: onLoginSucceeded(user)
Display->>Display: activateSession(user, xdgSessionId)
else session.xdgSessionType == x11
Display->>Auth: set type = X11
Display->>Treeland: stop()
Display->>Xorg: start(tty)
Xorg-->>Display: display, cookie
else Wayland session
Display->>Auth: set type = Wayland
Display->>Treeland: stop()
end
Display->>Auth: startUserProcess(session.exec(), cookie)
Auth-->>Display: userProcessStarted
Display->>Display: auths << auth
Display-->>Socket: loginSucceeded(socket, user)
Sequence diagram for updated unlock (re-login) flowsequenceDiagram
actor User
participant Treeland as TreelandGreeter
participant Socket as QLocalSocket
participant Display
participant TempAuth as Auth(temp)
participant AuthList as ExistingAuth
participant VT as VirtualTerminal
User->>Treeland: Enter password on lock screen
Treeland->>Socket: Send unlock(user, password)
Socket->>Display: unlock(socket, user, password)
alt user is dde
Display-->>Socket: loginFailed(socket, user)
Socket->>Socket: return
end
Display->>TempAuth: Auth(Display, user)
Display->>TempAuth: authenticate(password)
alt auth fails
TempAuth-->>Display: false
Display-->>Socket: loginFailed(socket, user)
Socket->>Socket: return
else auth ok
TempAuth-->>Display: true
end
Display->>Display: Update last activated user in config
loop search matching session
Display->>AuthList: iterate over auths
end
alt found Treeland session
Display-->>Socket: loginSucceeded(socket, user)
Display->>Display: activateSession(user, xdgSessionId)
else found X11/Wayland session
Display->>VT: jumpToVt(AuthList.tty, false)
else no matching session
Display-->>Socket: loginFailed(socket, user)
end
Class diagram for updated seat/display/auth architectureclassDiagram
class SeatManager {
+QList~Display*~ displays
+QHash~QString, LogindSeat*~ systemSeats
+void initialize()
+void createSeat(QString name)
+void removeSeat(QString name)
+void switchToGreeter(QString seat)
+void logindSeatAdded(QString name, QDBusObjectPath objectPath)
+void logindSeatRemoved(QString name, QDBusObjectPath objectPath)
+void displayStopped()
-void startDisplay(Display* display, int tryNr)
}
class Display {
<<QObject>>
+QString name
+int terminalId
+QList~Auth*~ auths
-bool m_started
-TreelandDisplayServer* m_treeland
-XorgDisplayServer* m_x11Server
-SocketServer* m_socketServer
+Display(SeatManager* parent, QString name)
+~Display()
+bool start()
+void stop()
+void activateSession(QString user, int xdgSessionId)
+void connected(QLocalSocket* socket)
+void login(QLocalSocket* socket, QString user, QString password, Session session)
+void logout(QLocalSocket* socket, int id)
+void unlock(QLocalSocket* socket, QString user, QString password)
+void userProcessFinished(int status)
+signal void stopped()
+signal void loginFailed(QLocalSocket* socket, QString user)
+signal void loginSucceeded(QLocalSocket* socket, QString user)
}
class Auth {
<<QObject>>
+bool active
+QString user
+Display::DisplayServerType type
+QString sessionId
+QString display
+int tty
+int xdgSessionId
-Pam* m_pam
-UserSession* m_session
-QProcessEnvironment m_env
+Auth(QObject* parent, QString user)
+~Auth()
+bool authenticate(QByteArray secret)
+int openSession(QProcessEnvironment env)
+void startUserProcess(QString command, QByteArray cookie)
+void stop()
+signal void userProcessFinished(int status)
}
class TreelandDisplayServer {
+bool start(int tty)
+void stop()
+void activateUser(QString user, int xdgSessionId)
}
class XorgDisplayServer {
+QString display
+QByteArray cookie()
+bool start(int vt)
+signal void stopped()
}
class SeatManagerLogindSeatNote {
<<LogindSeat>>
+QString name
+QDBusObjectPath objectPath
+bool canGraphical()
}
SeatManager "1" o-- "*" Display : manages
SeatManager "*" o-- "*" SeatManagerLogindSeatNote : systemSeats
Display "1" o-- "*" Auth : owns auths
Display "1" o-- "1" TreelandDisplayServer : uses
Display "0..1" o-- "1" XorgDisplayServer : optional X11
Auth "1" --> "1" Pam : uses
Auth "1" --> "1" UserSession : runs process
class Pam {
+QString user
+bool sessionOpened
+bool start()
+bool authenticate(QByteArray secret)
+std::optional~QProcessEnvironment~ openSession(QProcessEnvironment env)
+void closeSession()
}
class UserSession {
+void setProcessEnvironment(QProcessEnvironment env)
+void start(QString command, Display::DisplayServerType type, QByteArray cookie)
+void stop()
+qint64 processId()
+QProcessEnvironment processEnvironment()
+signal void finished(int exitCode, QProcess::ExitStatus status)
}
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, and left some high level feedback:
- In SeatManager::createSeat(), the new Display is added to the displays list twice (using both 'displays << display;' and 'displays.append(display);'); one of these should be removed to avoid duplicate entries.
- SeatManager now exposes 'displays' and 'systemSeats' as public mutable members; consider making these private with accessor methods to prevent unintended external modification and keep ownership/lifecycle centralized in SeatManager.
- The new SeatManager::startDisplay() retry logic uses synchronous QThread::msleep(500) in Display::startAuth paths and hard-coded retry counts/delays; consider making these constants/configurable and avoiding blocking sleeps in the main thread if this runs in the GUI/event thread.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In SeatManager::createSeat(), the new Display is added to the displays list twice (using both 'displays << display;' and 'displays.append(display);'); one of these should be removed to avoid duplicate entries.
- SeatManager now exposes 'displays' and 'systemSeats' as public mutable members; consider making these private with accessor methods to prevent unintended external modification and keep ownership/lifecycle centralized in SeatManager.
- The new SeatManager::startDisplay() retry logic uses synchronous QThread::msleep(500) in Display::startAuth paths and hard-coded retry counts/delays; consider making these constants/configurable and avoiding blocking sleeps in the main thread if this runs in the GUI/event thread.
## Individual Comments
### Comment 1
<location> `src/daemon/SeatManager.cpp:127-136` </location>
<code_context>
- return;
- }
-
- // create a new display
- qDebug() << "Adding new display...";
- Display *display = new Display(this);
</code_context>
<issue_to_address>
**issue (bug_risk):** Display is inserted into `displays` twice in `createSeat`, which can cause duplicate entries and confusing behavior.
In `createSeat`, `display` is pushed into `displays` twice (`<<` and `append`), leaving duplicate pointers to the same instance. This can cause double-iteration side effects and unexpected behavior with removal operations. Please keep only one insertion (e.g., drop the first one before `startDisplay(display)`).
</issue_to_address>
### Comment 2
<location> `CMakeLists.txt:129-130` </location>
<code_context>
+if (NOT SYSTEMD_FOUND)
# Set the VT on which ddm will normally appear, and the
# commands for shutdown and reboot. On FreeBSD, there are
# normally more getty's running than on Linux.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider correcting the plural form of "getty's" in this comment.
Since "getty's" is used as a plural here, the apostrophe is incorrect. Please change it to a plural form like "gettys" or "getties" to avoid confusion.
```suggestion
# commands for shutdown and reboot. On FreeBSD, there are
# normally more gettys running than on Linux.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zccrs
requested changes
Dec 24, 2025
zccrs
previously approved these changes
Dec 24, 2025
移除不再需要的功能和依赖项，精简项目结构： - 将 Seat 类并入 SeatManager 和 Display - 删除无用的配置项 - 移除 elogind 支持，统一使用 systemd - 移除对 XCB 和 XKB 的依赖 - 清理未使用的类：ThemeConfig、ThemeMetadata、SafeDataStream - 删除不需要的资源文件：国旗图片、默认头像 - 维护手册 - 移除测试代码和相关构建配置 - 清理旧的 PAM 配置文件和 systemd 服务文件 - 增加注释和文档 这些更改简化了构建系统，减少了维护负担，使代码易于上手维护
zccrs
approved these changes
Dec 24, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: calsys456, zccrs 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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
移除不再需要的功能和依赖项，精简项目结构：
这些更改简化了构建系统，减少了维护负担，使代码易于上手维护
Summary by Sourcery
Simplify the daemon’s seat/display and authentication handling while removing unused features, dependencies, and test-only paths.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores: