KWallet plugin logic implementation inside frameworks

[David Faure]

Hello Valentin,

Sorry for the delayed reply, too many conferences this month ;)

On Sunday 04 November 2012 16:03:12 Valentin Rusu wrote:
> Hello,
> 
> Now that I'm using a plug-in version of the KWallet API on my system, I
> think it would be the time to put it at the right place inside the
> frameworks branch, if you don't mind.

Sounds good.

> The logic actually lives inside kdelibs/ksecretsservice branch and,
> despite that branch's name, the only changes it brings to kdelibs for
> the moment is the kwallet plugin logic.

Ah, OK. Indeed, I wouldn't have guessed :)

> KWallet class actually lives inside kdeui. I modified it to load a
> plugin and delegate all the method calls to a plugin it loads when first
> called. I also added a new header file intended for plugin implementers,
> declaring the needed methods. See also [1]

My first reaction was, do we really need plugins, if secret-service is the way 
forward. But indeed on Windows and Mac OS, I presume another backend will be 
necessary, so this is probably a good design for this issue. A bit like QPA, 
in fact :)

> First question (and proposed answer) : which tier should KWallet class
> live in?

staging, for now :-)

> I already think it's a "functional" kind of class and because it needs
> plugin loading

Beep, incompatibility here. The type "functional" means "it's only a library".
Plugins are runtime dependencies, so this is an "integration" framework 
(confirmed by the fact that it aims at integrating with the OS wallet service).
But anyway, type classification is only to make Kévin happy, for now, it's not 
used anywhere yet.

> , it should go to tier3.

Possible. Depends on its dependencies, really.
Make the framework first, in staging, then we can look at its dependencies.

> When grepping for kwallet includes inside frameworks branch, I find that
> ./kdewebkit/kwebwallet.cpp:#include <kwallet.h>
> ./kdeui/tests/kwallettest.cpp:#include <kwallet.h>
> ./kdeui/util/kwallet.cpp:#include "kwallet.h"
> ./kdeui/util/kwallet.cpp:#include "config-kwallet.h"
> ./kdeui/util/kwallet_mac.cpp:#include "kwallet.h"
> ./khtml/html/html_formimpl.cpp:#include <kwallet.h>
> ./kio/kssl/ksslkeygen.cpp:#include <kwallet.h>
> ./plasma/remote/authorizationmanager.cpp:#include <kwallet.h>
> 
> All these classes, if I understand correctly, will also live in tier3,
> so the dependency rule will not be broken [2].

Probably right.

> So I propose to:
> - move KWallet API inside the tier3/kwallet to be created library
> - update the dependencies to reflect that

Make it staging/kwallet for now.
And make sure to respect the directory structure described at 
http://community.kde.org/Frameworks/Policies#Framework_directory_structure

> Second issue (and proposal): where should go the kwalletdefaultplugin?
> 
> This plugin actually contains the old KWallet API implementation, the
> one which calls the kwalletd.
> I think the best place for kwalletdefaultplugin should be
> kde-runtime/kwalletd. This way, this plugin will get installed at the
> same time as the daemon it communicates with.

In a different subdirectory of the same framework, as Jon said.

If the plugin doesn't exist in 4.x, then there's no merging issue, though, you 
can put it in the framework already.
He's right about the existing kde-runtime code though, it would be best not to 
touch that one yet. I guess this might make it difficult for you to test the 
whole thing, though... Unless the daemon is mostly unchanged, for now?

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5