sync: from linuxdeepin/dde-session-shell #477
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
Reviewer's GuideThis PR syncs updates from linuxdeepin/dde-session-shell by removing the legacy PowerManagerInter dependency and related generated interfaces, refactoring suspend/hibernate checks to rely solely on login1 and configuration settings, and adding VM detection to prevent power actions in virtual environments. Class diagram for updated AuthInterface (removal of PowerManagerInter, new methods, and attributes)classDiagram
class AuthInterface {
+bool canSuspend()
+bool canHibernate()
+bool detectVirtualMachine()
-PowerManagerInter* m_powerManagerInter
+bool m_isVM
SessionBaseModel* m_model
QGSettings* m_gsettings
uint m_lastLogoutUid
uint m_currentUserUid
std::list<uint> m_loginUserList
}
AuthInterface --> AccountsInter
AuthInterface --> LoginedInter
AuthInterface --> DBusLogin1Manager
AuthInterface --> Login1SessionSelf
AuthInterface --> DBusObjectInter
Class diagram for removal of PowerManagerInter type aliasesclassDiagram
class DSS_DBUS {
-const QString powerManagerService
-const QString powerManagerPath
}
class org_deepin_dde {
-const QString powerManagerService
-const QString powerManagerPath
}
Class diagram for removed PowerManager1 generated interface filesclassDiagram
class PowerManager1Adaptor {
<<removed>>
}
Flow diagram for new suspend/hibernate logic in AuthInterfaceflowchart TD
A["User initiates suspend/hibernate action"] --> B["AuthInterface checks m_isVM"]
B -- "If true" --> C["Action denied"]
B -- "If false" --> D["Check environment variable (POWER_CAN_SLEEP/POWER_CAN_HIBERNATE)"]
D -- "If 0" --> C
D -- "If not 0" --> E["Check /sys/power/mem_sleep for suspend"]
E -- "If missing" --> C
E -- "If present" --> F["Query login1 for CanSuspend/CanHibernate"]
F -- "If yes" --> G["Allow action"]
F -- "If no" --> C
Flow diagram for VM detection in AuthInterfaceflowchart TD
A["detectVirtualMachine() called"] --> B["Run /usr/bin/systemd-detect-virt"]
B --> C["Process exit code"]
C -- "!= 0" --> D["Return false (not VM)"]
C -- "== 0" --> E["Read output"]
E -- "Output is 'none' or empty" --> D
E -- "Output is VM type" --> F["Return true (is VM)"]
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 there - I've reviewed your changes - here's some feedback:
- Calling detectVirtualMachine synchronously in the constructor can block startup; consider deferring VM detection or running it asynchronously.
- In canSuspend, checking for /sys/power/mem_sleep existence alone may allow unsupported modes; you should inspect the file contents to confirm the presence of a valid sleep mode (e.g., 'deep' or 's2idle').
- Replace raw getenv calls with QProcessEnvironment or qgetenv and centralize the POWER_CAN_* keys as static constants to improve consistency and null safety.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Calling detectVirtualMachine synchronously in the constructor can block startup; consider deferring VM detection or running it asynchronously.
- In canSuspend, checking for /sys/power/mem_sleep existence alone may allow unsupported modes; you should inspect the file contents to confirm the presence of a valid sleep mode (e.g., 'deep' or 's2idle').
- Replace raw getenv calls with QProcessEnvironment or qgetenv and centralize the POWER_CAN_* keys as static constants to improve consistency and null safety.
## Individual Comments
### Comment 1
<location> `src/session-widgets/authinterface.cpp:285` </location>
<code_context>
+
+bool AuthInterface::detectVirtualMachine() {
+ QProcess process;
+ process.start("/usr/bin/systemd-detect-virt", QStringList());
+ process.waitForFinished(-1);
+
</code_context>
<issue_to_address>
**suggestion:** Hardcoding the path to systemd-detect-virt may reduce portability.
Using a hardcoded path assumes systemd-detect-virt is always at /usr/bin, which may not be true on all systems. To make this more robust, use QStandardPaths or search for the executable in $PATH.
Suggested implementation:
```cpp
bool AuthInterface::detectVirtualMachine() {
QString detectVirtPath = QProcess::findExecutable("systemd-detect-virt");
if (detectVirtPath.isEmpty()) {
// Executable not found, assume not a VM
return false;
}
QProcess process;
```
```cpp
process.start(detectVirtPath, QStringList());
process.waitForFinished(-1);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| bool AuthInterface::detectVirtualMachine() { | ||
| QProcess process; | ||
| process.start("/usr/bin/systemd-detect-virt", QStringList()); |
There was a problem hiding this comment.
suggestion: Hardcoding the path to systemd-detect-virt may reduce portability.
Using a hardcoded path assumes systemd-detect-virt is always at /usr/bin, which may not be true on all systems. To make this more robust, use QStandardPaths or search for the executable in $PATH.
Suggested implementation:
bool AuthInterface::detectVirtualMachine() {
QString detectVirtPath = QProcess::findExecutable("systemd-detect-virt");
if (detectVirtPath.isEmpty()) {
// Executable not found, assume not a VM
return false;
}
QProcess process;
process.start(detectVirtPath, QStringList());
process.waitForFinished(-1);
deepin-ci-robot
force-pushed
the
sync-pr-50-nosync
branch
3 times, most recently
from
51a0131 to
0849c11
Compare
Synchronize source files from linuxdeepin/dde-session-shell. Source-pull-request: linuxdeepin/dde-session-shell#50
deepin-ci-robot
force-pushed
the
sync-pr-50-nosync
branch
from
0849c11 to
c56b476
Compare
Contributor
Author
deepin pr auto review我来对这个diff进行全面的代码审查：
// 建议添加常量定义
namespace {
constexpr int VM_DETECTION_TIMEOUT = 3000;
const QString MEM_SLEEP_PATH = "/sys/power/mem_sleep";
}
bool AuthInterface::canSuspend()
{
const char* sleepEnv = getenv("POWER_CAN_SLEEP");
if (sleepEnv && QString(sleepEnv) == "0" || m_isVM) {
return false;
}
if (!QFile::exists(MEM_SLEEP_PATH)) {
qWarning() << "Memory sleep support file not found:" << MEM_SLEEP_PATH;
return false;
}
QString canSuspend = m_login1Inter->CanSuspend();
bool result = canSuspend == "yes";
qInfo() << "Suspend support check result:" << result;
return result;
}
bool AuthInterface::detectVirtualMachine()
{
const QString detectCmd = "/usr/bin/systemd-detect-virt";
if (!QFile::exists(detectCmd)) {
qWarning() << "Virtual machine detection command not found:" << detectCmd;
return false;
}
QProcess process;
process.start(detectCmd, QStringList());
if (!process.waitForFinished(VM_DETECTION_TIMEOUT)) {
qWarning() << "Timeout detecting virtual machine after" << VM_DETECTION_TIMEOUT << "ms";
process.kill();
return false;
}
if (process.exitCode() != 0) {
qWarning() << "Failed to detect virtual machine, error:" << process.errorString();
return false;
}
QString name = QString::fromUtf8(process.readAllStandardOutput()).trimmed();
bool isVM = name != "none" && !name.isEmpty();
qInfo() << "Virtual machine detection result:" << name << "isVM:" << isVM;
return isVM;
}
这些修改提高了代码的可维护性、安全性和可读性，同时保持了原有的功能完整性。 |
fly602
approved these changes
Nov 4, 2025
Contributor
Author
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot, fly602 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
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.
Synchronize source files from linuxdeepin/dde-session-shell.
Source-pull-request: linuxdeepin/dde-session-shell#50
Summary by Sourcery
Sync upstream updates to remove the deprecated PowerManager interface, add virtual machine detection, refactor suspend/hibernate checks to rely on login1 and environment variables, and clean up obsolete XML and generated files
New Features:
Enhancements:
Build: