chore: clean dependencies #75
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
Since the removal of greeter, helper and auth, and the refactor of existing components, many old dependencies is exactly obsolete today. Remove them.
|
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 GuideCleans up build and packaging dependencies after removal of greeter/helper/auth/QML components, modernizes pkg-config usage, simplifies Qt usage to Qt6 Core/DBus/Network, removes optional journald feature flagging in favor of always using libsystemd journald logging, and tightens error handling and build warnings. Sequence diagram for daemon logging via journald and fallback handlerssequenceDiagram
participant Qt as QtLoggingSystem
participant Daemon as DaemonCode
participant MH as DDM_MessageHandler
participant Journald as systemd_journal
participant FileLog as LogFile
participant Stderr as stderr
Qt->>Daemon: qInfo/qWarning/qCritical(msg)
Daemon->>MH: DaemonMessageHandler(type, context, msg)
MH->>MH: messageHandler(type, context, prefix, msg)
MH->>MH: isInteractive = isatty(STDERR_FILENO) && qgetenv(USER) != dde
alt non_interactive
MH->>Journald: journaldLogger(type, context, msg)
else interactive
MH->>FileLog: standardLogger(type, msg)
FileLog-->>MH: append to log file
MH->>Stderr: stderrLogger(type, msg)
end
Sequence diagram for Auth SessionLeader pipe writes with error handlingsequenceDiagram
participant Parent as ParentProcess
participant SL as SessionLeader
participant Pipe as UnixPipe
Parent->>SL: fork and set up pipe
SL->>SL: xdgSessionId = getenv(XDG_SESSION_ID)
SL->>Pipe: write(pipefd[1], &xdgSessionId, sizeof(int))
alt write_xdgSessionId_failed
Pipe-->>SL: return < 0
SL->>SL: qCritical Failed to write XDG_SESSION_ID
SL->>SL: exit(1)
else write_xdgSessionId_ok
Pipe-->>SL: return >= 0
SL->>SL: start UserSession
SL->>SL: sessionPid = session.processId()
SL->>Pipe: write(pipefd[1], &sessionPid, sizeof(qint64))
alt write_sessionPid_failed
Pipe-->>SL: return < 0
SL->>SL: qCritical Failed to write session PID
SL->>SL: exit(1)
else write_sessionPid_ok
Pipe-->>SL: return >= 0
SL-->>Parent: pipe conveys xdgSessionId and sessionPid
SL->>SL: session.waitForFinished(-1)
end
end
Class diagram for updated DDM MessageHandler logging functionsclassDiagram
namespace DDM {
class MessageHandlerFunctions {
+static void journaldLogger(QtMsgType type, QMessageLogContext context, QString msg)
+static void standardLogger(QtMsgType type, QString msg)
+static void stderrLogger(QtMsgType type, QString msg)
+static void messageHandler(QtMsgType type, QMessageLogContext context, QString prefix, QString msg)
+static void DaemonMessageHandler(QtMsgType type, QMessageLogContext context, QString msg)
}
}
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 left some high level feedback:
- Enabling
-Werrorglobally in the top-levelCMakeLists.txtcan make the project fragile across different compilers and distro toolchains; consider making this configurable via an option (e.g.ENABLE_WERROR) and defaulting it off for release/packaging builds. - The removal of
ENABLE_JOURNALDandHAVE_JOURNALDnow makes libsystemd/sd-journal a hard, unconditional dependency; if you still want to support non-systemd environments or alternate logging backends, consider reintroducing a feature option and guarding thesd-journalinclude and usage behind it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Enabling `-Werror` globally in the top-level `CMakeLists.txt` can make the project fragile across different compilers and distro toolchains; consider making this configurable via an option (e.g. `ENABLE_WERROR`) and defaulting it off for release/packaging builds.
- The removal of `ENABLE_JOURNALD` and `HAVE_JOURNALD` now makes libsystemd/sd-journal a hard, unconditional dependency; if you still want to support non-systemd environments or alternate logging backends, consider reintroducing a feature option and guarding the `sd-journal` include and usage behind it.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zccrs
requested changes
Jan 23, 2026
calsys456
force-pushed
the
master
branch
2 times, most recently
from
cdc9070 to
ac408d3
Compare
zccrs
previously approved these changes
Jan 23, 2026
There was a problem hiding this comment.
Pull request overview
This PR performs a comprehensive cleanup of obsolete dependencies following the removal of greeter, helper, and auth components, and refactoring of existing components. The changes modernize the build system, simplify Qt usage, and enforce stricter Linux/systemd requirements.
Changes:
- Modernized CMake build configuration by replacing custom FindPAM with pkg-config modules and using IMPORTED_TARGET pattern
- Simplified Qt dependencies to Qt6 Core/DBus/Network only, removing all QML-related components
- Made systemd/journald mandatory by removing conditional compilation flags
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Replaced custom PAM finder with pkg_search_module, added WERROR option, removed journald conditional logic, streamlined Qt6 dependencies |
| src/CMakeLists.txt | Removed obsolete Qt policy configuration |
| src/common/CMakeLists.txt | Updated to use Qt6 and PkgConfig::LibXau directly |
| src/common/Constants.h.in | Removed unused QML imports directory constant |
| src/common/MessageHandler.h | Removed conditional journald compilation, deleted unused Greeter and Helper message handlers |
| src/daemon/CMakeLists.txt | Removed manual include_directories, switched to PkgConfig IMPORTED_TARGET pattern for cleaner dependency management |
| src/daemon/Auth.cpp | Added error checking for write() calls when communicating session IDs and PIDs over pipes |
| cmake/FindPAM.cmake | Deleted custom PAM finder in favor of pkg-config |
| debian/control | Removed QML module dependencies, simplified to essential build requirements |
| debian/rules | Removed NO_SYSTEMD and ENABLE_JOURNALD flags (now Linux-only) |
| debian/ddm.postinst | Replaced manual user/group creation with systemd-sysusers and systemd-tmpfiles |
| services/debian.ddm.pam | Added pam_systemd.so for proper systemd session management |
| services/ddm.pam | Added pam_systemd.so for proper systemd session management |
| nix/default.nix | Removed pixman dependency, fixed SDDM_INITIAL_VT to DDM_INITIAL_VT, removed QML import path |
| flake.nix | Removed QML plugin paths from development shell environment |
calsys456
force-pushed
the
master
branch
3 times, most recently
from
dbaa552 to
4cd05d7
Compare
zccrs
approved these changes
Jan 23, 2026
|
[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.
Since the removal of greeter, helper and auth, and the refactor of existing components, many old dependencies is exactly obsolete today. Remove them.
Summary by Sourcery
Clean up obsolete dependencies and configuration while tightening build warnings and simplifying logging and Qt usage.
Enhancements:
Build:
Chores: