Also watch this space for migration tips:
http://live.gnome.org/Evolution/ESourceMigrationGuide

I'm writing the guide FAQ-style, adding content as questions arise.

So can I get an estimate on this?  I'm leaning towards disabling the e-d-s backend (at least temporarily) to keep the 3.6 build working (at a cost of the contact search feature regressing).

Also, I think there were some rumblings about having gnome-contacts be a search provider?

I've been working on this yesterday and today.  Matthew has made some fixes in eds itself as I've hit them also.  I expect a fix to be ready for review by the end of the day today.

(In reply to comment #4)
> I've been working on this yesterday and today.  Matthew has made some fixes in
> eds itself as I've hit them also.  I expect a fix to be ready for review by the
> end of the day today.

Awesome!  I can help with introspection stuff if you need it.  But for now:

http://git.gnome.org/browse/jhbuild/commit/?id=5698810efcd8c0d767c49f60a6d430707bf8c675

http://git.gnome.org/browse/folks/log/?h=edsport fixes the problem.  I'd like someone to review it before merge though, since I'm relatively new to vala.

I looked it over already and spotted a couple minor things that are now fixed.  Someone with more folks + vala experience should give it a second look.

Minor nitpick:

+          var extension = (E.SourceAddressBook) this._source_registry.find_extension (this.source, SOURCE_EXTENSION_ADDRESS_BOOK);

This should be wrapped (it appears twice).

(In reply to comment #3)
> Also, I think there were some rumblings about having gnome-contacts be a search
> provider?

bug #677442

(In reply to comment #6)
> http://git.gnome.org/browse/folks/log/?h=edsport fixes the problem.  I'd like
> someone to review it before merge though, since I'm relatively new to vala.

Please attach the patch to Bugzilla in future so that it can be reviewed using the excellent Splinter!

The branch looks good to me. There are a few comments below, and I have some ponderances about the memory management with GLib.List APIs like E.SourceRegistry.list_sources(). It might be worthwhile checking that the memory management of those lists is correct in C, since we’ve had GLib.Lists be leaked before (or duplicated every time the code touches them).

Thanks for working on this!

> + this._ab_source_list_changed_cb ( );

Extraneous space between the brackets.

> +[CCode (cname = "e_source_registry_new", cheader_filename = "libedataserver/libedataserver.h", finish_function = "e_source_registry_new_finish")]
> +public extern static async E.SourceRegistry create_source_registry (GLib.Cancellable? cancellable = null) throws GLib.Error;

Why isn’t this in EDS’ vapi file, and why is it not in a namespace?

> - /* FIXME: Add authentication support. That's:
> - * https://bugzilla.gnome.org/show_bug.cgi?id=653339

How is authentication affected by the changes in EDS? It’s handled by EDS now, iirc, so I think folks doesn’t need to do anything. If so, we should probably edit/close bug #653339.

> /* base_uri should be google:// for Google Contacts address books */
> - if (backend_name.has_prefix ("google:"))
> + if (backend_name.has_prefix ("google"))

Please update the comment above this line since it’s now out of date.

> + E.Source? needle = ((!) this._source_registry).ref_source(this.id);

Missing space after “ref_source”.

> -SUBDIRS += eds
> +#SUBDIRS += eds

What’s happening about the tests?

(In reply to comment #10)
> The branch looks good to me. There are a few comments below, and I have some
> ponderances about the memory management with GLib.List APIs like
> E.SourceRegistry.list_sources(). It might be worthwhile checking that the
> memory management of those lists is correct in C, since we’ve had GLib.Lists be
> leaked before (or duplicated every time the code touches them).

I tried to make that clear in the documentation:
http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html#e-source-registry-list-sources

Let me know if the return type annotation is missing anything.

Philip,

Thanks for the review.  I've fixed most of what you pointed out on that branch.  Is there an easy way to make one patch file from a whole branch with git, so I can attach the whole branch here?  Also, what's splinter that you talk of?

As for the tests, they need to be rethought, as relative sources are no longer possible with the eds changes.  We may need to change the XDG_DATA variable or somesuch instead.

As for why that method isn't in the eds vapi file it's because the eds vapi file only has the async method.  I was told in #vala that valac doesn't support async constructors and to do this instead.

I'll close that 653339 bug when I merge this branch to master, thanks for pointing that out.

As for the GList stuff, looking at the generated c code, it seems that the list is unrefed at the end of the method it is used in.  If I need to do something else in the vala sources to delete the Source objects, let me know. (I'll ask in #vala also).

(In reply to comment #12)
> As for why that method isn't in the eds vapi file it's because the eds vapi
> file only has the async method.  I was told in #vala that valac doesn't support
> async constructors and to do this instead.

A synchronous version exists: e_source_registry_new_sync()

http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html#e-source-registry-new-sync

Maybe another missing annotation is excluding it from the vapi file?

Ah, no it's not in the .vapi file that I could see.  But apparently new SourceRegistry.sync () is all that was needed.  Just pushed a commit that removes the custom function.

I also verified the generated code does free the GList we get from list_sources.

Ok to merge, Philip?

Is this compatible with evolution's daemons 3.4? ie, can I just upgrade the lib in my jhbuild and continue using evolution 3.4 from my system?

(In reply to comment #12)
>  Is there an easy way to make one patch file from a whole branch with git, so I
> can attach the whole branch here?  Also, what's splinter that you talk of?

I usually do the following, but I believe git-bz is easier (which I think Guillaume uses):
    git branch temp
    git rebase -i HEAD~(some number of commits)
    (mark all commits for the patch as squashed)
    git format-patch HEAD~1
    git co my-previous-branch
    git branch -D temp

Splinter is a patch review tool integrated into bugzilla.gnome.org. When someone uploads a patch as an attachment, it gives the reviewer a side-by-side view of the diff and the ability to make inline comments.

> As for the tests, they need to be rethought, as relative sources are no longer
> possible with the eds changes.  We may need to change the XDG_DATA variable or
> somesuch instead.

Right. Could you please add a comment in the Makefile where you’ve commented out the tests, pointing to this bug report with a ‘FIXME’?

> As for why that method isn't in the eds vapi file it's because the eds vapi
> file only has the async method.  I was told in #vala that valac doesn't support
> async constructors and to do this instead.

If it’s a choice between using the async constructor with that annotation in the Vala file, and using the synchronous constructor, I think I’d prefer the async constructor to be used. We don’t really want to be making synchronous D-Bus calls.

So I guess you should revert commit d921aee0bc840446e1f40ebc98826b64d1fc142d and add a comment above the create_source_registry() declaration explaining why it’s necessary.

> As for the GList stuff, looking at the generated c code, it seems that the list
> is unrefed at the end of the method it is used in.  If I need to do something
> else in the vala sources to delete the Source objects, let me know. (I'll ask
> in #vala also).

As long as the GIR annotations are present in the EDS code, Vala should magically sort out the memory management of GLists fine. From the documentation which Matthew provided, the annotations look good, so I’m not worried about this now. :-)

(In reply to comment #14)
> Ok to merge, Philip?

I’m happy for it to be merged once the comments above have been fixed. Don’t forget to update the NEWS file. Thanks!

(In reply to comment #15)
> Is this compatible with evolution's daemons 3.4? ie, can I just upgrade the lib
> in my jhbuild and continue using evolution 3.4 from my system?

No, Evolution-Data-Server broke backward compatibility in 3.5, hence the need for this libfolks patch.  GNOME Shell and several other modules have already been patched.

I posted a blog entry with more info about the breakage:
http://mbarnes.livejournal.com/4631.html

Merged edsport branch to master.

Re-enabled for the 3.6 build, thanks!

http://git.gnome.org/browse/jhbuild/commit/?id=f8f9491af1c62d52768002100f6d4b74b3a66cd7