Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move local config from ~/.cache to ~/.local/share on Linux #5325

Closed
wants to merge 2 commits into from
Closed

Move local config from ~/.cache to ~/.local/share on Linux #5325

wants to merge 2 commits into from

Conversation

z3ntu
Copy link
Contributor

@z3ntu z3ntu commented Aug 23, 2020

Some users have ~/.cache mounted on tmpfs which meant that the
'Recent Databases' were lost after every reboot. Move that config to
~/.local/share which is meant for this kind of data.

Fixes #4911
Fixes #5313

Testing strategy

~/.local/share/keepassxc/keepassxc.ini gets created now and not ~/.cache/keepassxc/keepassxc.ini

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

I wasn't sure which branch to target with this PR, it's currently targeting develop, #5313 has the milestone v2.6.2

Some users have ~/.cache mounted on tmpfs which meant that the
'Recent Databases' were lost after every reboot. Move that config to
~/.local/share which is meant for this kind of data.
@phoerious
Copy link
Member

Cache is just the wrong place to have on a tmpfs.

@droidmonkey
Copy link
Member

You should move a config file currently in cache to the new location.

@phoerious
Copy link
Member

phoerious commented Aug 23, 2020

I am actually again merging this. Putting .cache on a tmpfs is like deleting your cookies when you close the browser and then complaining about having to re-login on every website.

@droidmonkey
Copy link
Member

droidmonkey commented Aug 23, 2020

I can't disagree with that, but others have rightly stated there are no other config files being stored in .cache. perhaps at the very least we can detect if .cache is on tmpfs and use .local instead.

@z3ntu
Copy link
Contributor Author

z3ntu commented Aug 23, 2020

You should move a config file currently in cache to the new location.

On a side note, the migration from 2.5.4 to (e.g.) 2.6.1 didn't work either (excerpt from ~/.cache/keepassxc/keepassxc.ini after migrating):

[General]
LastOpenedDatabases=@Invalid()

I'll see what I can do so at least the stable versions don't get their state reset.

@phoerious
Copy link
Member

I can't disagree with that, but others have rightly stated there are no other config files being stored in .cache.

The things we store there are technically not config options.

@z3ntu
Copy link
Contributor Author

z3ntu commented Aug 26, 2020

You should move a config file currently in cache to the new location.

I've looked into this a bit but I'm not sure how to properly do that. Is it okay if I just move the config file at the start of the migrate() function or will this break things? I'm not familiar with the code base so a few pointers would be quite helpful :)

@droidmonkey
Copy link
Member

Bumping the config version and putting it in migrate is a good idea.

@phoerious
Copy link
Member

phoerious commented Aug 27, 2020

~/.local/share/keepassxc/keepassxc.ini is totally the wrong location and makes the whole split redundant. You might as well save the local config to ~/.config/keepassxc/keepassxc.ini then.

@droidmonkey
Copy link
Member

Well then how about keepassxc_local.ini on Linux.

@z3ntu
Copy link
Contributor Author

z3ntu commented Aug 27, 2020

@phoerious What's wrong with ~/.local/share ? That's the location for non-config user-specific application data.

@kotoko
Copy link

kotoko commented Aug 27, 2020

Let me reply to @phoerious . I don't want to fight you but only share my perspective.

Cache is just the wrong place to have on a tmpfs.

IMHO it's wrong assumption that ~/.cache directory won't be deleted on logout or reboot. The same situation as /tmp. I'm not sure what correct directory is but I'm sure that deleting ~/.cache should not change application settings or history.

I am actually again merging this. Putting .cache on a tmpfs is like deleting your cookies when you close the browser and then complaining about having to re-login on every website.

Web browsers have own cache also so I would say that deleting .cache is the same as deleting web browsers' cache. That triggers redownload of all images and open tabs but does not delete open tabs or cookies.

@phoerious
Copy link
Member

phoerious commented Aug 27, 2020

.cache is for "non-essential" (i.e. disposable) user data and it is supposed to store a lot of it such as image thumbnails. Putting that on a tmpfs is the wrong thing to do for several reasons, even though the Debian project may see that differently. The XDG spec says nothing about it being "volatile", contrary to XDG_RUNTIME_DIR.

In fact, I would even argue that losing your KeePassXC local config when deleting the contents of .cache is exactly desired behaviour. Similar to Windows's disk cleanup feature, you would want things like your KeePassXC file history to be deleted when you decide to clear .cache.

@kotoko
Copy link

kotoko commented Aug 27, 2020

It looks like XDG spec says almost nothing about $XDG_CACHE_HOME which is kind of sad. So assuming that it will be purged rarely/frequently is wrong. But it can be purged at some point. Interesting...

I guess my last question would be: is file history in kepassxc really "non-essential"?

@phoerious
Copy link
Member

I see it very much like cookies. Yes, it is essential for most in most circumstances, but it is disposable and for certain users it is absolutely crucial that it is so.

@z3ntu
Copy link
Contributor Author

z3ntu commented Aug 28, 2020

Cookies aren't stored in cache - they are still there when you delete the cache. I consider cache as something e.g. when you have a big complicated file where you need to extract certain information which takes long - then you put the result into the cache and you read from cache next time so it's faster. If the cache is deleted it doesn't matter at all because it will just read the original file again and regenerate the cache.
That's what I consider a cache.
And a "last used files" list is not that.

@phoerious
Copy link
Member

Not true. When you use the disk cleanup feature on Windows, you can select to clean up cookies as well next to any other disposable, temporary, or volatile data.

@z3ntu
Copy link
Contributor Author

z3ntu commented Aug 28, 2020

I don't think we will get anywhere with this discussion.
You say that the recently used list belongs into ~/.cache and I disagree.
I say the recently used list belongs into ~/.local/share and you disagree.
I don't see either side changing their opinion anytime soon.

But as 6 people (including me) have complained in either #4911 or #5313 that having the list on ~/.cache clears the list when they don't want to I hope you reconsider and allow the list to be put into ~/.local/share (or anywhere away from ~/.cache).

@droidmonkey
Copy link
Member

droidmonkey commented Aug 28, 2020

I'm in favor of an ENV VAR solution... KPXC_LOCAL_CONFIG=[directory]. Absent this ENV VAR it defaults to .cache. Everyone wins!

@droidmonkey droidmonkey added this to the v2.6.2 milestone Aug 28, 2020
@Kieltux
Copy link

Kieltux commented Sep 6, 2020

Source: https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html
"There is a single base directory relative to which user-specific configuration files should be written. This directory is defined by the environment variable $XDG_CONFIG_HOME."
An ini file (like keepassxc.ini) is in my opinion a user-specific configuration file, so it should be written to ~/.config.

"There is a single base directory relative to which user-specific non-essential (cached) data should be written. This directory is defined by the environment variable $XDG_CACHE_HOME."
I empty the ~/.cache directory from time to time. It should be safe to empty the ~/.cache directory without changing anything (like empty tmp directories).

@phoerious
Copy link
Member

Iit is safe. You di not lose any essential settings. And only because the format is ini doesn't make it a config file. Ini is just a generic key-value format.

@z3ntu z3ntu changed the title Move local config away from ~/.cache on Linux Move local config from ~/.cache to ~/.local/share on Linux Sep 6, 2020
@droidmonkey
Copy link
Member

droidmonkey commented Sep 6, 2020

I will only merge this if it uses the env var solution i mentioned above. No need to move the config file in that case. Documentation needs to be updated to point out the env var as well.

@z3ntu
Copy link
Contributor Author

z3ntu commented Sep 17, 2020

Sorry, I'm not willing to implement a solution which uses environment variables to achieve this. In my opinion (and I've talked with a couple of other people about this) the list of recently used files definitely doesn't belong into ~/.cache but into ~/.local/share (I agree that ~/.config was incorrect but ~/.cache is way worse). If you're not willing to accept the PR in its current form please close it.

@droidmonkey
Copy link
Member

Suit yourself, I'll do it myself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local config data is cleared when ~/.cache mounted on tempfs Does not open last used database automatically
5 participants