Re: [Evolution-hackers] New 'eclient' branch in eds
On Di, 2011-06-14 at 07:38 -0400, Matthew Barnes wrote: > On Tue, 2011-06-14 at 12:08 +0200, Patrick Ohly wrote: > > My two cents as a user of these APIs: having to deal with a major API > > change once is acceptable. Whether it is in 3.2 or 3.4 I don't really > > care. > > > > But having to rewrite code both for 3.2 *and* 3.4 goes a bit too far. So > > if the account handling doesn't land in 3.2, then please let's keep the > > current (EDS 3.0) APIs officially supported in 3.2. > > There are no major (nor minor, that I'm aware of) public API breaks in > 3.1 as of yet, just new alternative APIs for EBook and ECal. EBook and > ECal are deprecated but still work the same, I'm relieved to hear that. Milan had me worried for a while with the talk about deprecating these calls, but I only misunderstood the implications. -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-06-14 at 12:08 +0200, Patrick Ohly wrote: > My two cents as a user of these APIs: having to deal with a major API > change once is acceptable. Whether it is in 3.2 or 3.4 I don't really > care. > > But having to rewrite code both for 3.2 *and* 3.4 goes a bit too far. So > if the account handling doesn't land in 3.2, then please let's keep the > current (EDS 3.0) APIs officially supported in 3.2. There are no major (nor minor, that I'm aware of) public API breaks in 3.1 as of yet, just new alternative APIs for EBook and ECal. EBook and ECal are deprecated but still work the same, and will remain until we've verified all known users of the APIs have migrated. I would suggest staying with EBook and ECal until the account management changes land. I'm trying my best to get that done by 3.2, but there are inevitable APIs breaks that will accompany it regardless of when it lands. Half of libedataserver will be thrown out and replaced, with moderate impacts to libedataserverui, libebook and libecal. If you're interested yet, documentation for the new libedataserver API is posted here. I do plan to write a migration guide before merging. http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/index.html I also wrote about impacts to the other libraries awhile back. It's slightly dated and incomplete now, but what's there is still accurate: http://mail.gnome.org/archives/evolution-hackers/2010-December/msg00029.html ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-06-09 at 12:25 -0400, Matthew Barnes wrote: > On Thu, 2011-06-09 at 17:11 +0200, Milan Crha wrote: > > I hope not, I'm currently working on evo bits to be buildable with > > E_BOOK_DISABLE_DEPRECATED and E_CAL_DISABLE_DEPRECATED defined (eds is > > done, but I'm postponing it till I have evo and the standard rest done > > as well). Having one API deprecated and second "unstable" doesn't sound > > good to me, same as there doesn't seem to be many things to change > > anyway. We can always increase .so name version, that's just for it, > > isn't it? > > Anything in the EClient API dealing with ESourceList, URI strings, or > default sources will be removed when the account-mgmt branch is merged, > and there's a fair chance now that may not happen until 3.3. So the API > is unstable whether we claim it to be or not. My two cents as a user of these APIs: having to deal with a major API change once is acceptable. Whether it is in 3.2 or 3.4 I don't really care. But having to rewrite code both for 3.2 *and* 3.4 goes a bit too far. So if the account handling doesn't land in 3.2, then please let's keep the current (EDS 3.0) APIs officially supported in 3.2. -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 17:07 -0400, Matthew Barnes wrote: > On Thu, 2011-06-09 at 22:48 +0200, Milan Crha wrote: > > Thanks, I'll do that. Just the first transition will be about not using > > deprecated API, the second part of it, making calls async, is a very > > different task, as I go through calendar sources and see all the sync > > calls and the API being served synchronously as well. Thus I expect some > > design changes will be needed in calendar, to make this work as > > intended. Contacts seems mostly fine, for what I touched so far. > > Understood. If we could at least get the contact photo mess fixed up in > the mailer for 3.2 that would be awesome. It still blocks the UI. I plan that and itip-formatter plugin right after I finish this. I've a little idea what to do, but will see whether it'll be doable in that way. Within the initial changes I already changed some bits, which are to-be-tested, which might make the canceling working properly for the addressbook lookup, even when it's stuck. But will see later. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 22:48 +0200, Milan Crha wrote: > Thanks, I'll do that. Just the first transition will be about not using > deprecated API, the second part of it, making calls async, is a very > different task, as I go through calendar sources and see all the sync > calls and the API being served synchronously as well. Thus I expect some > design changes will be needed in calendar, to make this work as > intended. Contacts seems mostly fine, for what I touched so far. Understood. If we could at least get the contact photo mess fixed up in the mailer for 3.2 that would be awesome. It still blocks the UI. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 13:15 -0400, Matthew Barnes wrote: > Yes, please do get us using the async EClient APIs in Evolution ASAP. > I can deal with the impact to the branch -- I don't expect it to be too > severe. Thanks, I'll do that. Just the first transition will be about not using deprecated API, the second part of it, making calls async, is a very different task, as I go through calendar sources and see all the sync calls and the API being served synchronously as well. Thus I expect some design changes will be needed in calendar, to make this work as intended. Contacts seems mostly fine, for what I touched so far. It would be nice to have the first part ready for the Monday release, but I'm not that far yet, thus if no miracle will happen then I've this done for 3.1.3. Pity for another soname version bump due to API changes in libedataserverui, which I would prefer to have in once, but... Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 18:40 +0200, Milan Crha wrote: > By the way, thinking of your announcement, I hope you are fine if I'll > finish this "stop using deprecated Book/Cal API in evo" as soon as > possible and commit it, thus it'll have more testing (I'm pretty sure > I'll introduce few regressions and bugs, even it's more or less monkey > work). As I mentioned couple times on various threads, messages and > maybe also IRC chats, this will make your life pretty harder, as the > change will not be simple, and the initial merge of such change into > your account management branch will be painful. Yes, please do get us using the async EClient APIs in Evolution ASAP. I can deal with the impact to the branch -- I don't expect it to be too severe. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
(I cannot Ctrl+L the message) On Thu, 2011-06-09 at 12:25 -0400, Matthew Barnes wrote: > On Thu, 2011-06-09 at 17:11 +0200, Milan Crha wrote: > > I hope not, I'm currently working on evo bits to be buildable with > > E_BOOK_DISABLE_DEPRECATED and E_CAL_DISABLE_DEPRECATED defined (eds is > > done, but I'm postponing it till I have evo and the standard rest done > > as well). Having one API deprecated and second "unstable" doesn't sound > > good to me, same as there doesn't seem to be many things to change > > anyway. We can always increase .so name version, that's just for it, > > isn't it? > > Anything in the EClient API dealing with ESourceList, URI strings, or > default sources will be removed when the account-mgmt branch is merged, > and there's a fair chance now that may not happen until 3.3. So the API > is unstable whether we claim it to be or not. > > Obviously we would bump sonames when things change with or without the > #define. My suggestion was more about setting expectations for users of > the API. It's a way of saying "we think this API is stable but it's > still too soon to know for sure; fair warning". It's different, from my point of view, as it's only few functions, with compare of the whole EClient related API, not talking that it's trying to be compatible, in this account management area, with the previous API, where your change just changes core functions. Thus I would keep this as is. By the way, thinking of your announcement, I hope you are fine if I'll finish this "stop using deprecated Book/Cal API in evo" as soon as possible and commit it, thus it'll have more testing (I'm pretty sure I'll introduce few regressions and bugs, even it's more or less monkey work). As I mentioned couple times on various threads, messages and maybe also IRC chats, this will make your life pretty harder, as the change will not be simple, and the initial merge of such change into your account management branch will be painful. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 17:11 +0200, Milan Crha wrote: > I hope not, I'm currently working on evo bits to be buildable with > E_BOOK_DISABLE_DEPRECATED and E_CAL_DISABLE_DEPRECATED defined (eds is > done, but I'm postponing it till I have evo and the standard rest done > as well). Having one API deprecated and second "unstable" doesn't sound > good to me, same as there doesn't seem to be many things to change > anyway. We can always increase .so name version, that's just for it, > isn't it? Anything in the EClient API dealing with ESourceList, URI strings, or default sources will be removed when the account-mgmt branch is merged, and there's a fair chance now that may not happen until 3.3. So the API is unstable whether we claim it to be or not. Obviously we would bump sonames when things change with or without the #define. My suggestion was more about setting expectations for users of the API. It's a way of saying "we think this API is stable but it's still too soon to know for sure; fair warning". ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-06-09 at 07:12 -0400, Matthew Barnes wrote: > Since merging the account-mgmt branch will change nearly all client-side > APIs in E-D-S including EClient, and since it would be wise regardless > of my schedule to allow ourselves some extra time to tweak this brand > new EClient API before pronouncing it frozen, what do you think about > forcing users of it to acknowledge that the API is still unstable by > having them do something like: > > #define ECLIENT_API_IS_SUBJECT_TO_CHANGE > #include > > and leave that requirement in place until 3.4 or maybe even 3.6? Hi, I hope not, I'm currently working on evo bits to be buildable with E_BOOK_DISABLE_DEPRECATED and E_CAL_DISABLE_DEPRECATED defined (eds is done, but I'm postponing it till I have evo and the standard rest done as well). Having one API deprecated and second "unstable" doesn't sound good to me, same as there doesn't seem to be many things to change anyway. We can always increase .so name version, that's just for it, isn't it? Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, 2011-05-23 at 11:54 +0200, Milan Crha wrote: > I just committed changes from 'eclient' branch to eds' master branch. > This will be part of evolution-data-server 3.1.2. > > I'll adapt evolution-exchange, evolution-mapi, evolution-groupwise and > evolution-couchdb for the API and "opening phase" changes. Feel free to > poke me if you've any questions. Red Hat conscripted me into adding Evolution integration for the new GNOME Online Accounts service debuting in GNOME 3.2. The plan is to support Google accounts configured through GOA in Evolution 3.2. This is slowing down my progress on the account-mgmt branch and putting it at greater risk of not being ready in time for 3.2. The changes in that branch are too invasive to merge at the tail-end of a devel cycle, so I need to finish it up soon or I'll have to punt to 3.3. Since merging the account-mgmt branch will change nearly all client-side APIs in E-D-S including EClient, and since it would be wise regardless of my schedule to allow ourselves some extra time to tweak this brand new EClient API before pronouncing it frozen, what do you think about forcing users of it to acknowledge that the API is still unstable by having them do something like: #define ECLIENT_API_IS_SUBJECT_TO_CHANGE #include and leave that requirement in place until 3.4 or maybe even 3.6? This seems to be the de facto standard way among GNOME libraries of indicating that an API is still unstable in a stable release. Of course once we release 3.2 the usual rules apply to the gnome-3-2 branch; we can only make further changes to the API in 3.3. Sound reasonable? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
Hi, I just committed changes from 'eclient' branch to eds' master branch. This will be part of evolution-data-server 3.1.2. I'll adapt evolution-exchange, evolution-mapi, evolution-groupwise and evolution-couchdb for the API and "opening phase" changes. Feel free to poke me if you've any questions. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Fri, 2011-04-29 at 07:24 -0400, Matthew Barnes wrote: > On Fri, 2011-04-29 at 06:59 +0200, Milan Crha wrote: > > There was just a little problem with ECalView, which had 'client' > > property, which is referring to ECal, instead of ECalClient, and I was > > forced to "invent" different name for it. But after a bit of sleeping > > and small thinking I might be wrong on this point too, as it should be > > easy to define EBookClientView/ECalClientView and keep old > > EBook/CalView-s as they are, at least their public API. > > Cool. That was my initial thought too but didn't know how much extra > work that would be. Hi, so I did. There are new EBookClientView/ECalClientView defined. They use exactly the same interface, with same signal names defined and their signatures, the only difference is function naming and the GDBus object used at the background. Consider it as the first step on the "merge common code of cal/book factory and cal/book view", which will make it easier to adapt to such change in the future. As I mentioned earlier, factories and views, all concrete implementations and gdbus stubs can be merged and mostly reused between calendar and addressbook factories. The only issue is compilation order (some file placement restructuralization will be needed, for example to be able to define EClientView and have it available in e-client.h, but also in e-cal-client.h/.c and e-book-client.h/.c for exact implementation). I noticed one difference between factories, recently. The calendar factory keeps running all backends for all the time the factory is run (since the backend is opened for the first time), even when no consumers exist. The addressbook factory frees the backend as soon as the last client is gone. Each has its advantages for sure. I'm mentioning it here only to be aware of such difference and to anyone going further with the merge idea to pick the better of them. I'm not sure which it is, though. I'm not going any further in this merging effort, because I do not think it breaks anyhow the EClient idea as is. The merging can be postponed to any time later, even for 3.4 or beyond, from my point of view. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Fri, 2011-04-29 at 06:59 +0200, Milan Crha wrote: > There was just a little problem with ECalView, which had 'client' > property, which is referring to ECal, instead of ECalClient, and I was > forced to "invent" different name for it. But after a bit of sleeping > and small thinking I might be wrong on this point too, as it should be > easy to define EBookClientView/ECalClientView and keep old > EBook/CalView-s as they are, at least their public API. Cool. That was my initial thought too but didn't know how much extra work that would be. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 15:04 -0400, Matthew Barnes wrote: > On Thu, 2011-04-28 at 19:53 +0200, Milan Crha wrote: > > Only the last thing for the enum values, I would personally prefer > > something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it > > confusion-able with E_CLIENT_TYPE constant. > > Do you mean E_TYPE_CLIENT (for getting EClientClass's GType)? Heh, right, I meant E_TYPE_CLIENT. I should read-before-write more often, I'm sorry. > Regardless, I'm fine with the longer prefix if you'd prefer that. Preferred on my side, yup. > > By the way, the proposed merge of server side parts, it may also involve > > merging client side bits (for E*View) and thus finally drop all the old > > cruft. It's a benefit, I hope, even with broken backward compatibility. > > (I would prefer to break backward compatibility personally, rather than > > inventing special names for structs not intersect with old names.) > > I haven't been following the E*View changes very closely. What's > considered cruft? There was just a little problem with ECalView, which had 'client' property, which is referring to ECal, instead of ECalClient, and I was forced to "invent" different name for it. But after a bit of sleeping and small thinking I might be wrong on this point too, as it should be easy to define EBookClientView/ECalClientView and keep old EBook/CalView-s as they are, at least their public API. I see I did really too quick thinking on these. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 21:16 +0200, Patrick Ohly wrote: > Agreed, the library dependency issue is a problem. That could be solved > by an utility library on top of libecal and libebook which offers the > unified API. Or you could just write your own function in EvolutionSync. It's just a switch statement on an enum value. > What about merging libebook and libecal into one shared library instead? > Evolution 3.2 will require an soname bump and source code changes in > apps anyway, throwing a renaming of libs into the mix won't make a big > difference. > > I think it would make the overall API a lot cleaner, albeit with > slightly (?) higher memory footprint for apps which only need one or the > other. I'll have to think on that. Seems kinda drastic, but maybe I'm just not seeing how it would make the overall API cleaner. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 13:07 -0400, Matthew Barnes wrote: > It hadn't occurred to me the word "calendar" might be ambiguous. I > blame the iCalendar spec for overloading it. In the real world, one > does not make a "calendar of tasks", one makes a "list of tasks". But tasks are shown on calendars because they have deadlines. It depends whether you see a calendar as a container of something or a tool to work with date-dependent items. > Maybe this is too Evolution-centric, but to me the container/item > relationship is clear: > >an ADDRESS_BOOK contains CONTACTS > a CALENDAR contains EVENTS > a TASK_LIST contains TASKS / TODOS > a MEMO_LIST contains MEMOS / JOURNALS > a MAIL_STOREcontains MESSAGES > > The enum values should be named consistently. So if we change CALENDAR > to EVENTS, I think we should change the rest. > > FWIW, the new ESource API is already using container names. Key files > will have groups named [Address Book], [Calendar], [Task List], etc. > instead of [Contacts], [Events], [Tasks], etc. > > To me it sounds more natural to refer to containers than items, but > maybe I'm too indoctrinated in Evolution-speak. Perhaps it is really to ingrained into Evolution already to change it. Never mind, I'll cope with it either way ;-) -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 16:34 +0200, Milan Crha wrote: > [please reply to the list only, it makes my life worse when you CC me] > > On Thu, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote: > > It would mean EClient has to know that ECalClient and EBookClient exist > > in order to instantiate the right one for a given enum value. Normally > > in class design you want to avoid that kind of knowledge seeping into > > lower layers, but with the class hierarchy being fixed to these three > > classes (four if we add email), I think it's a good tradeoff to have a > > more consistent API. > > > > So it would be something like: > > > > typedef enum { > > E_CLIENT_TYPE_ADDRESS_BOOK, > > E_CLIENT_TYPE_CALENDAR, > > E_CLIENT_TYPE_MEMO_LIST, > > E_CLIENT_TYPE_TASK_LIST > > } EClientType; > > You obviously face of a circular dependency, so it's not doable in a > clean way. Also because EClient is in libedataserver, EBookClient in > addressbook/libebook and similarly ECalClient in calendar/libecal. Both > descendants link to libedataserver... Having another > register_client/unregister_client function will make things only less > clear for readers (like if one tries to follow signal handlers by > reading the code. Agreed, the library dependency issue is a problem. That could be solved by an utility library on top of libecal and libebook which offers the unified API. In that case we wouldn't get rid of the special-purpose calls in libecal and libebook, though, would we? What about merging libebook and libecal into one shared library instead? Evolution 3.2 will require an soname bump and source code changes in apps anyway, throwing a renaming of libs into the mix won't make a big difference. I think it would make the overall API a lot cleaner, albeit with slightly (?) higher memory footprint for apps which only need one or the other. > > > Talking of capabilities, I found their usefulness a bit limited because > > > they a) cannot change during the life time of a client and b) they only > > > report "exists/doesn't exit". > > This is partly fixed, as I faced of change inability of capabilities > too, so the EClient itself is caching capabilities until online state > changes. And how does it know that capability changes cannot occur at some other point in time? That sounds like it might work for the current set of capabilities, but not like a general solution to the problem. > When it changes the client side capabilities cache is purged > and the new set of capabilities is queried on the next access of them. > I do not see any problem in an "exists/doesn't exist" (or better > "capable/incapable") state for this particular thing. They are > capabilities, after all. For a capability like "how many events per calendar do you support" (can be queried for CalDAV, if I remember correctly) a single bit is not enough. > For CalDAV's CTag similarity, it might worth new API, than moving it > exposed on the capability side (I understood you are proposing something > like that), A dedicated API just for CTag would have the problem that I mentioned before: can only be used if the app is compiled against an EDS version which has that call. > but it's usually not supported by all backends, so even > you'll have a possibility, then I believe you'll end with a default > behaviour of returning "something changed" and you'll check items in an > old way, thus no benefit for it. Even if it only works with a few backends it would be an improvement for users of those backends and worthwhile in those cases, in particular if the file backend supports it. -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 19:53 +0200, Milan Crha wrote: > Only the last thing for the enum values, I would personally prefer > something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it > confusion-able with E_CLIENT_TYPE constant. Do you mean E_TYPE_CLIENT (for getting EClientClass's GType)? Regardless, I'm fine with the longer prefix if you'd prefer that. > By the way, the proposed merge of server side parts, it may also involve > merging client side bits (for E*View) and thus finally drop all the old > cruft. It's a benefit, I hope, even with broken backward compatibility. > (I would prefer to break backward compatibility personally, rather than > inventing special names for structs not intersect with old names.) I haven't been following the E*View changes very closely. What's considered cruft? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 12:35 -0400, Matthew Barnes wrote: > This may also be of benefit to Srini and his email-factory branch. I > can imagine extending the enum to include E_CLIENT_TYPE_MAIL_STORE or > something similar. Only the last thing for the enum values, I would personally prefer something with a prefix E_CLIENT_SOURCE_TYPE_... only to not have it confusion-able with E_CLIENT_TYPE constant. After all, the generic open/... functions can be added to libedataserverui, to some e-client-utils.h/c, possibly together with renaming actual e-client-authentication.h/c. Maybe? By the way, the proposed merge of server side parts, it may also involve merging client side bits (for E*View) and thus finally drop all the old cruft. It's a benefit, I hope, even with broken backward compatibility. (I would prefer to break backward compatibility personally, rather than inventing special names for structs not intersect with old names.) Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 14:49 +0200, Patrick Ohly wrote: > Please, let's avoid "E_CLIENT_TYPE_CALENDAR". For people less familiar > with Evolution terminology it is not clear whether this includes tasks. > In Evolution, it doesn't, but in other contexts it does. For example, > Nokia phones store events and tasks in the same "Calendar". In > SyncEvolution I followed the Evolution terminology and "calendar" has > caused a lot of confusion. > > It's not even obvious inside Evolution, because libe*cal*[endar] also > does include tasks and memos. > > What about E_CLIENT_TYPE_EVENTS instead? It hadn't occurred to me the word "calendar" might be ambiguous. I blame the iCalendar spec for overloading it. In the real world, one does not make a "calendar of tasks", one makes a "list of tasks". Maybe this is too Evolution-centric, but to me the container/item relationship is clear: an ADDRESS_BOOK contains CONTACTS a CALENDAR contains EVENTS a TASK_LIST contains TASKS / TODOS a MEMO_LIST contains MEMOS / JOURNALS a MAIL_STOREcontains MESSAGES The enum values should be named consistently. So if we change CALENDAR to EVENTS, I think we should change the rest. FWIW, the new ESource API is already using container names. Key files will have groups named [Address Book], [Calendar], [Task List], etc. instead of [Contacts], [Events], [Tasks], etc. To me it sounds more natural to refer to containers than items, but maybe I'm too indoctrinated in Evolution-speak. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 18:11 +0200, Milan Crha wrote: > Yes, when I was changing server side data-views for book and cal, then > if turned out that the D-Bus API is exactly the same except of the > DBUS_PROXY_NAME and book/cal strings in a file. > > Thus having type param for both factories too, though for book unused, > may clean the code very nicely, forcing us to exactly one implementation > of base EGDBusFactory, EGDBusView, EDataFactory, EDataView and subclass > these to EGDBusBookFactory, EGDBusCalFactory, ... and changing only > really minimum on them. Basically the effort which started Travis > Reitter in his eds branches. Excellent. I'm sold then. Sounds like the right thing to do. This may also be of benefit to Srini and his email-factory branch. I can imagine extending the enum to include E_CLIENT_TYPE_MAIL_STORE or something similar. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:59 -0400, Matthew Barnes wrote: > On Thu, 2011-04-28 at 16:34 +0200, Milan Crha wrote: > > You obviously face of a circular dependency, so it's not doable in a > > clean way. Also because EClient is in libedataserver, EBookClient in > > addressbook/libebook and similarly ECalClient in calendar/libecal. Both > > descendants link to libedataserver... Having another > > register_client/unregister_client function will make things only less > > clear for readers (like if one tries to follow signal handlers by > > reading the code. > > Way to douse me with a bucket of cold water there. :) > > You're right about the library dependencies. That does kinda put a > bullet in unifying the "new" function. I agree a type registration > system is overkill for a mere two GTypes. > > Still, is there any value in having such an enum defined in e-client.h? > > I cited one benefit in consolidating my "get_default_source" functions, > but that alone is not really compelling. Are there other use cases? Yes, when I was changing server side data-views for book and cal, then if turned out that the D-Bus API is exactly the same except of the DBUS_PROXY_NAME and book/cal strings in a file. Thus having type param for both factories too, though for book unused, may clean the code very nicely, forcing us to exactly one implementation of base EGDBusFactory, EGDBusView, EDataFactory, EDataView and subclass these to EGDBusBookFactory, EGDBusCalFactory, ... and changing only really minimum on them. Basically the effort which started Travis Reitter in his eds branches. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 16:34 +0200, Milan Crha wrote: > You obviously face of a circular dependency, so it's not doable in a > clean way. Also because EClient is in libedataserver, EBookClient in > addressbook/libebook and similarly ECalClient in calendar/libecal. Both > descendants link to libedataserver... Having another > register_client/unregister_client function will make things only less > clear for readers (like if one tries to follow signal handlers by > reading the code. Way to douse me with a bucket of cold water there. :) You're right about the library dependencies. That does kinda put a bullet in unifying the "new" function. I agree a type registration system is overkill for a mere two GTypes. Still, is there any value in having such an enum defined in e-client.h? I cited one benefit in consolidating my "get_default_source" functions, but that alone is not really compelling. Are there other use cases? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 16:37 +0200, Milan Crha wrote: > On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > > * "32 bit IDs" - check whether (read) or ensure that (write) IDs > > are 32 bit integers instead of strings, simplifies > > QtContacts-EDS [2]; I have a patch which reduces the size of > > IDs > > from 64 bit to 32 in the file backend, more on that in a > > separate email > > Hi, > I'm against this. It's too limiting. Or maybe elaborate more, what you > expect behind this and what exactly you want to change (really values > for UID properties of vCard and VEVENT/...?). > Bye, > Milan Never mind, I didn't notice your more detailed mail when writing this. I'm sorry. Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > * "32 bit IDs" - check whether (read) or ensure that (write) IDs > are 32 bit integers instead of strings, simplifies > QtContacts-EDS [2]; I have a patch which reduces the size of > IDs > from 64 bit to 32 in the file backend, more on that in a > separate email Hi, I'm against this. It's too limiting. Or maybe elaborate more, what you expect behind this and what exactly you want to change (really values for UID properties of vCard and VEVENT/...?). Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
[please reply to the list only, it makes my life worse when you CC me] On Thu, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote: > It would mean EClient has to know that ECalClient and EBookClient exist > in order to instantiate the right one for a given enum value. Normally > in class design you want to avoid that kind of knowledge seeping into > lower layers, but with the class hierarchy being fixed to these three > classes (four if we add email), I think it's a good tradeoff to have a > more consistent API. > > So it would be something like: > > typedef enum { > E_CLIENT_TYPE_ADDRESS_BOOK, > E_CLIENT_TYPE_CALENDAR, > E_CLIENT_TYPE_MEMO_LIST, > E_CLIENT_TYPE_TASK_LIST > } EClientType; You obviously face of a circular dependency, so it's not doable in a clean way. Also because EClient is in libedataserver, EBookClient in addressbook/libebook and similarly ECalClient in calendar/libecal. Both descendants link to libedataserver... Having another register_client/unregister_client function will make things only less clear for readers (like if one tries to follow signal handlers by reading the code. > > Talking of capabilities, I found their usefulness a bit limited because > > they a) cannot change during the life time of a client and b) they only > > report "exists/doesn't exit". This is partly fixed, as I faced of change inability of capabilities too, so the EClient itself is caching capabilities until online state changes. When it changes the client side capabilities cache is purged and the new set of capabilities is queried on the next access of them. I do not see any problem in an "exists/doesn't exist" (or better "capable/incapable") state for this particular thing. They are capabilities, after all. For CalDAV's CTag similarity, it might worth new API, than moving it exposed on the capability side (I understood you are proposing something like that), but it's usually not supported by all backends, so even you'll have a possibility, then I believe you'll end with a default behaviour of returning "something changed" and you'll check items in an old way, thus no benefit for it. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Do, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote: > On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > > First of all, +1 for rethinking the API. I'd like to suggest that > > besides modernizing the API we also take this opportunity to move more > > of EBook/ECal into a common core. > > > > Opening and listing databases don't have to be separate. Turn > > ECalClientSourceType into EClientSourceType and all of the _new, > > _new_from_uri, _new_system, _new_default, _get_sources functions can be > > moved into e-client.h. > > > > The advantage would be that clients (like SyncEvolution) which work with > > both only need to implement the common operations once. Right now I have > > a lot of duplicate code in the address book and calendar backends. > > That's not a bad idea, actually. > > It would mean EClient has to know that ECalClient and EBookClient exist > in order to instantiate the right one for a given enum value. Normally > in class design you want to avoid that kind of knowledge seeping into > lower layers, but with the class hierarchy being fixed to these three > classes (four if we add email), I think it's a good tradeoff to have a > more consistent API. I agree, it breaks layering, but overall I'd prefer such a unified API. > So it would be something like: > > typedef enum { > E_CLIENT_TYPE_ADDRESS_BOOK, > E_CLIENT_TYPE_CALENDAR, > E_CLIENT_TYPE_MEMO_LIST, > E_CLIENT_TYPE_TASK_LIST > } EClientType; > > I'd prefer *our* terminology in the enum over iCalendar terminology. I > always have to stop and think "okay a memo list is called a VJOURNAL" > and so on. Please, let's avoid "E_CLIENT_TYPE_CALENDAR". For people less familiar with Evolution terminology it is not clear whether this includes tasks. In Evolution, it doesn't, but in other contexts it does. For example, Nokia phones store events and tasks in the same "Calendar". In SyncEvolution I followed the Evolution terminology and "calendar" has caused a lot of confusion. It's not even obvious inside Evolution, because libe*cal*[endar] also does include tasks and memos. What about E_CLIENT_TYPE_EVENTS instead? > > BTW, in this particular example, wouldn't > > e_source_registry_get_default_calendar() need such an enum anyway to > > distinguish between default events, tasks and memos? > > I also wrote: > >e_source_registry_get_default_task_list() >e_source_registry_get_default_memo_list() > > http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html > > So there's four "get_default" functions all together. Hence why having > a single enum definition to cover them all would be nice. Here we have the first misunderstanding because of "calendar" - it wasn't obvious to me that this was exclusively for events ;-} -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 13:12 +0100, David Woodhouse wrote: > As I understand it, an soname bump (from libfoo.so.5 to libfoo.so.6) > should *only* be used for backwards-incompatible changes. Correct. One such example of a backwards-incompatible change is increasing the size of a public GObject class structure for which there exist or may exist subclasses. Hence the practice of reserving X number of pointers at the end of the class struct so X new methods can be added over time without altering the struct size. > If old clients can continue to use the newer library, then shouldn't it > be just a change of *minor* version from libfoo.so.5.2 to libfoo.so.5.3, > and the soname remains the same (libfoo.so.5). Yeah, we don't even do that much right. We usually just leave the soname alone from release to release unless we break compatibility. Blame me for not really groking libtool versioning practices, which just seems unnecessarily complex and confusing to my poor little mind. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > First of all, +1 for rethinking the API. I'd like to suggest that > besides modernizing the API we also take this opportunity to move more > of EBook/ECal into a common core. > > Opening and listing databases don't have to be separate. Turn > ECalClientSourceType into EClientSourceType and all of the _new, > _new_from_uri, _new_system, _new_default, _get_sources functions can be > moved into e-client.h. > > The advantage would be that clients (like SyncEvolution) which work with > both only need to implement the common operations once. Right now I have > a lot of duplicate code in the address book and calendar backends. That's not a bad idea, actually. It would mean EClient has to know that ECalClient and EBookClient exist in order to instantiate the right one for a given enum value. Normally in class design you want to avoid that kind of knowledge seeping into lower layers, but with the class hierarchy being fixed to these three classes (four if we add email), I think it's a good tradeoff to have a more consistent API. So it would be something like: typedef enum { E_CLIENT_TYPE_ADDRESS_BOOK, E_CLIENT_TYPE_CALENDAR, E_CLIENT_TYPE_MEMO_LIST, E_CLIENT_TYPE_TASK_LIST } EClientType; I'd prefer *our* terminology in the enum over iCalendar terminology. I always have to stop and think "okay a memo list is called a VJOURNAL" and so on. > Matthew suggested to replace some of these with > e_source_registry_get_default_calendar/address_book: > > e_cal_client_new_default() > > Instead do: > > source = e_source_registry_get_default_calendar (registry); > client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, > error); > > The same logic would apply here: instead of having separate calls for > calendar and address book, have one call with an enum. Yeah, having one "get_default" function would be desirable. The functions would have to be moved to e-client.h in order to use the enum, but now that I think about it, it kinda makes sense to move them to e-client.h regardless. The default values only come into play when you have to create a client connection to *something*. > BTW, in this particular example, wouldn't > e_source_registry_get_default_calendar() need such an enum anyway to > distinguish between default events, tasks and memos? I also wrote: e_source_registry_get_default_task_list() e_source_registry_get_default_memo_list() http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html So there's four "get_default" functions all together. Hence why having a single enum definition to cover them all would be nice. > Talking of capabilities, I found their usefulness a bit limited because > they a) cannot change during the life time of a client and b) they only > report "exists/doesn't exit". I'll leave it to Milan to address the capabilities stuff. He knows more about it than I. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: > Are you saying that a soname version bump can and should be done in a > backwards-incompatible way (= code compiled against older version no > longer works with new lib)? That's a major pain for people trying to > support multiple distros. Please avoid it whenever possible. As I understand it, an soname bump (from libfoo.so.5 to libfoo.so.6) should *only* be used for backwards-incompatible changes. If old clients can continue to use the newer library, then shouldn't it be just a change of *minor* version from libfoo.so.5.2 to libfoo.so.5.3, and the soname remains the same (libfoo.so.5). -- dwmw2 ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Di, 2011-04-19 at 10:57 -0400, Matthew Barnes wrote: > On Tue, 2011-04-19 at 16:03 +0200, Milan Crha wrote: > > As icaltimezone is the main timezone for calendar in > > evolution-data-server, and will be as long as libical will be used > > there, then what about having e_cal_client_set_default_gtimezone as a > > possible alternative for e_cal_client_set_default_timezone? On the other > > hand, wouldn't e-cal-time-util.h/.c fit better here? > > The fact that we use libical is an implementation detail and the fact > its data types are exposed in the public API is a shame, but we can't do > anything about that right now. To some extend I agree, but I would like to point out that libical, for better or worse, also is the common core in Linux for calendar. This has saved my arse while passing events from libecal to KCal in the (still on-going) "put C++ MeeGo APIs on top of EDS" work [1]. I rely on this implementation detail, please don't take it away. [1] https://meego.gitorious.org/meego-middleware/kcal-eds -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Di, 2011-04-19 at 16:03 +0200, Milan Crha wrote: > On Tue, 2011-04-19 at 09:27 -0400, Matthew Barnes wrote: > > There's a couple other simple things I forgot to mention yesterday. > > > > * We should add some padding to all the public class structs so we can > > add new class methods in the future without breaking ABI. Something > > like this: > > > > struct _ECalClientClass { > > > > ... methods and stuff ... > > > > gpointer reserved[16]; > > }; > > I never understood a need for this, neither used it when changing > structs. There was done a soname version bump when changing public > "class" structures, which, from my point of view, always involves also > API changes, and freezes on these both are tight together. Are you saying that a soname version bump can and should be done in a backwards-incompatible way (= code compiled against older version no longer works with new lib)? That's a major pain for people trying to support multiple distros. Please avoid it whenever possible. -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mo, 2011-04-18 at 15:57 +0200, Milan Crha wrote: > I just created a new branch 'eclient' in eds [1] where is added my work > on the new API which will deprecate EBook/ECal. It is following glib/gio > async pattern and, I believe, makes things more coherent. First of all, +1 for rethinking the API. I'd like to suggest that besides modernizing the API we also take this opportunity to move more of EBook/ECal into a common core. Opening and listing databases don't have to be separate. Turn ECalClientSourceType into EClientSourceType and all of the _new, _new_from_uri, _new_system, _new_default, _get_sources functions can be moved into e-client.h. The advantage would be that clients (like SyncEvolution) which work with both only need to implement the common operations once. Right now I have a lot of duplicate code in the address book and calendar backends. Matthew suggested to replace some of these with e_source_registry_get_default_calendar/address_book: e_cal_client_new_default() Instead do: source = e_source_registry_get_default_calendar (registry); client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error); The same logic would apply here: instead of having separate calls for calendar and address book, have one call with an enum. BTW, in this particular example, wouldn't e_source_registry_get_default_calendar() need such an enum anyway to distinguish between default events, tasks and memos? Matthew already made a similar comment about error codes and "get_capabilities". I agree that these should be common, too. I don't see why get_capabilities needs to be in ecal resp. ebook, except for the convenience wrapper functions which query specific capabilities that only apply to one or the other. Talking of capabilities, I found their usefulness a bit limited because they a) cannot change during the life time of a client and b) they only report "exists/doesn't exit". Their static nature IMHO only has one benefit, which is that they can be safely cached on the client side. I don't see that as very useful, because for those capabilities which are known to be static, the client is likely to only query them once at startup and then set some internal state accordingly. Even for that the API must be asynchronous because of the underlying D-Bus call, as Matthew said. What I'd prefer is a generic key/value mechanism, with value being a string. I prefer a string because it is easy to handle and more complex types (if ever needed) can be mapped to it on a case by case basis. Setting values should also be allowed. That gives us a way how a client can configure the storage to behave in certain ways. This can be per-database (tweak the actual on-disk storage) or per EClient (modify behavior as part of current session). Future use cases for such a key/value mechanism: * "change tag", read-only - a string which changes each time the database changes (same as the CTag in CalDAV), would be used to make change detection in SyncEvolution more efficient [1] * "32 bit IDs" - check whether (read) or ensure that (write) IDs are 32 bit integers instead of strings, simplifies QtContacts-EDS [2]; I have a patch which reduces the size of IDs from 64 bit to 32 in the file backend, more on that in a separate email There can also be convenience functions for this, if they are considered important enough. Only adding such functions and not the generic API would have the downside that code cannot be compiled against an old API implementation and still use the new features if they happen to be available at runtime. At the D-Bus level this could be mapped to properties. [1] http://wiki.meego.com/Architecture/planning/evolution-data-server/eds-improvements#quick_check_for_.22data_changed.22 [2] http://lists.meego.com/pipermail/meego-dev/2011-April/482731.html -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-21 at 08:32 +0200, Milan Crha wrote: > On Tue, 2011-04-19 at 17:00 +0200, Milan Crha wrote: > > On Tue, 2011-04-19 at 19:39 +0530, Chenthill Palanisamy wrote: > > > I would like you to a incorporate some change to the free/busy api. > > > Some servers allow querying free/busy information > > > for multiple users at a time and the results appear in a iterative > > > fashion. The freebusy information of some > > > users are delivered first, while the server keeps fetching other > > > user's free/busy information. So if we could have he > > > FreeBusy api such as this, we could leverage those features, > > > > > > ECalClientFreeBusy > > > e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t > > > start, time_t end, const GSList *users, GCancellable *cancellable, > > > GError **error); > > > (with a async counter-part) > > > > > > Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar > > > *user, GSList *ecalcomps) > > > > > > The clients could watch for the signal and update the freebusy > > > information as and when they receive from eds. > > Hi, > I rethought my thoughts about this and I think I follow your idea more > closely now. If I try to rephrase it, then it might be: > > On the server side, add new >e_data_cal_notify_free_busy_data (..., const GSList *ecalcomps); > which invokes a signal on the D-Bus connection about (partial) result > for a free/busy request (note it doesn't provide user separately). > This signal will be valid only while a get_free_busy request is running. > Calling e_data_cal_notify_free_busy_data() out of these functions will > not fail, but the ECalClient consumer may not expect that being done. > With this limitation we'll have a possibility to cancel pending request, > if needed. The e_data_cal_respond_get_free_busy() will have no return > values. We could term it start_free_busy.. > > On the client side, the 'finish' function will be also void and any > information about free-busy will be available exclusively in a > "free-busy-received" signal, which will have a parameter GSList > *ecalcomps. Once a free_busy_done signal is received, the finish function can be called. Yup and on the whole I agree to what you have mentioned here. Thanks, Chenthill. > > I hope we are on the same line now. If you agree, then I'll make a > change in this way. > Thanks and bye, > Milan > > ___ > evolution-hackers mailing list > evolution-hackers@gnome.org > To change your list options or unsubscribe, visit ... > http://mail.gnome.org/mailman/listinfo/evolution-hackers ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Fri, 2011-04-22 at 13:58 -0400, Matthew Barnes wrote: > The idea though is that what we do now in e-passwords.c -- namely > checking GNOME Keyring for a password and building and displaying our > own password dialog to the user -- is *one possible* implementation of > that simple abstract "get_password" interface that you could pass when > creating EClient objects. > > If MeeGo, for example, wanted to handle authentication in a completely > different way -- perhaps not using GNOME Keyring at all or using their > own authentication widget (I'm just making this up) -- they could write > their own implementation and pass *that* when creating EClient objects, > all without disturbing the public API at all. > > Maybe ECredentialsProvider is a better name after all. I don't know > yet. But does what I'm trying to get at make sense? Yes, I understand this, and it all seems to me pretty similar to the signal thing. By the way, how would you manage this change in MeeGo (or any other system) in evolution itself? Would they patch whole evolution for using their password provider instead of fixing one function in eds? ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Fri, 2011-04-22 at 19:03 +0200, Milan Crha wrote: > Yup, though it'll be (internally - aka in D-Bus) still a signal. This > request of ECredentials object seems strange to me, because I understand > the ECredentials as something which actually holds credentials, not > something what is asking for it something else. Not talking that as a > real object it adds, from my point of view, unnecessary overhead and > complications from simple signal callback. ECredentials might not be the best name for it, and that may be confusing the matter a little. I've been trying to think of a better name. EAuthenticator, EAuthenticationPolicy... nothing so far has really sounded right. I'm open to suggestions. The idea though is that what we do now in e-passwords.c -- namely checking GNOME Keyring for a password and building and displaying our own password dialog to the user -- is *one possible* implementation of that simple abstract "get_password" interface that you could pass when creating EClient objects. If MeeGo, for example, wanted to handle authentication in a completely different way -- perhaps not using GNOME Keyring at all or using their own authentication widget (I'm just making this up) -- they could write their own implementation and pass *that* when creating EClient objects, all without disturbing the public API at all. Maybe ECredentialsProvider is a better name after all. I don't know yet. But does what I'm trying to get at make sense? Maybe it would help if I wrote something like this for Camel, so I could point to something concrete and not be so hand-wavy about it. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-21 at 12:51 -0400, Matthew Barnes wrote: > On Thu, 2011-04-21 at 17:11 +0200, Milan Crha wrote: > > It's technically not passed in this function, it's a callback > > signature. :) It would be used as a signal handler for "auth-required" > > signal in the function, as I think of it right now. > > Yeah I'd like to kill the "auth-required" signal too as I've explained > already. Yup, though it'll be (internally - aka in D-Bus) still a signal. This request of ECredentials object seems strange to me, because I understand the ECredentials as something which actually holds credentials, not something what is asking for it something else. Not talking that as a real object it adds, from my point of view, unnecessary overhead and complications from simple signal callback. Though like with your requested GSList->GList, if you find more people willing to do the change (with a good reason?), then I can add a new object (not ECredentials, as it is used in the backends too), something like ECredentialsProvider, and the e_client_open_new would have it as a parameter instead of auth callback. Bye and have a happy weekend, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-21 at 17:11 +0200, Milan Crha wrote: > It's technically not passed in this function, it's a callback > signature. :) It would be used as a signal handler for "auth-required" > signal in the function, as I think of it right now. Yeah I'd like to kill the "auth-required" signal too as I've explained already. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-21 at 08:34 -0400, Matthew Barnes wrote: > On Thu, 2011-04-21 at 08:41 +0200, Milan Crha wrote: > > void e_client_open_new (ESource *source, gboolean (* auth_cb) (EClient > > *client, ECredentials *credentials, gpointer user_data), gpointer > > user_data); > > > > gboolean e_client_open_new_finish (GAsyncResult *result, EClient > > **client, GError **error); > > I'd would rather ECredentials be an object that gets passed to the "new" > function. It's technically not passed in this function, it's a callback signature. :) It would be used as a signal handler for "auth-required" signal in the function, as I think of it right now. Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Thu, 2011-04-21 at 08:41 +0200, Milan Crha wrote: > void e_client_open_new (ESource *source, gboolean (* auth_cb) (EClient > *client, ECredentials *credentials, gpointer user_data), gpointer > user_data); > > gboolean e_client_open_new_finish (GAsyncResult *result, EClient > **client, GError **error); I'd would rather ECredentials be an object that gets passed to the "new" function. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 13:12 -0400, Matthew Barnes wrote: > Then I can pass that ECredentials object any time I need to create a > new EClient. I just call e_cal_client_new() or e_book_client_new(), > pass the ECredentials object along with some ESource, wait for the > callback, and I'm done. I can start making calls to the calendar or > address book right away. If I'm in offline mode, then certain calls > will error out. That's fine. And if for some the connection needs to > re-authenticate with the server, the EClient already has my > ECredentials object so it can handle everything itself. Hi, what about a compromise here, having (async only): void e_client_open_new (ESource *source, gboolean (* auth_cb) (EClient *client, ECredentials *credentials, gpointer user_data), gpointer user_data); gboolean e_client_open_new_finish (GAsyncResult *result, EClient **client, GError **error); As I mentioned in the previous mail, the ECredentials is used differently, on both client and server sides, and offers much more freedom than yours proposal. And it definitely doesn't block the usage of a different centralized place to get password from, also because there is only one place to change, the libedataserverui/e-client-authenticate.c:e_credentials_authenticate_helper Does it sound good? Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 17:00 +0200, Milan Crha wrote: > On Tue, 2011-04-19 at 19:39 +0530, Chenthill Palanisamy wrote: > > I would like you to a incorporate some change to the free/busy api. > > Some servers allow querying free/busy information > > for multiple users at a time and the results appear in a iterative > > fashion. The freebusy information of some > > users are delivered first, while the server keeps fetching other > > user's free/busy information. So if we could have he > > FreeBusy api such as this, we could leverage those features, > > > > ECalClientFreeBusy > > e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t > > start, time_t end, const GSList *users, GCancellable *cancellable, > > GError **error); > > (with a async counter-part) > > > > Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar > > *user, GSList *ecalcomps) > > > > The clients could watch for the signal and update the freebusy > > information as and when they receive from eds. Hi, I rethought my thoughts about this and I think I follow your idea more closely now. If I try to rephrase it, then it might be: On the server side, add new e_data_cal_notify_free_busy_data (..., const GSList *ecalcomps); which invokes a signal on the D-Bus connection about (partial) result for a free/busy request (note it doesn't provide user separately). This signal will be valid only while a get_free_busy request is running. Calling e_data_cal_notify_free_busy_data() out of these functions will not fail, but the ECalClient consumer may not expect that being done. With this limitation we'll have a possibility to cancel pending request, if needed. The e_data_cal_respond_get_free_busy() will have no return values. On the client side, the 'finish' function will be also void and any information about free-busy will be available exclusively in a "free-busy-received" signal, which will have a parameter GSList *ecalcomps. I hope we are on the same line now. If you agree, then I'll make a change in this way. Thanks and bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 19:02 -0400, Matthew Barnes wrote: > On Tue, 2011-04-19 at 23:25 +0100, David Woodhouse wrote: > > Note the 'gboolean retrying' argument to the libsoup "authenticate" > > signal handler. We probably want to have something similar in the above > > API too, because that's what tells the UI that it needs to ditch the > > existing remembered password and ask for a new one. > > Excellent point, definitely want that. e_credentials_equal() or e_credentials_equal_keys() offers such functionality without a need of an extra parameter in a callback - I do that in the http calendar backend, if you want to look. Maybe it needs a little bit fixing, but such logic may work, mostly. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 23:25 +0100, David Woodhouse wrote: > Note the 'gboolean retrying' argument to the libsoup "authenticate" > signal handler. We probably want to have something similar in the above > API too, because that's what tells the UI that it needs to ditch the > existing remembered password and ask for a new one. Excellent point, definitely want that. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 10:40 -0400, Matthew Barnes wrote: > > 2) ECredentials is way too complex. My intent for ECredentials was to >have a self-contained authentication handling interface to avoid all >the current nonsense with ECal and EBook where you have to always >remember to connect to authentication signals and then implement the >keyring lookup and user password prompting yourself. > >I probably didn't spell this out very well initially, but what I had >in mind was a simple abstract interface to replace "auth-required" >signals. > >struct _ECredentialsInterface { >GTypeInterface parent_interface; > >void(*get_password) (ECredentials *credentials, > EClient *client, > GCancellable *cancellable, > GAsyncReadyCallback callback, > gpointer user_data); > >gchar * (get_password_finish) (ECredentials *credentials, > GAsyncResult *result, > GError **error); >}; Note the 'gboolean retrying' argument to the libsoup "authenticate" signal handler. We probably want to have something similar in the above API too, because that's what tells the UI that it needs to ditch the existing remembered password and ask for a new one. -- David WoodhouseOpen Source Technology Centre david.woodho...@intel.com Intel Corporation ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 18:37 +0200, Milan Crha wrote: > Hmm, I think I understand, but I do not see clearly the point. Sometimes > you do not know if the server requires the authentication, only after > open, which will deliver (on idle) the "auth-required" signal, which can > come before the backend notified "open-done", or after, it depends. What > about offline mode? What about offline server which requires > authentication when evolution is online? (The CalDAV calendar backend > deals with that case, somehow.) Try to think about this from an application author's point of view. All the things you're describing should be handled internally to EBookClient or ECalClient without cluttering up the public API. What I'm trying to do is think about all the things about ECal and EBook that have confused me or have been a pain in the ass to deal with over the years and try and make it easier for application authors like myself. And connection management and authentication is a big pain point with the current API. Right now we store passwords in GNOME Keyring and we build our own password dialog when we need to interact with the user. But that might not always be the case. In fact there's a thread right now over on desktop-devel-list that might change all that really soon. We need an authentication API that's generic enough that it won't break as these technologies and policies change and evolve. As an application author, all I want to have to do is create some ECredentials object that that implements the current authentication policies. I don't really care what those policies are, and I don't want to have to deal with them myself. Then I can pass that ECredentials object any time I need to create a new EClient. I just call e_cal_client_new() or e_book_client_new(), pass the ECredentials object along with some ESource, wait for the callback, and I'm done. I can start making calls to the calendar or address book right away. If I'm in offline mode, then certain calls will error out. That's fine. And if for some the connection needs to re-authenticate with the server, the EClient already has my ECredentials object so it can handle everything itself. That's the experience I'm after. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 10:40 -0400, Matthew Barnes wrote: > 1) There's no need for apps to use numeric IDs to track asynchronous >operations; GCancellable fills that role. GCancellable is the app's >handle to the ongoing operation. If the app wants to cancel an >unfinished operation, it calls g_cancellable_cancel(). > >The following functions should be dropped from the public API: > > e_client_cancel_op() > e_client_register_op() > e_client_unregister_op() > >All functions that kick off an asynchronous operation should return >"void". If you need to use numeric IDs to track D-Bus operations, >do so internally. Create an "e-client-private.h" if you need to. >Don't expose it in the public API. I was afraid of an overuse of the GCancellable, as it is used for the DBus communication and then for the whole operation lifetime too. But you've right, both ways are probably not needed. > 2) ECredentials is way too complex. My intent for ECredentials was to >have a self-contained authentication handling interface to avoid all >the current nonsense with ECal and EBook where you have to always >remember to connect to authentication signals and then implement the >keyring lookup and user password prompting yourself. > > ... >The benefit here is that apps can simply pass some pre-packaged >ECredentials implementation when creating calendar or address book >connections and not have to worry about handling authentication >beyond that. > >The ECredentials API you came up with would be one possible >implementation of the raw ECredentials interface illustrated above. >It should live in libedataserverui or maybe just in Evolution so it >can run a password dialog or whatever other GTK-related things it >may need to do. Please see libedataserverui/e-client-authenticate.*. It does that. The ECredentials is also used in authenticate_user backend implementations, allowing ask for any backend (as you cannot ask for a password for an address book from a contact calendar backend right now), and one is also able to ask for a PIN, as was asked for by the kolab developers. I covered your requirements and (at least) those two above in a simpler way, at least for me. > 3) The "new" functions for EClient classes should be asynchronous, and > ... >I'd really like for establishing a D-Bus connection to the backend, >connecting to a remote server, and authenticating via ECredentials to >be *one* step for the app. It either all works and you get back a >fully connected and authenticated EClient object, or something fails >and you get back NULL with a GError set. > >I want to avoid these weird in-between states where you're holding a >client object but you haven't connected to the backend yet, or you >failed to authenticate with the remoter server, etc. It just makes >life difficult and confusing for apps trying to figure what they're >supposed to do in these cases. > ... Hmm, I think I understand, but I do not see clearly the point. Sometimes you do not know if the server requires the authentication, only after open, which will deliver (on idle) the "auth-required" signal, which can come before the backend notified "open-done", or after, it depends. What about offline mode? What about offline server which requires authentication when evolution is online? (The CalDAV calendar backend deals with that case, somehow.) I agree that invoking operations on backends which are opened but not authenticated yet leads to issues, bugzilla shows "couple" of them, but I seem to miss here something too. Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 16:03 +0200, Milan Crha wrote: > No, not at all, EClient calls descendant's get_capabilities_sync on its > own, when it's needed. The public get_capabilities API on descendants is > sort of redundant in this case. It that case we should have: void e_client_get_capabilities (EClient *client, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); gboolean e_client_get_capabilities_finish (EClient *client, GAsyncResult *result, GList **capabilities, GError **error); gboolean e_client_get_capabilities_sync (EClient *client, GCancellable *cancellable, GList **capabilities, GError **error); and kill the public functions in the subclasses. > I never understood a need for this, neither used it when changing > structs. There was done a soname version bump when changing public > "class" structures, which, from my point of view, always involves also > API changes, and freezes on these both are tight together. If you add a > new member to the struct and not change the "reserved" array size (by > how many, by the way), then you end up with a crash/critical warning > about different structure size anyway. Doing change-and-try loops here > sounds rather silly, from my point of view. The need for this is to add methods to the class structure without breaking ABI and thus avoiding a soname bump. You can go from: struct _ECalClient { ... gpointer reserved[16]; }; to: struct _ECalClient { ... void some_new_method (ECalClient *client, ...); gpointer reserved[15]; }; and avoid forcing apps to be rebuilt. > As icaltimezone is the main timezone for calendar in > evolution-data-server, and will be as long as libical will be used > there, then what about having e_cal_client_set_default_gtimezone as a > possible alternative for e_cal_client_set_default_timezone? On the other > hand, wouldn't e-cal-time-util.h/.c fit better here? The fact that we use libical is an implementation detail and the fact its data types are exposed in the public API is a shame, but we can't do anything about that right now. GTimezone is GNOME's official timezone API now, so that should be reflected in our API. Passing icaltimezones in the public API should be secondary to passing GTimezone once we're able to support both, and I would even be in favor of eventually deprecating APIs that pass icaltimezone. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 19:39 +0530, Chenthill Palanisamy wrote: > I would like you to a incorporate some change to the free/busy api. > Some servers allow querying free/busy information > for multiple users at a time and the results appear in a iterative > fashion. The freebusy information of some > users are delivered first, while the server keeps fetching other > user's free/busy information. So if we could have he > FreeBusy api such as this, we could leverage those features, > > ECalClientFreeBusy > e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t > start, time_t end, const GSList *users, GCancellable *cancellable, > GError **error); > (with a async counter-part) > > Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar > *user, GSList *ecalcomps) > > The clients could watch for the signal and update the freebusy > information as and when they receive from eds. Hi, one more function would be needed, to tell backend from the client that it may stop delivering free/busy information and/or focus on a new query. Otherwise you may get responses really any time, which is not what you actually want. It all seems to me like an ECalView for free/busy, rather than anything doable on the backend client itself. Remember the factory shares backends between data-book/cal-s. With views, all known are notified about changes on objects (if they belong to the view), thus even as a view this will not be that easy on the server side to do, with some hard management of who (EDataCal) is looking for what (different users, time interval...). Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote: > I just created a new branch 'eclient' in eds [1] where is added my work > on the new API which will deprecate EBook/ECal. It is following glib/gio > async pattern and, I believe, makes things more coherent. There's a few other weightier issues in the client-side APIs that I wanted to address separately after getting the trivial stuff out of the way. Three topics: 1) There's no need for apps to use numeric IDs to track asynchronous operations; GCancellable fills that role. GCancellable is the app's handle to the ongoing operation. If the app wants to cancel an unfinished operation, it calls g_cancellable_cancel(). The following functions should be dropped from the public API: e_client_cancel_op() e_client_register_op() e_client_unregister_op() All functions that kick off an asynchronous operation should return "void". If you need to use numeric IDs to track D-Bus operations, do so internally. Create an "e-client-private.h" if you need to. Don't expose it in the public API. 2) ECredentials is way too complex. My intent for ECredentials was to have a self-contained authentication handling interface to avoid all the current nonsense with ECal and EBook where you have to always remember to connect to authentication signals and then implement the keyring lookup and user password prompting yourself. I probably didn't spell this out very well initially, but what I had in mind was a simple abstract interface to replace "auth-required" signals. struct _ECredentialsInterface { GTypeInterface parent_interface; void(*get_password) (ECredentials *credentials, EClient *client, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); gchar * (get_password_finish) (ECredentials *credentials, GAsyncResult *result, GError **error); }; Some kind of object implementing the ECredentials interface would be passed to the EClient as a construct property. Then when EClient gets notified by the backend that authentication is required, it would simply call the get_password() method from an idle callback. Whatever implements the get_password() method should then handle the password lookup and, if necessary, user prompting, all by itself. The benefit here is that apps can simply pass some pre-packaged ECredentials implementation when creating calendar or address book connections and not have to worry about handling authentication beyond that. The ECredentials API you came up with would be one possible implementation of the raw ECredentials interface illustrated above. It should live in libedataserverui or maybe just in Evolution so it can run a password dialog or whatever other GTK-related things it may need to do. 3) The "new" functions for EClient classes should be asynchronous, and I'd like to drop the "open" functions: e_client_open() e_client_open_finish() e_client_open_sync() (Keeping the open() *class method* around might make sense to help make subclassing EClient easier.) I'd really like for establishing a D-Bus connection to the backend, connecting to a remote server, and authenticating via ECredentials to be *one* step for the app. It either all works and you get back a fully connected and authenticated EClient object, or something fails and you get back NULL with a GError set. I want to avoid these weird in-between states where you're holding a client object but you haven't connected to the backend yet, or you failed to authenticate with the remoter server, etc. It just makes life difficult and confusing for apps trying to figure what they're supposed to do in these cases. To that end, EClient should implement the GAsyncInitable interface from GIO, and create instances with g_async_initable_new_async() instead of g_object_new(). http://developer.gnome.org/gio/unstable/GAsyncInitable.html The "new" function for ECalClient should be something like: void e_cal_client_new (ESource *source, ECredentials *credentials, ECalClientSourceType source_type, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); ECalClient * e_cal_client_new_finish (ESource *source, GAsyncResult *result, GError **error); Similarly for EBookClient. ___ evolution-hackers mailing list
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, Apr 18, 2011 at 10:18 PM, Matthew Barnes wrote: > On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote: >> I just created a new branch 'eclient' in eds [1] where is added my work >> on the new API which will deprecate EBook/ECal. It is following glib/gio >> async pattern and, I believe, makes things more coherent. > > Thanks for posting this, Milan. I want to emphasize how important these > new APIs are and ask everyone to look it over and provide comments. > > I had a sneak peek at this over the weekend and jotted some notes. So > far I've only reviewed in detail the client-side headers because that's > what I'm most concerned about getting right. The rest of it can be > tweaked and changed as needed -- even the backend APIs are never truly > frozen. But the client-side APIs *will* be frozen, since that's what > other projects will be migrating to and attempting to use. > > The new client-side APIs are: > > EClient (abstract base class) > http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-client.h?h=eclient > > ECalClient (replaces ECal) > http://git.gnome.org/browse/evolution-data-server/tree/calendar/libecal/e-cal-client.h?h=eclient I would like you to a incorporate some change to the free/busy api. Some servers allow querying free/busy information for multiple users at a time and the results appear in a iterative fashion. The freebusy information of some users are delivered first, while the server keeps fetching other user's free/busy information. So if we could have he FreeBusy api such as this, we could leverage those features, ECalClientFreeBusy e_cal_client_get_freebusy_object_sync (ECalClient *client, time_t start, time_t end, const GSList *users, GCancellable *cancellable, GError **error); (with a async counter-part) Signal: freebusy_received (ECalClientFreeBusy *fbobject, const gchar *user, GSList *ecalcomps) The clients could watch for the signal and update the freebusy information as and when they receive from eds. - Chenthill. > > EBookClient (replaces EBook) > http://git.gnome.org/browse/evolution-data-server/tree/addressbook/libebook/e-book-client.h?h=eclient > > ECredentials (authenication) > http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-credentials.h?h=eclient > > > I'll split my comments into two posts so this doesn't get too > overwhelming. Simple, hopefully non-controversial stuff here and > meatier topics in a separate post. Overall I'm pretty happy with > the APIs. > > > * There's some overlap between the new EClient API and the new > ESource API that I'm working on. Some functions will need to be > dropped once the new ESource API is in place, so I don't know if > you want to do this now or wait. > > ESourceList is being removed so obviously any functions involving > ESourceList will be dropped: > > e_client_util_get_system_source() > e_client_util_set_default() > e_client_util_get_source_for_uri() > e_cal_client_get_sources() > e_book_client_get_sources() > > We will no longer refer ESources using URIs. We will only refer > to ESources by their unique ID string. So the following functions > will be dropped: > > e_client_get_uri() > e_cal_client_new_from_uri() > e_book_client_new_from_uri() > > Default sources will be tracked using the new ESourceRegistry API, > so the following functions will be dropped: > > e_cal_client_set_default() > e_cal_client_set_default_source() > e_book_client_set_default() > e_book_client_set_default_source() > > There's a few functions that I think are too trivial to keep in light > of the lookup capabilities of ESourceRegistry. I'd like to keep the > EClient APIs as slim as possible initially and drop these functions: > > e_cal_client_new_system() > > Instead do: > > source = e_source_registry_lookup_by_uid (registry, "system"); > client = e_cal_client_new (source, source_type, error); > > e_cal_client_new_default() > > Instead do: > > source = e_source_registry_get_default_calendar (registry); > client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error); > > (similar functions exist for tasks and memos) > > e_book_client_new_system_addressbook() > > Instead do: > > source = e_source_registry_lookup_by_uid (registry, "system"); > client = e_book_client_new (source, error); > > e_book_client_new_default_addressbook() > > Instead do: > > source = e_source_registry_get_default_address_book (registry); > client = e_book_client_new (source, error); > > > * We should use GIO error codes whenever possible, and I see quite a bit > of overlap here. I think following error codes should be dropped: > > E_CAL_CLIENT_ERROR_SUCCESS > E_BOOK_CLIENT_ERROR_SUCCESS > > There's no need for an error code for successful operations. > > E_CAL_CLIENT_ERROR_INVALID_ARG > E_BOOK_CLIENT_ERROR_INVALID_ARG > > Use G_IO_ERROR_INVALID_ARGUMENT. > > E_CAL_CL
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 09:27 -0400, Matthew Barnes wrote: > Havoc Pennington kept having to answer this same type of thing in the > early days of GLib/GTK+ when people would ask why the API never uses > "const" in functions that take but don't modify a GObject and GLib data > structure. > > http://mail.gnome.org/archives/gtk-devel-list/2001-May/msg00485.html > > The take away there for me has always been "const" is useless for any > kind of struct that has a pointer member, which virtually all GObjects > do. The API docs are a better place to document that the argument is > not modified. Don't try to do it in the function prototype. I'm not sure if it's an apologize for it, but making it /* const */ may not hurt. > > > * Why do we have "get_capabilities" functions in EClient, ECalClient and > > > EBookClient? Are they different sets of capabilities? And if getting > > > capabilities involves a D-Bus call then doesn't it need to be async > > > everywhere? > > > > They are same capabilities. Same as ECal/EBook the EClient caches > > capabilities once it's asked for them, and reuses them, same as is > > responsible for its memory. The cached values are also used for > > capability checking. These are fetched on demand, and are always > > synchronous. For cases where this is unusable, and where the caller will > > take care of the memory, I added also get_capabilities functions to > > ECalClient/EBookClient, which has both sync and async versions. > > Hmm, so you're saying I first have to fetch the capabilities via the > async calls in ECalClient and EBookClient, then the result is cached in > EClient? I'd suggest renaming the EClient function then to something > like e_client_get_cached_capabilities(). No, not at all, EClient calls descendant's get_capabilities_sync on its own, when it's needed. The public get_capabilities API on descendants is sort of redundant in this case. > There's a couple other simple things I forgot to mention yesterday. > > * We should add some padding to all the public class structs so we can > add new class methods in the future without breaking ABI. Something > like this: > > struct _ECalClientClass { > > ... methods and stuff ... > > gpointer reserved[16]; > }; I never understood a need for this, neither used it when changing structs. There was done a soname version bump when changing public "class" structures, which, from my point of view, always involves also API changes, and freezes on these both are tight together. If you add a new member to the struct and not change the "reserved" array size (by how many, by the way), then you end up with a crash/critical warning about different structure size anyway. Doing change-and-try loops here sounds rather silly, from my point of view. > * Also, GLib 2.26 gained its own timezone API: GTimezone. > > http://developer.gnome.org/glib/unstable/glib-GTimeZone.html > > It would be good to eventually try and correlate GTimezone and > icaltimezone. Other projects will be using GTimezone now since > it's part of the GNOME platform libraries, and will likely expect > to be able to use GTimezone with E-D-S libraries. > > I'd like to add least make room for this in the name space now by > renaming all functions that expose libicaltimezone pointers from > "timezone" to "icaltimezone". As icaltimezone is the main timezone for calendar in evolution-data-server, and will be as long as libical will be used there, then what about having e_cal_client_set_default_gtimezone as a possible alternative for e_cal_client_set_default_timezone? On the other hand, wouldn't e-cal-time-util.h/.c fit better here? Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Tue, 2011-04-19 at 07:58 +0200, Milan Crha wrote: > > * In EBookClient, drop the 'const' qualifier from EContact arguments. > > Trying to use 'const' with GObjects is misguided and pointless. I've > > cursed at EIterator many times for this. > > Yet another thing I wanted to address was a clear memory management > recongnizable from function prototype. Thus if the function doesn't > change object's/variable's content, then I made it 'const' to indicate > it's used for reading only. Of course, inconsistencies all around the > code in this makes it hard to do right, so the function type-casts it > back to non-const pointer internally, because EContact API is not > written in that way. (Though it's not only about EContact.) Havoc Pennington kept having to answer this same type of thing in the early days of GLib/GTK+ when people would ask why the API never uses "const" in functions that take but don't modify a GObject and GLib data structure. http://mail.gnome.org/archives/gtk-devel-list/2001-May/msg00485.html The take away there for me has always been "const" is useless for any kind of struct that has a pointer member, which virtually all GObjects do. The API docs are a better place to document that the argument is not modified. Don't try to do it in the function prototype. > > * Why do we have "get_capabilities" functions in EClient, ECalClient and > > EBookClient? Are they different sets of capabilities? And if getting > > capabilities involves a D-Bus call then doesn't it need to be async > > everywhere? > > They are same capabilities. Same as ECal/EBook the EClient caches > capabilities once it's asked for them, and reuses them, same as is > responsible for its memory. The cached values are also used for > capability checking. These are fetched on demand, and are always > synchronous. For cases where this is unusable, and where the caller will > take care of the memory, I added also get_capabilities functions to > ECalClient/EBookClient, which has both sync and async versions. Hmm, so you're saying I first have to fetch the capabilities via the async calls in ECalClient and EBookClient, then the result is cached in EClient? I'd suggest renaming the EClient function then to something like e_client_get_cached_capabilities(). There's a couple other simple things I forgot to mention yesterday. * We should add some padding to all the public class structs so we can add new class methods in the future without breaking ABI. Something like this: struct _ECalClientClass { ... methods and stuff ... gpointer reserved[16]; }; * Also, GLib 2.26 gained its own timezone API: GTimezone. http://developer.gnome.org/glib/unstable/glib-GTimeZone.html It would be good to eventually try and correlate GTimezone and icaltimezone. Other projects will be using GTimezone now since it's part of the GNOME platform libraries, and will likely expect to be able to use GTimezone with E-D-S libraries. I'd like to add least make room for this in the name space now by renaming all functions that expose libicaltimezone pointers from "timezone" to "icaltimezone". For example, for now we'll have: void e_cal_client_set_default_icaltimezone (ECalClient *client, const icaltimezone *tz); and eventually we can add: void e_cal_client_set_default_timezone (ECalClient *client, GTimezone *timezone); which converts the GTimezone to icaltimezone internally. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, 2011-04-18 at 12:48 -0400, Matthew Barnes wrote: > * There's some overlap between the new EClient API and the new > ESource API that I'm working on. Some functions will need to be > dropped once the new ESource API is in place, so I don't know if > you want to do this now or wait. > > ... Hi, I do not think dropping all of them is any of gain, just the opposite. Why to write same code, even 4-lined, all around instead of call one simple function? Notice that all these functions are hiding where the sources' configuration is stored. (4-lined is because you may get somewhere the "registry" pointer and free it afterwards, instead of your simplified 2-lined sample). There are also simplified functions for freeing GSList-s and their content, which is mostly two-lined code too. > * We should use GIO error codes whenever possible, and I see quite a bit > of overlap here. I think following error codes should be dropped: Well, from my point of view, the error also tells you the place of origin, and the origin for these is not a GIO at all. > E_CAL_CLIENT_ERROR_SUCCESS > E_BOOK_CLIENT_ERROR_SUCCESS > > There's no need for an error code for successful operations. Ah, right, since returning GError instead of error status/code only it doesn't make sense to be there anymore. > ... > E_CAL_CLIENT_ERROR_INVALID_SERVER_VERSION > E_BOOK_CLIENT_ERROR_INVALID_SERVER_VERSION > > I don't think these are necessary anymore since we started > using versioned bus names for the addressbook and calendar > services. If there's a client/server version mismatch, > the client will get a D-Bus error about it. It's used by Groupwise exclusively. > * Of the remaining error codes, a number of them in ECalClient and > EBookClient can be combined and moved to EClient. Especially ones > related to offline and authentication, since that's what EClient > handles. Good idea, I didn't think of it in this way, also because the codes are different when passed through D-Bus, but it can be done on the client side for sure. > * I would really prefer we use GList instead of GSList throughout the > API. GSList is silly and really should never have been added to GLib, > in my opinion. Most modern GNOME APIs that I've seen prefer GList, > and it's a pain in the butt having to convert between the two. Yeah, my other goal was to harmonize internal API to consistently use one of these types, and I chose GSList, because it's simpler and smaller than GList, and because the usual case is to traverse the list in one direction and not both, thus the GSList is sufficient. If there will be more voices for this, then I can make it GList. > * I thought backends could send their own custom error messages now, so > are e_cal_client_error_to_string() and e_book_client_error_to_string() > still necessary? Hmm, maybe it isn't. Consider it as debugging function :) > * In EBookClient, drop the 'const' qualifier from EContact arguments. > Trying to use 'const' with GObjects is misguided and pointless. I've > cursed at EIterator many times for this. Yet another thing I wanted to address was a clear memory management recongnizable from function prototype. Thus if the function doesn't change object's/variable's content, then I made it 'const' to indicate it's used for reading only. Of course, inconsistencies all around the code in this makes it hard to do right, so the function type-casts it back to non-const pointer internally, because EContact API is not written in that way. (Though it's not only about EContact.) > * Why do we have "get_capabilities" functions in EClient, ECalClient and > EBookClient? Are they different sets of capabilities? And if getting > capabilities involves a D-Bus call then doesn't it need to be async > everywhere? They are same capabilities. Same as ECal/EBook the EClient caches capabilities once it's asked for them, and reuses them, same as is responsible for its memory. The cached values are also used for capability checking. These are fetched on demand, and are always synchronous. For cases where this is unusable, and where the caller will take care of the memory, I added also get_capabilities functions to ECalClient/EBookClient, which has both sync and async versions. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote: > I just created a new branch 'eclient' in eds [1] where is added my work > on the new API which will deprecate EBook/ECal. It is following glib/gio > async pattern and, I believe, makes things more coherent. Thanks for posting this, Milan. I want to emphasize how important these new APIs are and ask everyone to look it over and provide comments. I had a sneak peek at this over the weekend and jotted some notes. So far I've only reviewed in detail the client-side headers because that's what I'm most concerned about getting right. The rest of it can be tweaked and changed as needed -- even the backend APIs are never truly frozen. But the client-side APIs *will* be frozen, since that's what other projects will be migrating to and attempting to use. The new client-side APIs are: EClient (abstract base class) http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-client.h?h=eclient ECalClient (replaces ECal) http://git.gnome.org/browse/evolution-data-server/tree/calendar/libecal/e-cal-client.h?h=eclient EBookClient (replaces EBook) http://git.gnome.org/browse/evolution-data-server/tree/addressbook/libebook/e-book-client.h?h=eclient ECredentials (authenication) http://git.gnome.org/browse/evolution-data-server/tree/libedataserver/e-credentials.h?h=eclient I'll split my comments into two posts so this doesn't get too overwhelming. Simple, hopefully non-controversial stuff here and meatier topics in a separate post. Overall I'm pretty happy with the APIs. * There's some overlap between the new EClient API and the new ESource API that I'm working on. Some functions will need to be dropped once the new ESource API is in place, so I don't know if you want to do this now or wait. ESourceList is being removed so obviously any functions involving ESourceList will be dropped: e_client_util_get_system_source() e_client_util_set_default() e_client_util_get_source_for_uri() e_cal_client_get_sources() e_book_client_get_sources() We will no longer refer ESources using URIs. We will only refer to ESources by their unique ID string. So the following functions will be dropped: e_client_get_uri() e_cal_client_new_from_uri() e_book_client_new_from_uri() Default sources will be tracked using the new ESourceRegistry API, so the following functions will be dropped: e_cal_client_set_default() e_cal_client_set_default_source() e_book_client_set_default() e_book_client_set_default_source() There's a few functions that I think are too trivial to keep in light of the lookup capabilities of ESourceRegistry. I'd like to keep the EClient APIs as slim as possible initially and drop these functions: e_cal_client_new_system() Instead do: source = e_source_registry_lookup_by_uid (registry, "system"); client = e_cal_client_new (source, source_type, error); e_cal_client_new_default() Instead do: source = e_source_registry_get_default_calendar (registry); client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error); (similar functions exist for tasks and memos) e_book_client_new_system_addressbook() Instead do: source = e_source_registry_lookup_by_uid (registry, "system"); client = e_book_client_new (source, error); e_book_client_new_default_addressbook() Instead do: source = e_source_registry_get_default_address_book (registry); client = e_book_client_new (source, error); * We should use GIO error codes whenever possible, and I see quite a bit of overlap here. I think following error codes should be dropped: E_CAL_CLIENT_ERROR_SUCCESS E_BOOK_CLIENT_ERROR_SUCCESS There's no need for an error code for successful operations. E_CAL_CLIENT_ERROR_INVALID_ARG E_BOOK_CLIENT_ERROR_INVALID_ARG Use G_IO_ERROR_INVALID_ARGUMENT. E_CAL_CLIENT_ERROR_BUSY E_BOOK_CLIENT_ERROR_BUSY Use G_IO_ERROR_BUSY. E_CAL_CLIENT_ERROR_PERMISSION_DENIED E_BOOK_CLIENT_ERROR_PERMISSION_DENIED Use G_IO_ERROR_PERMISSION_DENIED. E_CAL_CLIENT_ERROR_NOT_SUPPORTED E_CAL_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED E_BOOK_CLIENT_ERROR_NOT_SUPPORTED E_BOOK_CLIENT_ERROR_PROTOCOL_NOT_SUPPORTED Use G_IO_ERROR_NOT_SUPPORTED. E_CAL_CLIENT_ERROR_CANCELLED E_BOOK_CLIENT_ERROR_CANCELLED Use G_IO_ERROR_CANCELLED. E_CAL_CLIENT_ERROR_INVALID_SERVER_VERSION E_BOOK_CLIENT_ERROR_INVALID_SERVER_VERSION I don't think these are necessary anymore since we started using versioned bus names for the addressbook and calendar services. If there's a client/server version mismatch, the client will get a D-Bus error about it. E_CAL_CLIENT_ERROR_DBUS_ERROR E_BOOK_CLIENT_ERROR_DBUS_ERROR I expect GDBus will handle D-Bus errors and hand us back a G_IO_ERROR_DBUS_ERROR. E_
Re: [Evolution-hackers] New 'eclient' branch in eds
On Mon, 2011-04-18 at 15:36 +0100, Philip Withnall wrote: > I haven't had a chance to look at it all in detail, but two things > strike me from a quick glance: > > • If we're following the GIO async pattern, why do the > e_data_book_respond_*() functions still exist? Hi, the server part (e-data*) didn't change much, not with respect of API changes, apart of dropping outdated functions. The part which changed was the client side. The 'respond' functions are there to tell the client, from the server, that the certain operation was finished. What I forgot to mention in the previous mail is that the "asynchronous" server operation have been divided into two parts, the first is to start the operation by calling particular function on the client side, and then notifying the client from the server that this operation was finished. It's done by a "done" signal for the particular operation. It avoids timeouts on DBus, as long as the main thread isn't stuck, because when an operation is started, then the first thing the EDataBook/Cal does is a response to the client with an operation ID, (this ID can be used to cancel this particular operation). Only after that is the function run and is considered running until backend finishes it by the 'respond' function. The only disadvantage I faced with this approach is that the _sync client calls are supposed to run their own mainloop, to behave like blocking, but still being able to receive the done notification, when invoked from the main thread. > • Please, please, please add some documentation to the new EDataBook. > Trying to understand how the old one was supposed to function was a > nightmare, and I would hope that the same mistake isn't repeated with > the shiny new version. Yeah, can be added any time later, though you may use EBookBackend documentation, rather than a wrapper around dbus and factory, for which the EDataBook is. Bye, Milan ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] New 'eclient' branch in eds
Hey, I haven't had a chance to look at it all in detail, but two things strike me from a quick glance: • If we're following the GIO async pattern, why do the e_data_book_respond_*() functions still exist? • Please, please, please add some documentation to the new EDataBook. Trying to understand how the old one was supposed to function was a nightmare, and I would hope that the same mistake isn't repeated with the shiny new version. Thanks for working on this (and porting the Google Contacts backend!). Philip On Mon, 2011-04-18 at 15:57 +0200, Milan Crha wrote: > Hi, > I just created a new branch 'eclient' in eds [1] where is added my work > on the new API which will deprecate EBook/ECal. It is following glib/gio > async pattern and, I believe, makes things more coherent. > > This change, apart of other things, influences also backends, as I added > GCancellable to each "virtual" function which deserved it, made > ECalBackend authentication process similar to that used on > EBookBackend-s (with authenticate_user function), and I dropped unused > and/or unnecessary "virtual" functions from backends as well (mostly > from ECalBackend). > > The GDBus interface functions were also completely rewritten, for better > readability, I believe (and hope). > > Please have a look and comment here, before I'll commit this to git > master, which I would like to do before the first 3.1 release of eds, > thus all other descendants of backends will have enough time to change > their code appropriately. > > The next step, after such commit, will be to slowly adapt evolution > itself, with which I would prefer to wait till Matt's account management > changes will land, definitely on places which are interleaving, because > I would like to avoid most of the pain when merging changes, unless > we'll make other deal on this. > > Bye, > Milan > > [1] http://git.gnome.org/browse/evolution-data-server/log/?h=eclient > > ___ > evolution-hackers mailing list > evolution-hackers@gnome.org > To change your list options or unsubscribe, visit ... > http://mail.gnome.org/mailman/listinfo/evolution-hackers signature.asc Description: This is a digitally signed message part ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] New 'eclient' branch in eds
Hi, I just created a new branch 'eclient' in eds [1] where is added my work on the new API which will deprecate EBook/ECal. It is following glib/gio async pattern and, I believe, makes things more coherent. This change, apart of other things, influences also backends, as I added GCancellable to each "virtual" function which deserved it, made ECalBackend authentication process similar to that used on EBookBackend-s (with authenticate_user function), and I dropped unused and/or unnecessary "virtual" functions from backends as well (mostly from ECalBackend). The GDBus interface functions were also completely rewritten, for better readability, I believe (and hope). Please have a look and comment here, before I'll commit this to git master, which I would like to do before the first 3.1 release of eds, thus all other descendants of backends will have enough time to change their code appropriately. The next step, after such commit, will be to slowly adapt evolution itself, with which I would prefer to wait till Matt's account management changes will land, definitely on places which are interleaving, because I would like to avoid most of the pain when merging changes, unless we'll make other deal on this. Bye, Milan [1] http://git.gnome.org/browse/evolution-data-server/log/?h=eclient ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... http://mail.gnome.org/mailman/listinfo/evolution-hackers