GNOME Bugzilla – Bug 668415
Port to Vala 0.15.x
Last modified: 2012-02-23 02:37:08 UTC
Created attachment 205781 [details] [review] Colin Walters' patch to fix the build against Vala git master The latest 0.15.x (and git master) versions of Vala have broken API, thus causing build errors for Folks in jhbuild. This needs to be fixed as soon as possible.
(In reply to comment #0) > Created an attachment (id=205781) [details] [review] > Colin Walters' patch to fix the build against Vala git master > > The latest 0.15.x (and git master) versions of Vala have broken API, thus > causing build errors for Folks in jhbuild. > > This needs to be fixed as soon as possible. Note that while this fixes the build against Vala git master, it doesn't work with any released (as of this writing) version of Vala (including 0.15.0), so we can't yet merge it.
(In reply to comment #1) > (In reply to comment #0) > > Created an attachment (id=205781) [details] [review] [details] [review] > > Colin Walters' patch to fix the build against Vala git master > > > > The latest 0.15.x (and git master) versions of Vala have broken API, thus > > causing build errors for Folks in jhbuild. > > > > This needs to be fixed as soon as possible. > > Note that while this fixes the build against Vala git master, it doesn't work > with any released (as of this writing) version of Vala (including 0.15.0), so > we can't yet merge it. Are the problems not solvable with an “#if VALA_0_15_1” and poking Jürg for a Vala release?
Anyone mind if I push a wip/vala-0.15 branch to git?
(In reply to comment #3) > Anyone mind if I push a wip/vala-0.15 branch to git? Please do. :-)
(In reply to comment #2) > (In reply to comment #1) > > (In reply to comment #0) > > > Created an attachment (id=205781) [details] [review] [details] [review] [details] [review] > > > Colin Walters' patch to fix the build against Vala git master > > > > > > The latest 0.15.x (and git master) versions of Vala have broken API, thus > > > causing build errors for Folks in jhbuild. > > > > > > This needs to be fixed as soon as possible. > > > > Note that while this fixes the build against Vala git master, it doesn't work > > with any released (as of this writing) version of Vala (including 0.15.0), so > > we can't yet merge it. > > Are the problems not solvable with an “#if VALA_0_15_1” and poking Jürg for a > Vala release? Yes, this should be fixable shortly (there's supposed to be a release soon anyhow). Though our next release is also blocking on bug #666728, its merge and subsequent Vala release and a libgee release re-built with that new release of Vala.
Created attachment 206058 [details] [review] Build with Vala 0.15 and latest libgee 0.6 branch Patches from branch: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo668415-port-to-vala-0.15 =========== This certainly still needs some work; particularly, in the way that some tests fail consistently (some eds tests fail to find the eds backend for some reason) and others fail intermittently (eg, telepathy/individual-properties). I haven't had time to do more research - maybe you could try it out, Philip? I've fixed some minor issues caused/exposed by the newer versions of GLib, Vala, and libgee, but it's not clear what's triggering the new test failures. I don't think it's something we've suddenly changed since things have been fairly quiet for a while.
Review of attachment 206058 [details] [review]: Looks OK to me apart from the folks-inspect change. > This certainly still needs some work; particularly, in the way that some tests > fail consistently (some eds tests fail to find the eds backend for some reason) > and others fail intermittently (eg, telepathy/individual-properties). For the avoidance of doubt, do the tests all run fine for you without this patch applied? I just want to rule it out as being a problem with your jhbuild setup. ::: tools/inspect/inspect.vala @@ +300,3 @@ } +public abstract class Folks.Inspect.Command What's the need for this?
Created attachment 206280 [details] [review] Some trivial patches to clean up the build and tests for Vala 0.14.x Patches from branch: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=trivia ========================================== These fix the build and test issues solved in the other branch (and another new one in the Tracker tests) and they apply and work with the exception of a couple of EDS test failures (which also fail in the other branch). We should apply these to master and rebase the 0.15.x branch upon it before merging the 0.15.x branch and closing this bug. Here are the failures: (this one only fails some of the time - maybe due to bad temporary file clean-up?) make[1]: Entering directory `/home/treitter/collabora/folks/tests/eds' /StoreRemoved/single store: folks-WARNING **: Failed to find primary PersonaStore with type ID 'eds' and ID '1327681660.23006.1@ridley'. Individuals will not be linked properly and creating new links between Personas will not work. The configured primary PersonaStore's backend may not be installed. If you are unsure, check with your distribution.
+ Trace 229539
----------------------------- I haven't had time to root these out, but we clearly should before the next release.
Now that Vala 0.15.1 has been released, we can depend upon that in the final fix for this bug.
Review of attachment 206280 [details] [review]: Looks good to me.
(In reply to comment #10) > Review of attachment 206280 [details] [review]: > > Looks good to me. Merged: commit 3f78e4a1c5b6cb9084608fa3a28264341d4274c7 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Thu Jan 26 16:27:03 2012 -0800 Only add non-empty Role or PostalAddress fields in Tracker backend This prevents some newly-exposed test failures. commit 50171f5392901dca6eb1af48cc76a7b0c7ad7b5b Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Tue Jan 24 18:00:29 2012 -0800 Don't assume every Tpf.Persona has a contact commit 22e4318d3ff4a10c94d51d509553428529c9a729 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Tue Jan 24 17:00:58 2012 -0800 Fix the nullity of test functions. commit 05e72fcf261fb7a45cebf6db58c754d1b548f72f Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Tue Jan 24 15:57:32 2012 -0800 Make Individual implement PresenceDetails properly commit 84ada94322ade52392f386b09450d1a92ed9d02c Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Sat Jan 21 17:24:05 2012 -0800 Make Tpf.Persona properly implement the PresenceDetails properties.
Created attachment 206469 [details] [review] Build with Vala 0.15 and latest libgee 0.6 branch (try 2) Rebased patches here from here: http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo668415-port-to-vala-0.15-try2 =================== These fix a warning with several invalid uses of typeof (Foo<Bar>). The two EDS tests still fail, so I haven't merged this yet. As soon as I get some time to fix those, I'll merge and make a new release.
Review of attachment 206469 [details] [review]: Looks good to me.
The bugs we're hitting with the eds/change-primary-store test seem to similar to that old bug where the EDS tests were creating (and never destroying) a ton of address books. They didn't seem to show up in Evolution immediately, but upon checking just now, it's completely flooded and they don't even seem to be deletable! There seem to be two core problems with the test and Vala 0.15 (at least the first of which doesn't show up with Vala 0.14.x): * a race condition almost always (about 90% of the time) ends up trying to prepare the Eds.PersonaStore after the corresponding EdsTest backend has been torn down and its addressbook has been deleted, resulting in errors like: folks-WARNING **: Error preparing persona store 'eds:1328052896.7064.1@ridley': Couldn't get view for address book ?1328052896.7064.1@ridley?: The name :1.6 was not provided by any .service files * even at the very beginning of the test run, there are a ton of ESourceGroups (each containing exactly one ESource - either the "system" or "other" -- these are the two abooks we add in the test). Over many runs, the total count of these ESourceGroups/ESources didn't seem to increase. But I'm not sure why it got so big in the first place (or why I can't delete them manually). So, honestly, this needs a lot more work and I'm not sure what's causing those problems above. It seems like a change in Vala 0.15 exposed some bug in Folks and/or the Folks EDS backend. I'll try to look at this again when I get some free time, but I don't expect that to be any time soon, unfortunately.
Hmm, after some seriously weird behavior in Evolution and forced process killing, I got the exact address books showing up that I expected (and the test passed, unsurprisingly). One "Test" address book was still in Evo, but I could remove it without a problem. (I'm not 100% sure whether I removed this before or after everything got to the final good state) Running the test again after this results in both test address books not being properly deleted (despite E.BookClient.remove_sync() supposedly succeeding), so the list of pre-existing books grows by two every run. However, they don't show up in Evolution (even if I completely kill it and e-addressbook-factory ahead of time). And we do properly delete the /tmp/tmp.* directories that we create during the test (which store the test addressbook files). Matthew, do you know what could be causing this behavior? (I'm running e-d-s a97a7aff3ea212f7046a929942a6c884d4e1cfe7 - git master from today)
*** Bug 669852 has been marked as a duplicate of this bug. ***
Matthew, ping?
I think it's a red herring for our failing tests, but all the extra Test address books are due to bug #658327 not actually being fixed. So I'll relegate that issue to that bug. Manually clearing out the %gconf.xml file does indeed clear out the extra Test address books, but the test fails with similar frequency and in the same way.
Would be great to fix this ASAP. Jhbuild currently uses the vala-0-15 branch making hard to test latest changes in master.
Btw, I can't build this branch either using jhbuild vala: aggregation.vala:58.48-58.68: error: Argument 1: Cannot convert from `string?' to `uint8[]' kf_relationships_f.replace_contents (kf_relationships_data, ^^^^^^^^^^^^^^^^^^^^^ aggregation.vala:83.39-83.42: error: Argument 1: Cannot convert from `string?' to `uint8[]' backend_f.replace_contents (data,
(In reply to comment #20) > Btw, I can't build this branch either using jhbuild vala: > > > aggregation.vala:58.48-58.68: error: Argument 1: Cannot convert from `string?' > to `uint8[]' > kf_relationships_f.replace_contents (kf_relationships_data, > ^^^^^^^^^^^^^^^^^^^^^ > aggregation.vala:83.39-83.42: error: Argument 1: Cannot convert from `string?' > to `uint8[]' > backend_f.replace_contents (data, Ignore that, http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo668415-port-to-vala-0.15-try2 fixed it. Shouldn't we merge it to wip/vala-0-15 as that's the branch used by jhbuild?
(In reply to comment #21) > (In reply to comment #20) > > Btw, I can't build this branch either using jhbuild vala: > > > > > > aggregation.vala:58.48-58.68: error: Argument 1: Cannot convert from `string?' > > to `uint8[]' > > kf_relationships_f.replace_contents (kf_relationships_data, > > ^^^^^^^^^^^^^^^^^^^^^ > > aggregation.vala:83.39-83.42: error: Argument 1: Cannot convert from `string?' > > to `uint8[]' > > backend_f.replace_contents (data, > > Ignore that, > http://cgit.collabora.com/git/user/treitter/folks.git/log/?h=bgo668415-port-to-vala-0.15-try2 > fixed it. > > Shouldn't we merge it to wip/vala-0-15 as that's the branch used by jhbuild? I'd been delaying merging this until I'd found fixes for our newly-broken tests. Just as I was going to push these fixes anyhow, I tried running the tests and suddenly they pass again. After no changes on my part. After some bisecting, it looks like this commit caused the problems in the EDS tests: commit cc1f7e2af35219c69ea4b1e3222e25f5ab2b8c23 Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Tue Jan 31 13:19:35 2012 -0800 CUT -- debugging diff --git a/backends/eds/lib/edsf-persona-store.vala b/backends/eds/lib/edsf-pe index e71a9d8..94eeedf 100644 --- a/backends/eds/lib/edsf-persona-store.vala +++ b/backends/eds/lib/edsf-persona-store.vala @@ -881,8 +881,12 @@ public class Edsf.PersonaStore : Folks.PersonaStore throw new PersonaStoreError.INVALID_ARGUMENT ( /* Translators: the first parameter is an address book URI * and the second is an error message. */ - _("Couldn't get view for address book (%s) ‘%s’: %s"), - this._addressbook.source.peek_uid (), this.id, e3.message); + _("Couldn't get view for address book (%s -- %s -- %s -- %s) + this._addressbook.source.peek_uid (), + this._addressbook.source.peek_name (), + this._addressbook.source.peek_relative_uri (), + this._addressbook.source.peek_absolute_uri (), + this.id, e3.message); } finally { diff --git a/tests/lib/eds/backend.vala b/tests/lib/eds/backend.vala index b004973..f6b68d3 100644 --- a/tests/lib/eds/backend.vala +++ b/tests/lib/eds/backend.vala @@ -137,6 +137,15 @@ public class EdsTest.Backend this._source = new E.Source ("Test", this.address_book_uri); + /* FIXME: cut */ + GLib.message ("&&& preparing source %p -- %s -- %s -- %s -- %s", + this._source, + this._source.peek_uid (), + this._source.peek_name (), + this._source.peek_relative_uri (), + this._source.peek_absolute_uri ()); + + if (is_default) this._source.set_property ("default", "true"); =========== I didn't include this commit in my public branch because it was just noise. And I honestly still can't see what caused it to break the test. Maybe one of the peek_*() functions causes some side effects I don't know about? =========== At any rate, would everyone please try out git master and let me know if they have any problems with it (by reopening this bug)? If no one complains by early next week, I'll do a new release to include this fix.
Merged: commit c0003918e4be83e13e8823e0801486f05e66227d Author: Travis Reitter <travis.reitter@collabora.co.uk> Date: Mon Jan 30 11:23:26 2012 -0800 Cut invalid overly-specific type cast The Vala compiler now correctly warns that typeof(Foo<Bar>) is invalid, so this stops pretending we can be that specific. (The generated C code can't make a GValue as specific as the above Vala code fragment suggests; historically, the compiler would let you get away with this, likely with the false assumption that the generic type would ever be considered again.) folks/individual-aggregator.vala | 2 +- tests/eds/add-persona.vala | 12 ++++++------ tests/eds/link-personas.vala | 10 +++++----- tests/tracker/add-persona.vala | 14 +++++++------- tests/tracker/duplicated-emails.vala | 4 ++-- tests/tracker/duplicated-phones.vala | 4 ++-- tests/tracker/link-personas.vala | 4 ++-- tests/tracker/match-email-addresses.vala | 4 ++-- tests/tracker/match-im-addresses.vala | 4 ++-- tests/tracker/match-known-emails.vala | 4 ++-- tests/tracker/match-phone-number.vala | 4 ++-- tests/tracker/remove-persona.vala | 2 +- tests/tracker/set-duplicate-email.vala | 2 +- 13 files changed, 35 insertions(+), 35 deletions(-) commit 55539ee4accd29d89d28d23e6ef76ad2ab023724 Author: Colin Walters <walters@verbum.org> Date: Fri Jan 20 14:46:14 2012 -0500 Build with vala 0.15 NEWS | 1 + backends/key-file/kf-persona-store.vala | 13 +++---------- configure.ac | 2 +- folks/backend-store.vala | 16 ++++++++-------- folks/object-cache.vala | 6 +++--- tests/folks/backend-loading.vala | 6 +++--- tests/lib/eds/backend.vala | 2 +- tests/libsocialweb/aggregation.vala | 10 ++++------ tools/inspect/inspect.vala | 2 +- 9 files changed, 25 insertions(+), 33 deletions(-)
Colin, you should be able to remove your wip/vala-0-15 branch now.
(In reply to comment #24) > Colin, you should be able to remove your wip/vala-0-15 branch now. I updated jhbuild to track master again and removed the branch.
Note to anyone tracking this bug waiting on fixes being released - I've just released Folks 0.6.7, which includes the fixes for this bug.