Repository navigation
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>
Ivy233
force-pushed
the
fix/dde-shell-warning-batch3
branch
from
October 5, 2026 14:27
1c6db18 to
47db64e
Compare
Ivy233
force-pushed
the
fix/dde-shell-warning-batch3
branch
from
October 14, 2026 03:24
47db64e to
e551102
Compare
18202781743
reviewed
Oct 15, 2026
| // daemon's ResourceNotFound reply is authoritative and also covers | ||
| // the case where the local meta lookup is incomplete (e.g. | ||
| // linglong-only meta files). | ||
| if (resourceMissing) { |
Contributor
There was a problem hiding this comment.
daemon只是dconfig的一层dbus包装，不需要输出这一堆吧，
Author
There was a problem hiding this comment.
补充实机 journal 证据：这些行都是客户端进程打的，daemon 只是返回 error message 文本、由客户端带进日志（本机该 daemon 日志不走 journal，journal 里对应输出均在客户端进程名下）。
最新一例（trayplugin-loader，读取的 config meta 未安装）：
2026-10-15T17:07:02 trayplugin-loader[277670]: 26-10-15T17:07:02.533 [Warning] [dtk.dsg.config] <> Can't acquire config manager. error:"Can't load resource: /org.deepin.dde.dock.plugin.bluetooth, for the appid:[org.deepin.dde.dock]."
2026-10-15T18:38:12 trayplugin-loader[277670]: 26-10-15T18:38:12.813 [Warning] [dtk.dsg.config] <> DConfig is invalid of appid=org.deepin.dde.dock name=org.deepin.dde.dock.plugin.bluetooth, subpath=
该 (appid, name) 从 10-14 15:13 到 10-15 18:38 累计 13 条（5 次 acquire + 8 次 invalid）：
| 时间 | acquire | invalid | PID |
|---|---|---|---|
| 10-14 15:13:39.724 / .727 | ✅ | ✅ | 8302 |
| 10-14 15:35:06.638 | ✅ | ✅ | 4687 |
| 10-15 09:20:05.919 | — | ✅ | 4687 |
| 10-15 12:22:50.993 | — | ✅ | 4687 |
| 10-15 14:29:16.810 / .811 | ✅ | ✅ | 217513 |
| 10-15 14:29:21.654 / .655 | ✅ | ✅ | 217756 |
| 10-15 17:07:02.533 | ✅ | ✅ | 277670 |
| 10-15 18:38:12.813 | — | ✅ | 277670 |
DConfig is invalid是每次 API 访问都会打，所以 09:20 / 12:22 / 18:38 这几条没有对应的 acquire——长驻进程里会反复出现；Can't acquire只在 DConfig 对象创建（每次进程启动）时打。- 来源：dde-tray-loader 蓝牙插件经
DConfigHelper读org.deepin.dde.dock//org.deepin.dde.dock.plugin.bluetooth（bluetoothadapteritem.cpp:407-409），该 meta 有源码、没有dtk_add_config_meta_files安装项，属"资源未安装"场景。 - 规模参考：本机 10-14 至今同口径告警合计约 200 条（acquire 127 + invalid 72），除本条外还来自 dde-shell、uos-ai-assistant、dde-lock、ll-cli 等进程。
按本 PR 的判定（本地 metaPath 为空即视为资源未安装），上面 acquire / invalid 两条都会降为 debug 输出；warning 仅保留给真实故障（meta 存在但后端建立失败）。
Contributor
|
TAG Bot New tag: 6.7.51 |
Ivy233
force-pushed
the
fix/dde-shell-warning-batch3
branch
from
October 16, 2026 16:24
e551102 to
1fb8a81
Compare
DConfig::create() eagerly loads the configuration, so a missing meta file produced a qCWarning before callers could check isValid(). isValid() also called invalid(), which printed a second warning for the same probe, even though probing an optional configuration is expected behavior. Keep the warning in invalid() for data access, but make isValid() check the backend directly so the probe is silent. In load(), use the local meta lookup to demote only the expected missing-meta case to debug; real D-Bus failures (ServiceUnknown, NoReply) and parse errors keep the warning. Log: avoid DConfig warnings when probing uninstalled meta files Influence: 1. Verify isValid() does not log for an uninstalled meta 2. Verify data access on an invalid config still logs the warning 3. Verify missing meta is demoted to debug at load 4. Verify real backend failures keep the warning fix: 避免探测未安装 DConfig meta 时误告警 DConfig::create() 构造时会立即加载配置，meta 未安装时会在调用者检查 isValid() 之前先打印 qCWarning；isValid() 自身又会调用 invalid()，为同 一次探测再打印一条 warning，而探测可选配置本来就是预期行为。 保留 invalid() 对实际数据访问的告警，但让 isValid() 直接检查 backend， 使探测过程保持静默。load() 阶段使用本地 meta 查找，仅把“meta 未安装” 这一预期情况降为 debug；真实 D-Bus 故障（ServiceUnknown、NoReply）和 解析失败仍保留 warning。 Log: 避免探测未安装 DConfig meta 时误告警 Influence: 1. 验证 meta 未安装时 isValid() 不打印日志 2. 验证实际访问无效配置时仍打印告警 3. 验证 load 阶段缺 meta 降为 debug 4. 验证真实后端故障仍保留 warning PMS: TASK-394335
Ivy233
force-pushed
the
fix/dde-shell-warning-batch3
branch
from
October 19, 2026 09:22
1fb8a81 to
d8553e4
Compare
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰。metaInstalled() 方法通过创建临时 DConfigFile 检查 metaPath 是否为空，逻辑正确；DBusBackend::load() 中错误分类逻辑严密，通过 errorName.startsWith 区分真实 D-Bus 错误与通用 Failed 错误，再以本地 meta 查找区分缺资源场景；isValid() 改为直接检查 backend，避免探测时产生日志噪音，逻辑合理。#ifndef D_DISABLE_DCONFIG 守卫确保禁用 DConfig 时回退为全量 warning，边界处理完善。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整。metaInstalled() 有3行注释说明用途和查找策略；invalid() 方法新增注释解释 isValid() 不调用它的原因；DBusBackend::load() 错误处理有6行注释详述分类逻辑及兼容性考量。方法小而专注，无代码重复，无残留调试代码。qCDebug 调用为功能性日志降级，非调试残留。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。isValid() 从 !d->invalid() 改为 d->backend && d->backend->isValid()，避免了无效探测时的日志开销，略有性能提升。建议：如后续 load() 频繁失败，可考虑缓存 metaInstalled() 结果，但当前实现已足够。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 存在0个安全漏洞。本 PR 变更未引入任何安全漏洞：无用户输入未校验、无命令注入、无路径遍历、无硬编码密钥。D-Bus 错误名称比较为纯字符串操作，安全无风险。metaInstalled() 仅读取文件路径信息，不执行写入或执行操作。安全扫描器报告的6项发现均为预存在代码的误报（如将日志消息中的 call 关键词误判为 RCE），非本 PR 引入。OCR 审查结果：未发现任何问题（Looks good to me）。 💡 改进建议代码示例// 当前实现已经很好，无需修改
// 以下是 metaInstalled() 的推荐缓存优化（可选）：
class DConfigPrivate : public DObjectPrivate
{
// ...
mutable std::optional<bool> m_metaInstalledCache;
bool metaInstalled() const
{
if (!m_metaInstalledCache.has_value()) {
DConfigFile configFile(appId, name, subpath);
m_metaInstalledCache = !configFile.meta()->metaPath().isEmpty();
}
return m_metaInstalledCache.value();
}
};本报告由 AI 代码审查工具自动生成 |
Ivy233
marked this pull request as draft
October 19, 2026 09:45
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
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 an expected case: 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 most failures, so the expected case could not be told apart from real failures by error name alone.
Prefer the dedicated org.desktopspec.ConfigManager.ResourceNotFound error replied by dde-dconfig-daemon for a missing resource, and keep the local meta file lookup (DConfigMetaImpl::metaPath via DConfigFile) as a fallback for older daemons. Both messages are demoted to debug output for a missing resource, while the warning level is kept for real backend failures (meta present but the backend could not be created, e.g. dde-dconfig-daemon unavailable).
修复 meta 文件未安装时 DConfig 的告警误报：
配置资源的 meta 文件未安装属预期场景（如可选组件的配置被无条件读取），此前 "Can't acquire config manager" 与 "DConfig is invalid" 均以 warning 级别打印。daemon 对多数失败返回通用 Failed 错误，无法按错误名区分预期场景与真实故障。
现优先按 dde-dconfig-daemon 对缺资源返回的专用错误名 org.desktopspec.ConfigManager.ResourceNotFound 分流日志级别：缺资源降为 qCDebug，真实失败（如 daemon 不可用）保留 qCWarning；本地 meta 查找（DConfigFile 的 metaPath）保留为旧版 daemon 的回退路径。
配套 daemon 侧改动：linuxdeepin/dde-app-services PR #165（缺资源返回专用错误名；本 PR 单独合入亦安全，旧 daemon 下自动回退本地 meta 判断）。
PMS: TASK-394335
Summary by Sourcery
Distinguish missing DConfig meta resources from real backend failures to prevent expected warning-level log noise.
Bug Fixes:
Enhancements: