GNOME Bugzilla – Bug 679851
libsecret migration
Last modified: 2013-03-04 18:05:29 UTC
Created attachment 218712 [details] [review] WIP patch to migrate to libsecret libsecret is a new client for the Secret Service DBus API. The Secret Service allows storage of passwords in a common way on the desktop. Supported by gnome-keyring and ksecretservice. It solves many problems with libgnome-keyring. Relevant to gnome-shell: it solves threading issues, uses GDBus instead of dbus-glib, and makes cancellation much cleaner. A future GNOME goal will be to migrate away from libgnome-keyring to libsecret: https://live.gnome.org/GnomeGoals/LibsecretMigration The use of libgnome-keyring in gnome-shell was in the network manager agent. I've done a rough WIP patch in order to make sure that the libsecret API covered all the use cases. I'll attach that patch here. You'll probably want to wait for bug #679849 before doing this, so that both nm-agents can use the same schema name for the stored passwords. Note that libsecret can read passwords stored via libgnome-keyring, so this isn't a hard dependency. I hope the patch is a help for the migration, but I don't plan to iterate on it at the current time. Some notes about the patch: * I chose an arbitrary schema name, probably want to sync this up with network-manager-applet. org.freedesktop.NetworkManager.Connection * I haven't tested the patch. I'm not familiar with how to trigger all the various code paths and logic. Note that the patch uses the unstable 'advanced' parts of the libsecret API. See bug #679849 for why. I'm aiming to get most of this stable by GNOME 3.8, but if you do migrate to libsecret before then, I would patch network-manager-applet for any API changes that come up.
Per r-t: Targetting GNOME 3.6
Created attachment 218791 [details] [review] WIP patch to migrate to libsecret Updated for last minute API change.
BTW, just a heads up: Please look at the patch critically. I did the patch as a way to try out the API, and it's not ready to commit. There may be memory leaks or other logic errors. Most libsecret getters return data that must be unreferenced or freed.
Just want to point out that a large part of shell-network-agent.c was stolen from http://git.gnome.org/browse/network-manager-applet/tree/src/applet-agent.c, so you should probably update that at the same time.
Moving off the 3.6 blocker list
Would be nice to get this done for 3.8
Review of attachment 218791 [details] [review]: Looks OK, one minor nit. ::: src/shell-network-agent.c @@ +278,3 @@ + { + g_error_free (secret_error); + return; Should we not remove it from the requests HT? Seems to be an existing error, but still...
if it is an existing error, should we file a separate bug about it, and get this one out of the way ?
Created attachment 237985 [details] [review] Migrate from libgnome-keyring to libsecret * See: https://live.gnome.org/GnomeGoals/LibsecretMigration https://bugzilla.gnome.org/show_bug.cgi?id=679851
Rebased on master... (In reply to comment #7) > Should we not remove it from the requests HT? Seems to be an existing error, > but still... It's cancelled because it has been removed from the hash table already. See the g_cancellable_cancel() call in shell_agent_request_free(), which is only called as a GDestroyNotify for the hash table.
Review of attachment 237985 [details] [review]: LGTM
Pushed to git master. Attachment 237985 [details] pushed as dac54a6 - Migrate from libgnome-keyring to libsecret