After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679870 - libsecret migrations
libsecret migrations
Status: RESOLVED FIXED
Product: tracker
Classification: Core
Component: Miners
unspecified
Other Linux
: Normal normal
: ---
Assigned To: tracker-general
Jamie McCracken
Depends on:
Blocks: 679893
 
 
Reported: 2012-07-13 15:30 UTC by Stef Walter
Modified: 2013-01-03 17:34 UTC
See Also:
GNOME target: 3.8
GNOME version: ---


Attachments
WIP patch to migrate to libsecret (9.67 KB, patch)
2012-07-13 15:30 UTC, Stef Walter
none Details | Review
WIP patch to migrate to libsecret (9.67 KB, patch)
2012-07-14 06:53 UTC, Stef Walter
needs-work Details | Review

Description Stef Walter 2012-07-13 15:30:39 UTC
Created attachment 218741 [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.

libsecret solves many problems with libgnome-keyring. Relevant to tracker: it
solves threading issues, uses GDBus instead of dbus-glib.

A future GNOME goal will be to migrate away from libgnome-keyring to libsecret:

https://live.gnome.org/GnomeGoals/LibsecretMigration

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. 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 for the stored items, you may want to
   change it to something better:
     org.gnome.Tracker.Miner
 * I haven't tested the patch. I'm not familiar with how to trigger all the
   various code paths and logic.
 * secret_password_remove_sync() removes all matching unlocked items as
   opposed the way that gnome_keyring_find_items_sync() +
   gnome_keyring_item_delete_sync() were previously used to unlock all the
   items to delete them. This is a corner case, but if you want the previous
   behavior, you can use secret_service_search() and secret_item_delete() to
   make that happen.

Note that the patch uses the unstable 'advanced' parts of the libsecret API.

In particular, it's not "best practice" to use gnome-keyring as a place to
store account or connection details. In an ideal world gnome-keyring would be
used just to store secrets, and attributes are used to lookup those secrets.
However libsecret supports this use case, but that functionality is not yet
absolutely stable.

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 tracker for any API
changes that come up.
Comment 1 Olav Vitters 2012-07-13 20:42:31 UTC
Per r-t: Targetting GNOME 3.6
Comment 2 Stef Walter 2012-07-14 06:53:55 UTC
Created attachment 218794 [details] [review]
WIP patch to migrate to libsecret

Updated patch for last minute API change
Comment 3 Stef Walter 2012-07-14 06:54:39 UTC
The above comment should say:

 * secret_password_clear_sync() removes all matching unlocked items as
   opposed the way that gnome_keyring_find_items_sync() +
   gnome_keyring_item_delete_sync() were previously used to unlock all the
   items to delete them. This is a corner case, but if you want the previous
   behavior, you can use secret_service_search() and secret_item_delete() to
   make that happen.
Comment 4 Stef Walter 2012-07-14 11:20:09 UTC
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.
Comment 5 Martyn Russell 2012-08-10 14:04:22 UTC
Comment on attachment 218794 [details] [review]
WIP patch to migrate to libsecret

Hi Stef, my comments below:

>@@ -173,23 +174,21 @@ password_provider_gnome_store (TrackerPasswordProvider  *provider,
>                                const gchar              *password,
>                                GError                  **error)
> {
>-	GnomeKeyringResult result;
>+	GError *secret_error = NULL;
> 
>-	result = gnome_keyring_store_password_sync (&password_schema,
>-	                                            NULL,
>-	                                            description,
>-	                                            password,
>-	                                            "service", service,
>-	                                            "username", username,
>-	                                            NULL);
>+	secret_password_store_sync (&password_schema, NULL, description,
>+	                            password, NULL, &secret_error,
>+	                            "service", service,
>+	                            "username", username,
>+	                            NULL);

My only request is that we switch to async APIs if possible. I know we didn't before, but that's something we would prefer. I don't remember if it's not possible to do without looking at the code a bit further, but if we can we should.

Thanks again for an initial patch here.

Also, do you happen to know when libsecret will be available to distros? I don't see it here in Ubuntu 12.04 yet. We like to depends on packages that are available in Ubuntu and RedHat usually before adopting them. We have other patches waiting for new versions in Bugzilla too for the same reason.

Thanks,
Comment 6 Stef Walter 2012-08-10 14:07:53 UTC
(In reply to comment #5)
> My only request is that we switch to async APIs if possible. I know we didn't
> before, but that's something we would prefer. I don't remember if it's not
> possible to do without looking at the code a bit further, but if we can we
> should.

Async is always good when it works out. I hope async works out for tracker. There's nothing fun about handing control the beating heart (main loop) of your application to another process, especially one that prompts. Can lead to some spectacular deadlocks.

> Also, do you happen to know when libsecret will be available to distros? I
> don't see it here in Ubuntu 12.04 yet. We like to depends on packages that are
> available in Ubuntu and RedHat usually before adopting them. We have other
> patches waiting for new versions in Bugzilla too for the same reason.

Fedora has it in rawhide. The Ubuntu packagers have been on my case about libsecret, so I think they are packaging it for 12.10.
Comment 7 Matthias Clasen 2012-09-17 19:48:26 UTC
Moving off the 3.6 blocker list
Comment 8 Matthias Clasen 2013-01-02 04:26:41 UTC
any update ? would be nice to have for gnome 3.8
Comment 9 Martyn Russell 2013-01-03 17:34:57 UTC
(In reply to comment #8)
> any update ? would be nice to have for gnome 3.8

Sorry for the late response here Matthias, I was planning on committing this to master and releasing a 0.15.1 with it soon.

I've committed it now to master. It needs a bit more testing. Everything builds, but I need to do some flickr miner checks this end. JHBuild should at least have this now.

AFAICS, if anything goes wrong from here, it's the flickr miner's bug, not libsecret migration. So will mark this as fixed.

Thanks for the patches and work everyone.