feat: manage QML cache versioning proactively#2898
Conversation
Reviewer's GuideImplements proactive QML disk cache versioning by introducing a refreshQmlCache helper, wiring it into main() early in application startup, and adjusting application version initialization so that stale QML cache directories are removed when the app version changes. Sequence diagram for application startup with QML cache refreshsequenceDiagram
actor User
participant App as Application
participant Env as Environment
participant FS as FileSystem
User->>App: start(argc, argv)
App->>App: setOrganizationName
App->>App: setApplicationName
App->>App: determine applicationVersion
App->>App: setApplicationVersion(version)
App->>Env: qgetenv(QML_DISK_CACHE_PATH)
Env-->>App: envCachePath
App->>FS: refreshQmlCache(version)
FS->>FS: resolve cache directory
FS->>FS: check directory exists
alt cache directory exists
FS->>FS: check subdirectory version exists
alt version directory exists
FS-->>App: keep existing cache
else version directory does not exist
FS->>FS: removeRecursively(cacheDirectory)
FS-->>App: old cache removed
FS->>FS: mkpath(version)
FS-->>App: new version cache created
end
else cache directory does not exist
FS->>FS: mkpath(version)
FS-->>App: version cache created
end
App->>App: continue startup and QML loading
App-->>User: control center ready
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Using
app->applicationVersion()directly as a directory name may lead to invalid or problematic paths on some platforms (e.g. characters like/,:); consider normalizing/sanitizing the version string before using it inrefreshQmlCache. - The cache invalidation path logs with
qWarning()on every version change, which is expected rather than exceptional behavior; consider downgrading this toqInfo()or adding more context so that real issues stand out in logs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `app->applicationVersion()` directly as a directory name may lead to invalid or problematic paths on some platforms (e.g. characters like `/`, `:`); consider normalizing/sanitizing the version string before using it in `refreshQmlCache`.
- The cache invalidation path logs with `qWarning()` on every version change, which is expected rather than exceptional behavior; consider downgrading this to `qInfo()` or adding more context so that real issues stand out in logs.
## Individual Comments
### Comment 1
<location> `src/dde-control-center/main.cpp:55-64` </location>
<code_context>
return path;
}
+static void refreshQmlCache(const QString &version)
+{
+ if (version.isEmpty())
+ return;
+ static const QByteArray envCachePath = qgetenv("QML_DISK_CACHE_PATH");
+
+ QString directory = envCachePath.isEmpty()
+ ? QStandardPaths::writableLocation(QStandardPaths::CacheLocation)
+ + QLatin1String("/qmlcache/")
+ : QString::fromLocal8Bit(envCachePath) + QLatin1String("/");
+
+ QDir dir(directory);
+ if (dir.exists()) {
+ if (dir.exists(version))
+ return;
+ const auto ret = dir.removeRecursively();
+ qWarning() << "Remove old QML cache:" << directory << "result:" << ret;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing the entire cache directory on version change may be overly aggressive and risky.
The logic deletes the entire cache root (e.g. `${CacheLocation}/qmlcache/` or `QML_DISK_CACHE_PATH`) whenever the specific `version` subdirectory is missing. If that path is shared between QML versions or apps, this can erase unrelated caches. Please either scope the cache directory more narrowly (e.g. per app) or delete only outdated version subdirectories instead of calling `removeRecursively()` on the root path.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, caixr23 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 |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, caixr23 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 |
Added QML cache management to handle version changes and prevent stale cache issues. The changes include: 1. Added refreshQmlCache() function to manage QML disk cache based on application version 2. Moved application version setting earlier in main() to enable cache management 3. The cache management function checks for existing cache directory and removes it if version mismatch is detected 4. Creates new cache directory with current version to ensure fresh QML compilation 5. Added QDir include for directory operations This change is necessary because QML cache can become stale when application version changes, leading to potential runtime issues. By proactively managing the cache directory based on version, we ensure that QML components are recompiled when needed. Log: Improved QML cache management to handle version changes automatically Influence: 1. Test application startup with different version strings 2. Verify QML cache directory creation and cleanup 3. Test with QML_DISK_CACHE_PATH environment variable set 4. Verify QML component loading after version change 5. Test cache persistence across application restarts 6. Monitor for any QML compilation performance changes feat: 主动管理 QML 缓存版本控制 添加了 QML 缓存管理功能以处理版本变更并防止缓存过期问题。变更包括: 1. 添加 refreshQmlCache() 函数来根据应用版本管理 QML 磁盘缓存 2. 将应用版本设置移到 main() 函数更早的位置以启用缓存管理 3. 缓存管理函数检查现有缓存目录,如果检测到版本不匹配则删除旧缓存 4. 创建带有当前版本的新缓存目录以确保 QML 重新编译 5. 添加 QDir 包含以支持目录操作 此变更是必要的,因为当应用版本变更时,QML 缓存可能变得过时,导致潜在的 运行时问题。通过基于版本主动管理缓存目录,我们确保在需要时重新编译 QML 组件。 Log: 改进 QML 缓存管理以自动处理版本变更 Influence: 1. 使用不同版本字符串测试应用启动 2. 验证 QML 缓存目录的创建和清理 3. 测试设置 QML_DISK_CACHE_PATH 环境变量的情况 4. 验证版本变更后的 QML 组件加载 5. 测试跨应用重启的缓存持久性 6. 监控 QML 编译性能变化
deepin pr auto review我来对这个diff进行代码审查:
改进建议:
static bool isValidVersion(const QString &version) {
// 检查版本字符串是否只包含数字和点
QRegularExpression re("^[0-9.]+$");
return re.match(version).hasMatch();
}
static void refreshQmlCache(const QString &version) {
if (version.isEmpty() || !isValidVersion(version))
return;
// 其余代码保持不变
}
QDir dir(directory);
if (dir.exists()) {
// 添加权限检查
if (!dir.isReadable()) {
qWarning() << "Cannot read cache directory:" << directory;
return;
}
// 其余代码保持不变
}
// 可以使用QtConcurrent::run来异步执行清理操作
QtConcurrent::run([directory, version] {
QDir dir(directory);
if (dir.exists()) {
const auto ret = dir.removeRecursively();
qDebug() << "Remove old QML cache:" << directory << "result:" << ret;
}
dir.mkpath(version);
});
if (!dir.mkpath(version)) {
qWarning() << "Failed to create cache directory:" << directory + version;
}这些改进可以提高代码的健壮性、安全性和性能。特别是版本字符串的检查和异步执行清理操作,这些改进对于生产环境来说是很重要的。 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Added QML cache management to handle version changes and prevent stale
cache issues. The changes include:
application version
management
removes it if version mismatch is detected
QML compilation
This change is necessary because QML cache can become stale when
application version changes, leading to potential runtime issues. By
proactively managing the cache directory based on version, we ensure
that QML components are recompiled when needed.
Log: Improved QML cache management to handle version changes
automatically
Influence:
feat: 主动管理 QML 缓存版本控制
添加了 QML 缓存管理功能以处理版本变更并防止缓存过期问题。变更包括:
此变更是必要的,因为当应用版本变更时,QML 缓存可能变得过时,导致潜在的
运行时问题。通过基于版本主动管理缓存目录,我们确保在需要时重新编译 QML
组件。
Log: 改进 QML 缓存管理以自动处理版本变更
Influence:
Summary by Sourcery
Add proactive QML disk cache management based on the application version during startup to avoid using stale QML cache after upgrades.
New Features:
Enhancements: