GNOME Bugzilla – Bug 662274
Failed to link personas: Can't link personas with no primary store.
Last modified: 2011-10-28 16:20:04 UTC
Hi, This bug has been reported downstream: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645950 https://bugs.launchpad.net/ubuntu/+source/empathy/+bug/847078 it seems that empathy cannot link contacts anymore: (empathy:30762): Gtk-CRITICAL **: gtk_tree_model_filter_real_unref_node: assertion `elt->ref_count > 0' failed (empathy:30762): empathy-WARNING **: Failed to link personas: Can't link personas with no primary store. Empathy: 3.2.0.1 folks: 0.6.3.2
Could you please get one of the original reporters to run Empathy with the following command and attach the resulting log file here? (You may want to get them to censor any personal details from the log, such as the names/addresses of their IM contacts.) FOLKS_DEBUG=folks empathy &> folks.log Thanks. (Basically the problem is that some configuration issue is causing folks to not choose a primary store, which is where linking information is kept. This should normally be EDS, but if that isn't working folks should fall back to a key file.)
Created attachment 199752 [details] Output of FOLKS_DEBUG=folks empathy I am facing this problem too. I'm attaching the output of empathy with FOLKS_DEBUG=folks
I need a bit more information, since there's not much in that log. 1. What's the value of your /system/folks/backends/primary_store GConf key? 2. Could you run Empathy under gdb as follows please? gdb empathy (Let Empathy start up, and then press Ctrl+C) set $folks_debug = folks_debug_dup() print folks_debug_emit_print_status($folks_debug) cont (Close Empathy) Could you then attach some of the output printed by folks_debug_emit_print_status() here please? Specifically, I need: • “Primary store”, “Configured store type id”, “Configured store id” from the IndividualAggregator section. • “Is primary store?” from the PersonaStores in the BackendStore section. Ideally I need all of the BackendStore section, with only minor details redacted, but I understand if you don't want to provide that. Thanks.
Philip: I'm one of the original reporters - I hope I can help with resolving this one. As for /system/folks/backends/primary_store - what is the proper way to retrieve such value? If `gconftool-2 -R /system/folks/backends/primary_store` then nothing. I checked with gconf-editor and I don't have /system/folks tree. There are /system/dns_sd /system/gstreamer or /system/pulseaudio (and four others) but no /system/folks As for the gdb'ing Empathy: Dumping status information… IndividualAggregator (0x799520) Ref. count: 2 Primary store: (nil) Linking enabled?: yes Prepared?: yes Quiescent?: yes and then 1015 individuals... Couldn't find "Configured store type id" nor "Configured store id". $ grep "Is primary store" /tmp/file Is primary store?: no Is primary store?: no Is primary store?: no Is primary store?: no Is primary store?: no Is primary store?: no Is primary store?: no Is primary store?: no If you need any other details - please let me know.
*** Bug 662570 has been marked as a duplicate of this bug. ***
(In reply to comment #4) > As for /system/folks/backends/primary_store - what is the proper way to > retrieve such value? If `gconftool-2 -R /system/folks/backends/primary_store` > then nothing. I checked with gconf-editor and I don't have /system/folks tree. > There are /system/dns_sd /system/gstreamer or /system/pulseaudio (and four > others) but no /system/folks The fact that it's not being found by gconftool-2 means that it isn't set (which is what I wanted to hear). (folks doesn't install a GConf schema, which is a bit naughty but harmless.) > As for the gdb'ing Empathy: > > Dumping status information… > IndividualAggregator (0x799520) > Ref. count: 2 > Primary store: (nil) > Linking enabled?: yes > Prepared?: yes > Quiescent?: yes > > and then 1015 individuals... Couldn't find "Configured store type id" nor > "Configured store id". It's odd that you can't find “Configured store type id” or “Configured store id”, since they should be just below “Primary store”. What version of folks do you have installed? > If you need any other details - please let me know. If you set the environment variable FOLKS_PRIMARY_STORE=key-file, do things start working?
(In reply to comment #6) [...] > It's odd that you can't find “Configured store type id” or “Configured store > id”, since they should be just below “Primary store”. What version of folks do > you have installed? I can try gdb again, but I'm pretty sure I hadn't those two. libfolks25 version is 0.6.3.2 > > If you need any other details - please let me know. > > If you set the environment variable FOLKS_PRIMARY_STORE=key-file, do things > start working? Yeah. ;-) I able to link contacts when empathy is started with this environment variable.
And yes. I don't have those two: $ grep "Configured store id" /tmp/empathy || echo "Not found" Not found $ grep "Configured store id" /tmp/empathy || echo "Not found" Not found (/tmp/empathy is the gdb logfile which includes this print statement)
(In reply to comment #8) > And yes. I don't have those two: That's because they were added (just) after 0.6.3.2 was released (sorry). In that case, could you run Empathy under gdb again please? gdb empathy set pagination off break _folks_individual_aggregator_set_primary_store commands bt print store print folks_persona_store_get_type_id(store) print folks_persona_store_get_id(store) print folks_persona_store_get_is_user_set_default(store) print self->priv->_configured_primary_store_type_id print self->priv->_configured_primary_store_id print self->priv->_user_configured_primary_store print self->priv->_primary_store print folks_persona_store_get_type_id(self->priv->_primary_store) print folks_persona_store_get_id(self->priv->_primary_store) cont end break _folks_individual_aggregator_backend_persona_store_removed_cb commands bt print store print folks_persona_store_get_type_id(store) print folks_persona_store_get_id(store) print folks_persona_store_get_is_user_set_default(store) print self->priv->_configured_primary_store_type_id print self->priv->_configured_primary_store_id print self->priv->_user_configured_primary_store print self->priv->_primary_store print folks_persona_store_get_type_id(self->priv->_primary_store) print folks_persona_store_get_id(self->priv->_primary_store) cont end run I need the entirety of the resulting output, please. None of it should be sensitive. Thanks.
(In reply to comment #9) [...] > That's because they were added (just) after 0.6.3.2 was released (sorry). ;-) no need to be sorry. > In that case, could you run Empathy under gdb again please? Sure thing.
Created attachment 199860 [details] gdb empathy
At what point did you interrupt and kill the process? Was Empathy fully loaded and connected to all your IM accounts? _folks_individual_aggregator_set_primary_store() should be being called several times in that trace, but it isn't. (It should be called at least once for each of the PersonaStores in use by folks — and you have several PersonaStores according to comment #4.)
Yes. Empathy was fully loaded and connected to all my IM accounts. I even started a chat to be sure it's connected. Does it matter that when creating a break(point) gdb says: Function "_folks_individual_aggregator_backend_persona_store_removed_cb" not defined. ? Or maybe should I run Empathy with FOLKS_PRIMARY_STORE environment variable?
(In reply to comment #13) > Yes. Empathy was fully loaded and connected to all my IM accounts. I even > started a chat to be sure it's connected. That's good. > Does it matter that when creating a break(point) gdb says: > Function "_folks_individual_aggregator_backend_persona_store_removed_cb" not > defined. ? No, as long as you answered with ‘y’ (which it looks like you did). > Or maybe should I run Empathy with FOLKS_PRIMARY_STORE environment variable? No, that would undermine the test. Could you try again with the following (different) commands please? gdb empathy set pagination off break _folks_individual_aggregator_backend_persona_store_added_cb commands bt print store print folks_persona_store_get_type_id(store) print folks_persona_store_get_id(store) print folks_persona_store_get_is_user_set_default(store) print self->priv->_configured_primary_store_type_id print self->priv->_configured_primary_store_id print self->priv->_user_configured_primary_store print self->priv->_primary_store print folks_persona_store_get_type_id(self->priv->_primary_store) print folks_persona_store_get_id(self->priv->_primary_store) cont end run Thanks.
Created attachment 199972 [details] Empathy GDB session (comment #14) (In reply to comment #14) [...] > Could you try again with the following (different) commands please? Of course. But I suppose that this session won't help as well... I also installed empathy debug symbols afterwards but the output still doesn't include anything relevant (there are only "New Thread (...)" and "Thread (...) exited" entries). Kind regards,
Created attachment 199973 [details] Empathy GDB session (good one) Ok. Disregard that. I suck at debugging. I've installed libfolks-telepathy25-dbg and libfolks-dbg and have something. I've redacted one jabber account, but the rest is public so there was only one minor edit in this file. Hope this helps.
(In reply to comment #16) > Created an attachment (id=199973) [details] > Empathy GDB session (good one) > > Ok. Disregard that. I suck at debugging. I've installed > libfolks-telepathy25-dbg and libfolks-dbg and have something. > > I've redacted one jabber account, but the rest is public so there was only one > minor edit in this file. > > Hope this helps. It looks like your primary store is showing up as "eds:system" (which would be causing the trouble).: $15 = (gchar *) 0x19a7f20 "eds" $16 = (gchar *) 0x19a7f40 "system" We fixed this in bug #660140, so please upgrade libfolks, libfolks-eds, etc., to 0.6.4 or later and try again.
(In reply to comment #17) > We fixed this in bug #660140, so please upgrade libfolks, libfolks-eds, etc., > to 0.6.4 or later and try again. I don't think that's the case, since Debian builds with --disable-eds-backend [1]. After some deeper investigation, I think I've found the problem. We have a #if to decide the default value for _configured_primary_store_type_id in IndividualAggregator. This is fine (if a little ugly) when building with valac available (as on our systems, which is why I didn't spot this before). However, if valac isn't available when compiling, the user gets stuck with eds:system, since that's what was compiled into individual-aggregator.c by whoever rolled the tarball. I'm guessing this is (somehow) what's happened with the Debian package. Probing the libfolks.so in the package[2] with `strings` shows no instances of “key-file”, but does show an instance of “system”. It doesn't find any instances of “eds”, but I guess that might be because it's only 3 characters long. I guess the solution is to remove the #if from IndividualAggregator; but a short-term fix would be to rebuild the Debian packages with --enable-eds-backend. A work around is to set FOLKS_PRIMARY_STORE=key-file on affected systems. Travis, does this all make sense? [1]: http://anonscm.debian.org/gitweb/?p=pkg-telepathy/folks.git;a=blob;f=debian/rules;hb=HEAD [2]: http://packages.debian.org/sid/libfolks25
FYI the required EDS version is not available in debian yet
(In reply to comment #18) > (In reply to comment #17) > > We fixed this in bug #660140, so please upgrade libfolks, libfolks-eds, etc., > > to 0.6.4 or later and try again. > > I don't think that's the case, since Debian builds with --disable-eds-backend > [1]. > > After some deeper investigation, I think I've found the problem. We have a #if > to decide the default value for _configured_primary_store_type_id in > IndividualAggregator. This is fine (if a little ugly) when building with valac > available (as on our systems, which is why I didn't spot this before). > > However, if valac isn't available when compiling, the user gets stuck with > eds:system, since that's what was compiled into individual-aggregator.c by > whoever rolled the tarball. I'm guessing this is (somehow) what's happened with > the Debian package. Yeah, that seems to be exactly it. It would be fine if Vala passed the #if down to the generated C file, but it doesn't (and there's no way to do that). See my patch for a work-around. > Probing the libfolks.so in the package[2] with `strings` shows no instances of > “key-file”, but does show an instance of “system”. It doesn't find any > instances of “eds”, but I guess that might be because it's only 3 characters > long. This is a result of the "key-file" branch of the #if-#else being compiled out in the Vala -> C step. > I guess the solution is to remove the #if from IndividualAggregator; but a > short-term fix would be to rebuild the Debian packages with > --enable-eds-backend. A work around is to set FOLKS_PRIMARY_STORE=key-file on > affected systems. > > Travis, does this all make sense? I believe my patch should resolve all these problems without needing to special-case anything. But it needs more testing -- Could any of the reporters please test out the forthcoming patch?
Created attachment 200070 [details] [review] Generate the same C code regardless of configure options Patches from branch: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo662274-linking
Created attachment 200071 [details] [review] Generate the same C code regardless of configure options (try 2) Patches from: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo662274-linking ============== These last two fixups make it work even if configure is run with --disable-eds-backend (which is the point...).
Review of attachment 200071 [details] [review]: This looks OK (apart from the one comment below), but I can't help thinking that perhaps we're taking the wrong approach. For example, this is still broken if the user compiles with --enable-eds-backend, but then the EDS backend can't be dynamically loaded (for whatever reason; perhaps because it's in a different distro package which isn't installed). I think it might be better to set the default _configured_primary_store_type_id by examining which backends are actually available and enabled in the BackendStore. Then again, that's a lot more work to cope with a situation which probably won't happen. ::: configure.ac @@ +83,3 @@ +else + AC_DEFINE(HAVE_TRACKER, [0], + [Define as 1 if you have the Tracker backend]) Isn't it more traditional to leave HAVE_* symbols *undefined* rather than defining them to be 0 in the negative case?
(In reply to comment #23) > Review of attachment 200071 [details] [review]: > > This looks OK (apart from the one comment below), but I can't help thinking > that perhaps we're taking the wrong approach. For example, this is still broken > if the user compiles with --enable-eds-backend, but then the EDS backend can't > be dynamically loaded (for whatever reason; perhaps because it's in a different > distro package which isn't installed). I think it might be better to set the > default _configured_primary_store_type_id by examining which backends are > actually available and enabled in the BackendStore. > > Then again, that's a lot more work to cope with a situation which probably > won't happen. We could hard-code a priority list (like [eds, tracker, key-file] or [eds, key-file]), but both of those run into ugly edge cases and hard-to-predict behavior (from the user's perspective). What we probably should do is fail early and/or warn loudly when the expected primary store is unavailable (and offer some clues as to what the user can do to fix it). > ::: configure.ac > @@ +83,3 @@ > +else > + AC_DEFINE(HAVE_TRACKER, [0], > + [Define as 1 if you have the Tracker backend]) > > Isn't it more traditional to leave HAVE_* symbols *undefined* rather than > defining them to be 0 in the negative case? Yes, it is - I hesitated on this yesterday. I'd be OK with changing their names if you can think of something better. The important part is that they have to be defined in both cases, or we'll get a cc build error.
(In reply to comment #24) > (In reply to comment #23) > > Review of attachment 200071 [details] [review] [details]: > > > > This looks OK (apart from the one comment below), but I can't help thinking > > that perhaps we're taking the wrong approach. For example, this is still broken > > if the user compiles with --enable-eds-backend, but then the EDS backend can't > > be dynamically loaded (for whatever reason; perhaps because it's in a different > > distro package which isn't installed). I think it might be better to set the > > default _configured_primary_store_type_id by examining which backends are > > actually available and enabled in the BackendStore. > > > > Then again, that's a lot more work to cope with a situation which probably > > won't happen. > > We could hard-code a priority list (like [eds, tracker, key-file] or [eds, > key-file]), but both of those run into ugly edge cases and hard-to-predict > behavior (from the user's perspective). Icky. I agree. > What we probably should do is fail early and/or warn loudly when the expected > primary store is unavailable (and offer some clues as to what the user can do > to fix it). I guess so, but that can be the subject of a different bug report. > > ::: configure.ac > > @@ +83,3 @@ > > +else > > + AC_DEFINE(HAVE_TRACKER, [0], > > + [Define as 1 if you have the Tracker backend]) > > > > Isn't it more traditional to leave HAVE_* symbols *undefined* rather than > > defining them to be 0 in the negative case? > > Yes, it is - I hesitated on this yesterday. I'd be OK with changing their names > if you can think of something better. > > The important part is that they have to be defined in both cases, or we'll get > a cc build error. Ah, right, I see why the [0] definition is necessary now. My bad. The patch looks good to me in light of these clarifications.
(In reply to comment #23) > Review of attachment 200071 [details] [review]: > > This looks OK (apart from the one comment below), but I can't help thinking > that perhaps we're taking the wrong approach. For example, this is still broken > if the user compiles with --enable-eds-backend, but then the EDS backend can't > be dynamically loaded (for whatever reason; perhaps because it's in a different > distro package which isn't installed). I think it might be better to set the > default _configured_primary_store_type_id by examining which backends are > actually available and enabled in the BackendStore. > > Then again, that's a lot more work to cope with a situation which probably > won't happen. After more thought, I leaned more toward this as well -- but the problem is that that information isn't available by the time we need it. I put together a partly-working proof-of-concept, but it's a little ugly and ultimately doesn't work (since it would make IndividualAggregator's constructor depend upon an async function that yields on the BackendStore being prepared). Ideally, we'd yield on backends_enabled being filled, but that's even later in the process. I think we'd need to do some fairly-hairy changes (including some behavior changes, at the least) to do this correctly. So, in the end, I think it's better to just use my first approach with extra warnings.
Created attachment 200151 [details] [review] Generate the same C code regardless of configure options (Add warning) Final patch - just adds an extra warning with steps to correct. Patches from branch: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo662274-linking
Merged (including an extra patch to fix our tests to use exactly the primary store they need so they don't fail on the new warning): To everyone who reported this bug: please re-open this if you still hit it with these fixes in place. commit b15f6e0efb4b0191e8f516609e11369afce08b3c Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed Oct 26 17:30:37 2011 -0700 Generate the same C code whether the eds backend is enabled or not. Previously, the configure options used would alter the release tarball. This would cause problems for anyone building from the shipped C files (specifically, if they also disabled the eds backend, as the Debian packagers did). Closes: bgo#662274 - Failed to link personas: Can't link personas with no primary store. NEWS | 2 ++ folks/individual-aggregator.vala | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) commit 7e20483ebc830438fc5d377e90e9017e4befead2 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu Oct 27 16:44:18 2011 -0700 Warn if the primary store is not found. This includes steps for the user to fix the problem. Helps: bgo#662274 - Failed to link personas: Can't link personas with no primary store. folks/individual-aggregator.vala | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-) commit f48cf529d06ced6bcb2b597ce1ff57602f886d38 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Fri Oct 28 08:08:54 2011 -0700 Properly set primary stores for tests to exactly what's used. Helps: bgo#662274 - Failed to link personas: Can't link personas with no primary store. tests/eds/add-contacts-stress-test.vala | 7 +++++-- tests/eds/add-persona.vala | 7 +++++-- tests/eds/avatar-details.vala | 7 +++++-- tests/eds/email-details.vala | 7 +++++-- tests/eds/im-details.vala | 7 +++++-- tests/eds/individual-retrieval.vala | 7 +++++-- tests/eds/name-details.vala | 7 +++++-- tests/eds/phone-details.vala | 7 +++++-- tests/eds/postal-address-details.vala | 7 +++++-- tests/eds/remove-persona.vala | 7 +++++-- tests/eds/removing-contacts.vala | 7 +++++-- tests/eds/set-avatar.vala | 7 +++++-- tests/eds/set-birthday.vala | 7 +++++-- tests/eds/set-emails.vala | 7 +++++-- tests/eds/set-gender.vala | 7 +++++-- tests/eds/set-im-addresses.vala | 7 +++++-- tests/eds/set-is-favourite.vala | 7 +++++-- tests/eds/set-names.vala | 7 +++++-- tests/eds/set-notes.vala | 7 +++++-- tests/eds/set-phones.vala | 7 +++++-- tests/eds/set-postal-addresses.vala | 7 +++++-- tests/eds/set-properties-race.vala | 7 +++++-- tests/eds/set-roles.vala | 7 +++++-- tests/eds/set-structured-name.vala | 7 +++++-- tests/eds/set-urls.vala | 7 +++++-- tests/eds/store-removed.vala | 7 +++++-- tests/eds/updating-contacts.vala | 7 +++++-- tests/key-file/Makefile.am | 1 + tests/libsocialweb/Makefile.am | 2 +- tests/libsocialweb/aggregation.vala | 3 +++ tests/telepathy/Makefile.am | 2 +- 31 files changed, 141 insertions(+), 56 deletions(-) commit 1432a353880ad1d1784b20f5edb90fc6179558cd Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Wed Oct 26 17:28:56 2011 -0700 Expose the details of which backends are enabled to libfolks. Helps: bgo#662274 - Failed to link personas: Can't link personas with no primary store. configure.ac | 20 +++++++++++++++++++- folks/build-conf.vapi | 9 +++++++++ 2 files changed, 28 insertions(+), 1 deletions(-)