Change conf path XDG_CACHE_HOME to XDG_STATE_HOME #9755
Conversation
Keepassxc saves application state at XDG_CACHE_HOME which can be cleared on some systems periodicly. This is not desireable as app state like window size is not consistent when openning the app. To avoid this this commit is switching the path to XDG_STATE_HOME which is more fitting based on the freedesktop basedir spec (https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html), this will allow to prevent state file deletion as well. Solves keepassxreboot#9738
jNullj
force-pushed
the
feat/switch-to-XDG_STATE_HOME-9738
branch
from
9845079
to
c5ca0bb
Compare
src/core/Config.cpp
Outdated
| QString oldLocalConfigPath; | ||
| oldLocalConfigPath = QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation) + "/keepassxc"; | ||
| QString suffix; | ||
| #ifdef QT_DEBUG | ||
| suffix = "_debug"; | ||
| #endif | ||
| oldLocalConfigPath += QString("/keepassxc%1.ini").arg(suffix); | ||
| oldLocalConfigPath = QDir::toNativeSeparators(oldLocalConfigPath); | ||
| if (!localConfigFileName.isEmpty() && !QFile::exists(localConfigFileName) && QFile::exists(oldLocalConfigPath)) { |
There was a problem hiding this comment.
I don't think you should check for localConfigFileName being empty. That's just something that shouldn't happen and would be an error elsewhere. I think you should just check if it exists and if not, then check the old location. The definition of oldLocalConfigPath can be moved into the smaller scope of the corresponding if statement.
There was a problem hiding this comment.
I missed that, the isEmpty there is not relevant, removed it with 2eecf98
I also understand that this is not optimal to generate oldLocalConfigPath on every startup, i changed the conditions and moved it inside the first if statement, we will only generated if the config is missing, fixed in d77ef9d
src/core/Config.cpp
Outdated
| QString xdgStateHome; | ||
| xdgStateHome = QFile::decodeName(qgetenv("XDG_STATE_HOME")); |
There was a problem hiding this comment.
My bad, fixed with 0cd5ce4
Only generate oldLocalConfigPath if the local config is missing and the old config path should be checked. Before this commit oldLocalConfigPath would always get generated, while it won't be needed most of the time.
src/core/Config.cpp
Outdated
| QString oldLocalConfigPath; | ||
| oldLocalConfigPath = QStandardPaths::writableLocation(QStandardPaths::GenericCacheLocation) + "/keepassxc"; |
There was a problem hiding this comment.
You should not separate declaration/definition and assignment. Rest looks good.
|
Looks good now. Thanks for your patience. |
Keepassxc saves application state at XDG_CACHE_HOME which can be cleared on some systems periodicly. This is not desireable as app state like window size is not consistent when openning the app. To avoid this this commit is switching the path to XDG_STATE_HOME which is more fitting based on the freedesktop basedir spec (https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html), this will allow to prevent state file deletion as well. Resolves keepassxreboot#9738
Keepassxc saves application state at XDG_CACHE_HOME which can be cleared on some systems periodicly.
This is not desireable as app state like window size is not consistent when openning the app.
To avoid this, this PR commit is switching the path to XDG_STATE_HOME which is more fitting based on the freedesktop basedir spec, this will allow to prevent state file deletion as well.
Changes made based on how qt implements QStandardPaths (very similar to internal qt code)
Also added code to migrate old config files to new location and cleanup files from old location.
Fixes #9738
Screenshots
N/A
Testing strategy
Manually tested file changes in
~/.local/stateand that migration from~/.cacheworks.Type of change