Conversation
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ivy233 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 PR uses DConfigFile's daemon-aligned meta-path lookup to identify uninstalled config resources and demotes the two resulting expected warnings to debug output, while keeping warnings for failures when metadata exists, such as an unavailable or uncreatable backend. Sequence diagram for DConfig missing-meta diagnostic handlingsequenceDiagram
participant Client
participant DConfigPrivate
participant DConfigFile
participant Daemon as dde-dconfig-daemon
participant Logger
Client->>DConfigPrivate: invalid()
DConfigPrivate->>Daemon: acquire config manager
Daemon-->>DConfigPrivate: DBus reply
alt manager acquisition fails
DConfigPrivate->>DConfigFile: metaPath()
DConfigFile-->>DConfigPrivate: empty or present path
alt meta file not installed
DConfigPrivate->>Logger: qCDebug(Can't acquire config manager)
else meta file present
DConfigPrivate->>Logger: qCWarning(Can't acquire config manager)
end
end
DConfigPrivate->>DConfigFile: metaPath()
DConfigFile-->>DConfigPrivate: empty or present path
alt backend invalid and meta missing
DConfigPrivate->>Logger: qCDebug(DConfig is invalid)
else backend invalid and meta present
DConfigPrivate->>Logger: qCWarning(DConfig is invalid)
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/dconfig.cpp" line_range="158-165" />
<code_context>
virtual ~DConfigPrivate() override;
+ // Checks whether the meta file for this config is installed locally,
+ // using the same lookup logic as dde-dconfig-daemon
+ // (DConfigMetaImpl::metaPath). Used to distinguish "config resource
+ // never installed" (expected, debug-level) from real backend failures.
+ bool metaInstalled() const
+ {
+ DConfigFile configFile(appId, name, subpath);
+ return !configFile.meta()->metaPath().isEmpty();
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When `D_DISABLE_DCONFIG` is defined, `dconfig.cpp` deliberately does not include `dconfigfile.h`, but the new `metaInstalled()` method still instantiates `DConfigFile`; that configuration therefore fails to compile with an unknown type or incomplete declaration.
**Triggers:** When building the supported `D_DISABLE_DCONFIG` configuration.
**Suggested fix:** Guard `metaInstalled()` and its call sites with `#ifndef D_DISABLE_DCONFIG`, or provide an equivalent implementation that does not depend on `DConfigFile` in that configuration.
</issue_to_address>
"Can't acquire config manager" and "DConfig is invalid" were printed at warning level in two expected cases: the config resource's meta file is simply not installed on the system (e.g. an optional component's config accessed unconditionally). The daemon replies with a generic org.freedesktop.DBus.Error.Failed for a missing resource, which cannot be distinguished from real failures by error name. Check the meta path locally with the same lookup logic the daemon uses (DConfigMetaImpl::metaPath via DConfigFile) and demote both messages to debug output when the meta file is missing, keeping the warning level for real backend failures (meta present but the backend could not be created, e.g. dde-dconfig-daemon unavailable). 修复元数据未安装时 DConfig 的告警误报： 配置资源的 meta 文件未安装属预期场景（如可选组件的配置被无条件 读取），此前 "Can't acquire config manager" 与 "DConfig is invalid" 均以 warning 级别打印。daemon 对缺资源返回通用 Failed 错误，无法按 错误名区分。现用与 daemon 同源的 meta 查找（DConfigFile 的 metaPath）在本地判断：meta 未安装降为 qCDebug，meta 在位但后端 创建失败（如 daemon 不可用）保留 qCWarning。 PMS: TASK-394335
Ivy233
force-pushed
the
fix/dde-shell-warning-batch3
branch
from
October 5, 2026 14:27
1c6db18 to
47db64e
Compare
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰，无需修改 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整，无需修改 3. 代码性能 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 建议缓存 metaInstalled() 的结果（如在 DConfigPrivate 中添加 mutable bool metaCached 和 bool metaInstalledResult 成员），避免在错误路径中重复创建 DConfigFile 和执行文件系统查找 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 无安全风险，安全合规 💡 改进建议代码示例// 建议在 DConfigPrivate 中缓存 metaInstalled() 结果
#ifndef D_DISABLE_DCONFIG
mutable bool metaCached = false;
mutable bool metaInstalledResult = false;
bool metaInstalled() const
{
if (!metaCached) {
DConfigFile configFile(appId, name, subpath);
metaInstalledResult = !configFile.meta()->metaPath().isEmpty();
metaCached = true;
}
return metaInstalledResult;
}
#endif本报告由 AI 代码审查工具自动生成 |
This branch has not been deployed
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
2 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.
"Can't acquire config manager" and "DConfig is invalid" were printed at warning level in two expected cases: the config resource's meta file is simply not installed on the system (e.g. an optional component's config accessed unconditionally). The daemon replies with a generic org.freedesktop.DBus.Error.Failed for a missing resource, which cannot be distinguished from real failures by error name.
Check the meta path locally with the same lookup logic the daemon uses (DConfigMetaImpl::metaPath via DConfigFile) and demote both messages to debug output when the meta file is missing, keeping the warning level for real backend failures (meta present but the backend could not be created, e.g. dde-dconfig-daemon unavailable).
修复元数据未安装时 DConfig 的告警误报：
配置资源的 meta 文件未安装属预期场景（如可选组件的配置被无条件
读取），此前 "Can't acquire config manager" 与 "DConfig is invalid" 均以 warning 级别打印。daemon 对缺资源返回通用 Failed 错误，无法按
错误名区分。现用与 daemon 同源的 meta 查找（DConfigFile 的
metaPath）在本地判断：meta 未安装降为 qCDebug，meta 在位但后端
创建失败（如 daemon 不可用）保留 qCWarning。
PMS: TASK-394335
Summary by Sourcery
Reduce false-positive DConfig warnings for configuration resources whose meta files are not installed.
Bug Fixes:
Enhancements: