Prefer kwallet over libsecret for KDE #111
Conversation
keychain_unix.cpp
Outdated
| @@ -91,13 +99,18 @@ static KeyringBackend detectKeyringBackend() | |||
| case DesktopEnv_Plasma5: | |||
| return Backend_Kwallet5; | |||
| break; | |||
| // fall through | |||
|
|
|||
| // fall through | |||
There was a problem hiding this comment.
(nitpick: fall through what? there is already one return AND a redundent break :-) )
There was a problem hiding this comment.
Yes. I didn't add it.
keychain_unix.cpp
Outdated
| @@ -91,13 +99,18 @@ static KeyringBackend detectKeyringBackend() | |||
| case DesktopEnv_Plasma5: | |||
| return Backend_Kwallet5; | |||
There was a problem hiding this comment.
Could we check somehow if kwallet5 is running. And if it is not, then we'd fallback to libsecret?
|
@ogoffart Updated. But with this addition the risk of the change also increased since my kwallet detection code might be wrong. |
ckamm
force-pushed
the
prefer-kwallet-over-libsecret
branch
from
4dfde7a to
9bd44f0
Compare
|
Looks good to me. I wonder if KWallet4 is still the best fallback for non-KDE, but that's somewhat unrelated to this patch. |
|
@frankosterfeld So we merge this, adjust the changelog, bump the version to 0.9.0, bump the version for master? |
keychain_unix.cpp
Outdated
| return Backend_LibSecretKeyring; | ||
| } | ||
|
|
||
| if ( GnomeKeyring::isAvailable() ) { | ||
| return Backend_GnomeKeyring; | ||
| } else { | ||
| return Backend_Kwallet4; |
There was a problem hiding this comment.
Please change to Backend_Kwallet5
So this would cover the use case that we are running Plasma 5, but neither isKwallet5Available() nor LibSecretKeyring::isAvailable() (because the application just started at startup before kwallet5d is running)
There was a problem hiding this comment.
Right, that might happen during startup. So:
Plasma: Try kwallet5, try libsecret, try gnome-keyring, fallback kwallet5
Gnome and others: Try libsecret, try gnome-keyring, try kwallet5, fallback gnome-keyring
?
In particular, fallback onto gnome-keyring on gnome/unity and on kwallet5 on plasma sessions.
|
@ogoffart Updated |
|
@frankosterfeld Hi! You would still need to :
thanks :) |
|
@frankosterfeld Sorry, didnt see that. Feel free to wait a bit more if needed. |
|
Ok, I wait a few more days. Please ping me if I didn't release by the end of the week ;) |
|
@frankosterfeld from our side, it would be OK to have the release. Waiting for #103 is not important.. :) |
|
@guruz done! |
|
@frankosterfeld Thanks a ton :) |
See discussion in #99 #103 and owncloud/client#6455