GNOME Bugzilla – Bug 629084
Add a folks-import tool
Last modified: 2010-09-11 00:02:41 UTC
This would be a simple tool to allow migration of meta-contact data from one format to anther. Initially, it would only support copying meta-contact data, aliases and groups from Pidgin's blist.xml to the key-file backend's relationships.ini; but in future, more formats could be supported. For 0.1.x, the tool will be stand-alone, but once bug #628868 is fixed, it could be rebased to use libfolks' Pidgin backend.
Created attachment 169791 [details] [review] Add a folks-import tool http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629084-folks-import I haven't tested it especially thoroughly; I'll do that tomorrow. Apart from that, it's ready for review.
Created attachment 169877 [details] [review] Add a folks-import tool (updated) http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629084-folks-import-attempt2 Testing it revealed that Kf.PersonaStore wasn't flushing changes to disk properly if the main loop was exited too soon after the changes were made. The first commit in this branch rectifies that (and also stops I/O operations piling up on top of each other) by manually iterating the main loop until any pending operation is finished.
Note that I haven't taken advantage of the flush() method anywhere inside libfolks (e.g. in Backend or BackendStore), as I think this requires a bit more thought and probably some API breaking. Empathy's fine without such changes for this cycle.
Review of attachment 169877 [details] [review]: As discussed on IRC, you need a delete statement for the XMLDoc*. Also, import() should throw an error instead of returning 0 in the case that document parsing fails, so its caller can easily separate errors from empty buddy lists. blist_protocol_to_tp_protocol() should be moved after its first dependent function, to match the convention we're using in the rest of the Vala code. + success = ImportTool.import.end (r); * If the destination relationships.ini is missing, folks-import just hangs. * folks-import spews a ton of .goutputstream-* files into ~/.local/share/folks * if the alias == ID, don't bother setting the alias * I've got a <contact> with several entries from one account. One of these IDs is also on another account (and also shows up in a separate <contact>). For some reason, the final entry in relationships.ini gives me the set difference (missing this duplicate entirely) instead of union. The final relationships.ini doesn't end with a newline. This isn't a huge deal, but we might want to just ensure that it does, to make appending less problematic. This isn't a blocker for merging just now, but it'd be nice if you could identify and notify Permission Denied errors when failing due to that. Another non-blocker enhancement would be to filter incoming duplicates. I've got several metacontacts with entries like "aim=foo;foo". It's not clear whether it's because I've got "foo" from two different accounts in the same <contact> entry or because I've got another <contact> entry in another place containing that same ID (from one of the two accounts mentioned before). Other than that, it seems to work well.
(In reply to comment #4) > Review of attachment 169877 [details] [review]: > > As discussed on IRC, you need a delete statement for the XMLDoc*. Also, > import() should throw an error instead of returning 0 in the case that document > parsing fails, so its caller can easily separate errors from empty buddy lists. Done locally; it'll appear in an updated patch if I do one. > blist_protocol_to_tp_protocol() should be moved after its first dependent > function, to match the convention we're using in the rest of the Vala code. Done locally. > + success = ImportTool.import.end (r); > > * If the destination relationships.ini is missing, folks-import just hangs. Bug #628853. > * folks-import spews a ton of .goutputstream-* files into ~/.local/share/folks That sounds like a GIO bug; they're probably not getting cleaned up after a file operation is cancelled. > * if the alias == ID, don't bother setting the alias > * I've got a <contact> with several entries from one account. One of these IDs > is also on another account (and also shows up in a separate <contact>). For > some reason, the final entry in relationships.ini gives me the set difference > (missing this duplicate entirely) instead of union. > > The final relationships.ini doesn't end with a newline. This isn't a huge deal, > but we might want to just ensure that it does, to make appending less > problematic. We never append to the file; it's always written out afresh with the string output of KeyFile.to_data(). I think it's more trouble than it's worth to start meddling with the data we put into relationships.ini (given how bad the GKeyFile API is). > This isn't a blocker for merging just now, but it'd be nice if you could > identify and notify Permission Denied errors when failing due to that. I'll keep the bug open and work on that after 0.1.17's released. > Another non-blocker enhancement would be to filter incoming duplicates. I've > got several metacontacts with entries like "aim=foo;foo". It's not clear > whether it's because I've got "foo" from two different accounts in the same > <contact> entry or because I've got another <contact> entry in another place > containing that same ID (from one of the two accounts mentioned before). I don't think it's worth doing that. We've already got code to do it: the IndividualAggregator. Writing similar code again is asking for bugs. It doesn't gain us anything, and the user isn't expected to examine relationships.ini for prettiness.
(In reply to comment #5) > > * folks-import spews a ton of .goutputstream-* files into ~/.local/share/folks > > That sounds like a GIO bug; they're probably not getting cleaned up after a > file operation is cancelled. Filed as bug #629301.
Seems I missed these two comments: (In reply to comment #4) > * if the alias == ID, don't bother setting the alias The code doesn't add buddies which only have one IM address and an alias matching that IM address. Do we want to not store the alias for buddies with multiple IM addresses if it matches one of the IM addresses? I'm not sure. > * I've got a <contact> with several entries from one account. One of these IDs > is also on another account (and also shows up in a separate <contact>). For > some reason, the final entry in relationships.ini gives me the set difference > (missing this duplicate entirely) instead of union. Could you give an example?
Created attachment 169970 [details] [review] Add a folks-import tool (updated again) http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/629084-folks-import-attempt3
(In reply to comment #7) > Seems I missed these two comments: > > (In reply to comment #4) > > * if the alias == ID, don't bother setting the alias > > The code doesn't add buddies which only have one IM address and an alias > matching that IM address. Do we want to not store the alias for buddies with > multiple IM addresses if it matches one of the IM addresses? I'm not sure. Upon further thought, I've decided this is a bad idea. In most cases, we'd want to automatically discard a matching alias - but if a protocol supported significant whitespace and capital letters, we certainly wouldn't want to prevent the user from setting/using the alias, eg, "Frank Reynolds" just because it matched their ID. This will also be a moot point for anyone who uses a Facebook Jabber account and links those Individuals with that person's IM-created Individuals. > > * I've got a <contact> with several entries from one account. One of these IDs > > is also on another account (and also shows up in a separate <contact>). For > > some reason, the final entry in relationships.ini gives me the set difference > > (missing this duplicate entirely) instead of union. > > Could you give an example? This turned out to be a matter of the ID in relationships.ini not being normalized, and thus failing to match with the ID of the incoming TpContact. I've punted this as bug #629311 for now, since it'd be fairly non-trivial to solve.
I've fixed the Permission Denied case (and a couple related cases) and merged this to master, so we shouldn't need to keep this bug open: commit 17d54039f6b4a505ca83fbfb88b6dcccb9cd9d8f Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Fri Sep 10 12:13:42 2010 -0700 Add some more detailed validation/error reporting to the importing tool. tools/import-pidgin.vala | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) commit e41c4a953bf36bd13ebabb0fb78bd1389b17ef56 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Sep 8 19:36:04 2010 +0100 Bug 629084 <E2><80><94> Add a folks-import tool Add a folks-import tool which allows importing of Pidgin meta-contact information to libfolks' key file. Closes: bgo#629084 Makefile.am | 1 + configure.ac | 9 ++ folks/backend-store.vala | 2 +- tools/Makefile.am | 34 +++++++ tools/import-pidgin.vala | 247 ++++++++++++++++++++++++++++++++++++++++++++++ tools/import.vala | 189 +++++++++++++++++++++++++++++++++++ 6 files changed, 481 insertions(+), 1 deletions(-) commit 964ed5760f9dabe6562cca7bdc2f564ee7a8a8a1 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Thu Sep 9 16:07:08 2010 +0100 Block flushing of Kf.PersonaStore on any pending file operations If a file operation is still underway, don't allow the Kf.PersonaStore to be finalized until it's finished. This prevents libfolks from being closed befo changes to relationships.ini have been written out. We also prevent multiple file operations from happening (pseudo-) concurrently, by cancelling any pending operation when we schedule a new one. backends/key-file/kf-persona-store.vala | 40 ++++++++++++++++++++++++++++-- folks/persona-store.vala | 17 +++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-)
The folks-import tool potentially exposes bug #629311 for AIM accounts imported from Pidgin. A work-around is to manually normalize AIM account names in ~/.local/share/folks/relationships.ini (make the contact IDs all lower-case and remove whitespace). (The same issue is theoretically true for any other type of account where the contact ID wasn't normalized in the blist.xml file before importing, but I'm not aware any other protocols where this might be common).