cherry-pick bug-fix of release/eagle#360
Merged
Merged
Conversation
Reviewer's GuideThis PR cherry-picks a bug fix to ensure QApt transactions run under the real user’s environment (particularly when launched via sudo), by passing the appropriate SUDO_USER value into uninstall and install transactions when virtual package enhancements are enabled. Sequence diagram for installDebs transaction with real user environmentsequenceDiagram
actor User
participant FileManager
participant DebInstaller as DebInstallerApp
participant DebListModel
participant QAptTransaction
User->>FileManager: Double-click .deb/.ddim
FileManager->>DebInstaller: Launch with package path
DebInstaller->>DebListModel: installDebs()
DebListModel->>DebListModel: Create QAptTransaction transaction
DebListModel->>DebListModel: m_currentTransaction = transaction
opt ENABLE_VIRTUAL_PACKAGE_ENHANCE
DebListModel->>DebListModel: env = QProcessEnvironment::systemEnvironment()
DebListModel->>DebListModel: currentUser = env.value(USER)
DebListModel->>DebListModel: realUser = env.value(SUDO_USER)
DebListModel->>DebListModel: if realUser is empty then realUser = currentUser
DebListModel->>QAptTransaction: setEnvVariable({ SUDO_USER: realUser })
end
DebListModel->>QAptTransaction: run()
QAptTransaction-->>DebListModel: Installation result
Sequence diagram for slotUninstallPackage with real user environmentsequenceDiagram
actor User
participant DebInstaller as DebInstallerApp
participant DebListModel
participant QAptTransaction
User->>DebInstaller: Request uninstall package
DebInstaller->>DebListModel: slotUninstallPackage(index)
DebListModel->>DebListModel: Create QAptTransaction transsaction
DebListModel->>DebListModel: m_currentTransaction = transsaction
opt ENABLE_VIRTUAL_PACKAGE_ENHANCE
DebListModel->>DebListModel: env = QProcessEnvironment::systemEnvironment()
DebListModel->>DebListModel: currentUser = env.value(USER)
DebListModel->>DebListModel: realUser = env.value(SUDO_USER)
DebListModel->>DebListModel: if realUser is empty then realUser = currentUser
DebListModel->>QAptTransaction: setEnvVariable({ SUDO_USER: realUser })
end
DebListModel->>QAptTransaction: run()
QAptTransaction-->>DebListModel: Uninstall result
Class diagram for DebListModel environment-aware transactionsclassDiagram
class DebListModel {
- QAptTransaction* m_currentTransaction
+ bool slotUninstallPackage(int index)
+ void installDebs()
}
class QAptTransaction {
+ void setEnvVariable(QVariantMap env)
+ void run()
}
class QProcessEnvironment {
+ static QProcessEnvironment systemEnvironment()
+ QString value(QString name)
}
class QVariantMap
class QString
DebListModel --> QAptTransaction : uses
DebListModel --> QProcessEnvironment : reads_system_env
DebListModel --> QVariantMap : builds_env_map
DebListModel --> QString : uses_for_usernames
QAptTransaction --> QVariantMap : env_variables
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:
- The logic that builds and sets the SUDO_USER environment for transactions is duplicated in both uninstall and install; consider extracting this into a small helper method to keep the behavior in one place and reduce maintenance overhead.
- In slotUninstallPackage, m_currentTransaction is assigned twice (once before the #ifdef and once inside it); the second assignment appears redundant and could be removed to avoid confusion about when the member is initialized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic that builds and sets the SUDO_USER environment for transactions is duplicated in both uninstall and install; consider extracting this into a small helper method to keep the behavior in one place and reduce maintenance overhead.
- In slotUninstallPackage, m_currentTransaction is assigned twice (once before the #ifdef and once inside it); the second assignment appears redundant and could be removed to avoid confusion about when the member is initialized.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
lzwind
previously approved these changes
Dec 18, 2025
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
双击ddim文件使用软件包安装器打开 Bug: https://pms.uniontech.com/bug-view-285473.html Log: 双击ddim文件使用软件包安装器打开
deepin pr auto review我来对这个diff进行详细的代码审查：
(1) 代码重复： // 在slotUninstallPackage和installDebs中存在重复代码块
#ifdef ENABLE_QAPT_SETENV
QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
QVariantMap map;
// 获取当前真实用户信息
QString currentUser = env.value("USER");
// 如果SUDO_USER存在，说明当前是通过sudo启动的
QString realUser = env.value("SUDO_USER");
if (realUser.isEmpty())
realUser = currentUser;
map.insert("SUDO_USER", realUser);
transsaction->setEnvVariable(map);
#endif建议：将这段代码抽取为一个独立的方法，如 (2) 变量重复赋值： m_currentTransaction = transsaction; // 第一次赋值
...
m_currentTransaction = transsaction; // 在#ifdef块内重复赋值建议：删除重复赋值，保留一处即可。
(1) 重构代码： void DebListModel::setTransactionEnvironment(QApt::Transaction* transaction) {
#ifdef ENABLE_QAPT_SETENV
static const QString realUser = []() {
QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
QString user = env.value("SUDO_USER");
return user.isEmpty() ? env.value("USER") : user;
}();
QVariantMap envMap;
envMap.insert("SUDO_USER", realUser);
transaction->setEnvVariable(envMap);
#endif
}(2) 使用lambda表达式和static变量来缓存环境变量值，避免重复获取 (3) 增加错误处理： if (!transaction) {
qCWarning(appLog) << "Invalid transaction pointer";
return;
}
这些改进将使代码更加健壮、可维护，并提高代码复用性。 |
lzwind
approved these changes
Dec 18, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LiHua000, lzwind 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 |
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
Author
|
/merge |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
fix: [env] set user`s environment to QApt (#334)
fix: 双击ddim文件使用软件包安装器打开
Summary by Sourcery
Propagate the real invoking user into QApt transactions when uninstalling or installing packages to ensure the correct environment is used for deb operations.
Bug Fixes: