Re: [Evolution-hackers] Move authentication of backends back to the client (3.13.90)
On Thu, 2015-02-19 at 07:43 +0100, Milan Crha wrote: > On Wed, 2015-02-18 at 13:54 +0100, Patrick Ohly wrote: > > What I would prefer instead of the additional int parameter is a > > string->variant hash with a list of keys which can be extended in > > the future without breaking the API. Old clients not passing enough > > information then can either use reasonable defaults (when possible) > > or get an error telling the user to get his software updated. > > Hi, > I would not do that this way. It would be horrible to call the > function and create extra arguments for it in some sort of array and > variants and so on. It's a pita to convert parameters forth and back > when passing them through GDBus already, thus rather not add the same > burden to the client functions too. I'm not convinced, but it's your project. However, I reserve the right to say "told you so" once the API needs to be changed the next time. > > I think that's better than causing old software unconditionally to > > not compile (due to a API change) or to not run (due to an > > ABI/soname change), because it keeps the option open to run some old > > software > > unchanged. > > I always understood that it's the soname version which is meant to > cover these situations. Not that the two versions of eds can be > installed in parallel in one prefix. But distro's typically don't do that because it's extra work to maintain two versions of the same software in parallel. It also would not work for EDS, because each client library is typically tightly coupled with a certain daemon version, and those cannot be installed or run in parallel. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Move authentication of backends back to the client (3.13.90)
On Mon, 2015-02-02 at 17:25 +0100, Milan Crha wrote: > Hello, > I just committed a change, for 3.13.90 development version, into > evolution-data-server [1], evolution [2], evolution-ews [3] and > evolution-mapi [4], which moves authentication back to the clients. As discussed with Milan on IRC, I was worried that this change would break compatibility with SyncEvolution although SyncEvolution primarily (exclusively?!) is used with local storage, which works without authentication. I now looked further and found that SyncEvolution already works with the new EDS, despite the changed API (e_book_client_connect, e_cal_client_connect). That's because SyncEvolution opens an ESource with e_client_open(), which hasn't changed. SyncEvolution obviously won't work for sources which require authentication, but I don't think that'll be a big issue. [...] I have not looked at the concept in detail, but it looks reasonable at first glance. > The other follow-up work will be to adapt any "clients" and libraries > which might be affected by this change. I'd like to help as much as > I'll be able, thus if there will be any issues spotted, feel free to > ping me or drop me an email and I'll help you to migrate your code. I > also removed all the old authentication code and functions, to avoid > those system-modal prompts and basically all the old behavior users > didn't like, the same as to cleanup API as much as possible. > Bye, > Milan > > [1] > https://git.gnome.org/browse/evolution-data-server/commit/?id=884fb8d872787d9 One comment that I already made on IRC: adding one additional int parameter to e_book_client_connect and e_cal_client_connect falls a bit short IMHO. I bet there will be some point in the future when some other new parameter will also be needed, causing another API break which could be avoided now. What I would prefer instead of the additional int parameter is a string->variant hash with a list of keys which can be extended in the future without breaking the API. Old clients not passing enough information then can either use reasonable defaults (when possible) or get an error telling the user to get his software updated. I think that's better than causing old software unconditionally to not compile (due to a API change) or to not run (due to an ABI/soname change), because it keeps the option open to run some old software unchanged. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Refactoring contact editor
On Thu, 2014-09-04 at 18:16 +0200, schaarsc wrote: > Am Mittwoch, den 03.09.2014, 18:14 +0200 schrieb Milan Crha: > > it sounds like Patrick Ohly's "[Evolution-hackers] custom labels" > > discussion [1] from this year's spring. That's much more work than > > just replace custom widgets with a stock expander. > > besides of being more work than just removing code... > From my point of view there is still the unsolved issue on where to > store the custom label. > I'd rather like to see this standardized first instead of using > X-ABLABEL, You mean, an official "ITEMLABEL" property (note that LABEL is taken in vCard) in a future vCard RFC? While that would be useful, I wouldn't hold my breath while waiting for it ;-) > for example by allowing free text in TYPE or by adding a new > type (TYPE=CUSTOM;CTYPE=my-label) Using a parameter instead of a property has the problem of not being able to store arbitrary text. The MIME DIR spec simply doesn't support it at the moment. See the earlier mail thread referenced by Milan about this - as I said there, my conclusion was that the limitations probably are acceptable for the kind of values entered by users, but I am open for other suggestions. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Refactoring contact editor
On Wed, 2014-09-03 at 18:14 +0200, Milan Crha wrote: > On Wed, 2014-09-03 at 12:55 -0300, Tristan Van Berkom wrote: > > While we're here, do you think it would be a good idea to > > migrate away from the hard coded list of vcard labels/ids ? > > > > If you're going to redo the UI anyway, it might be a good > > idea to move towards something more flexible, with UI > > functionalities such as: > > o Add a new email/phonenumber/address > > - When adding a new phone number, let the > > user enter their own label, a user may have > > one or more "HOME" phone numbers, etc > > o Delete an email/phonenumber/address > > o View all emails/phonenumbers/addresses regardless > > of whether they are in any hard coded list of labels > > > > With the possibility of synchronizing your addressbook with > > another addressbook (on your mobile or wherever), it would > > be interesting to support the data which can be found in > > a given vCard that may not have been created by evolution, > > in which case it becomes important to read the whole vCard > > for what it is. > > > > However, I suppose there might be localization issues with > > this, I suppose the hard coded fields that evolution lets > > you edit have translatable labels in the UI, perhaps there > > is some not-too-difficult way to allow both (to be honest, > > it's possible that evolution at least lets you view all > > of the emails/phonenumbers/addresses, I'm not sure, but > > at least allowing that would be worthwhile). > > > > Hi, > it sounds like Patrick Ohly's "[Evolution-hackers] custom labels" > discussion [1] from this year's spring. That's much more work than > just replace custom widgets with a stock expander. That could be part of it. The more important aspect (to me at least) is the increased flexibility (no longer limited to a fixed number of telephone numbers/emails/etc.) and easier access to information. In most cases, all data will fit into one page, while at the moment one has to switch between different tabs to see everything. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] custom labels
On Mon, 2014-05-26 at 07:38 +0200, Milan Crha wrote: > On Sun, 2014-05-25 at 21:41 +0200, Patrick Ohly wrote: > > Instead if just the pre-defined "Work", "Home", "Other", etc., the user > > can also enter arbitrary text. For example, instead of "Other Tel: foo" > > the user can enter "Vacation Tel: bar" for a telephone number that is to > > be used when that contact is in his vacation home. > > > > It's even more important for dates. There might be a lot more dates to > > be stored than just birthday and anniversary, so a repeating X-ABDate > > property is used with custom labels to allow that. Same for related > > persons. > > Hi, > aha, I see, so it's more like TYPE parameter, with more fine-grained > values. Would it be too complicated to use TYPE instead? So you mean something like TYPE=X-? The problem would be to distinguish between "X-FOOBAR as a TYPE extensions" and "X-FOOBAR as user entered text". X-ABLabel avoids that by introducing a new parameter. > It'll be > slightly complicated for evolution's UI, but probably easier than > dealing with it in a separate widget. I'm not sure whether this is easier for the UI. The whole EContact API deals poorly with arbitrary number of repeating properties. For some properties one can iterate over all of them (E_CONTACT_TEL), for others one can't (E_CONTACT_ADDRESS is documented as a single EContactAddress? Might be a documentation bug). For IM information it's definitely not possible to access arbitrary chat handles; the API only supports those for which a E_CONTACT_ADDRESS_ exists. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] custom labels
On Fri, 2014-05-23 at 15:58 +0200, Milan Crha wrote: > On Fri, 2014-05-23 at 14:56 +0200, Patrick Ohly wrote: > > I went ahead with the X-ABLabel as parameter approach. > > Hi, > I'm sorry you didn't get any answer for this thread. I always forgot of > it, also due to not having much opinion on the subject. > > I still do not fully understand what are the custom labels for. Is it a > per-attribute property or a per-contact property? It feels like > categories, if the later. What are the labels used for in UI? Instead if just the pre-defined "Work", "Home", "Other", etc., the user can also enter arbitrary text. For example, instead of "Other Tel: foo" the user can enter "Vacation Tel: bar" for a telephone number that is to be used when that contact is in his vacation home. It's even more important for dates. There might be a lot more dates to be stored than just birthday and anniversary, so a repeating X-ABDate property is used with custom labels to allow that. Same for related persons. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] custom labels
On Fri, 2014-05-02 at 10:51 +0200, Patrick Ohly wrote: > I'm still undecided. A too complex X-ABLabel property value could be > simplified to store it in a X-ABLabel parameter, but that'll drop user > data. I went ahead with the X-ABLabel as parameter approach. My gut feeling is that this will be easier to support in EDS and Evolution than connecting two different properties. The full flexibility of a property is not needed for labels; for example, line breaks can be entered neither on an iPad nor in Google Contacts. SyncEvolution >= 1.4.99.2 will start to copy custom labels into X-ABLabel when syncing with a CardDAV server. As mentioned before, Evolution currently drops the parameter extension. I've created a bug for that: https://bugzilla.gnome.org/show_bug.cgi?id=730636 Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Introspection enablement for libecal - huge changes needed?
On Fri, 2014-05-16 at 19:28 +0200, Milan Crha wrote: > The main problem is that the API returns pointers which are part of the > structure that you ask the value of it - like when you ask for a > subcomponent of an icalcomponent. If it exists, you get a child of the > parent component. This makes a disaster due to the unpredictable memory > management in python or javascript (introspection-using languages in > general?). The libecal uses this libical behaviour too. As an example, > if you ask for an alarm of an ECalComponent, then you receive a new > structure which holds a libical structure, which is part of the > ECalComponent (icalcomponent of it). Any changes done through this > structure are immediately propagated into the parent's ECalComponent, > aka there is no "save" involved. If you free ECalComponent before the > alarm structure, then the free of the alarm structure causes a crash. As > you cannot influence the memory-free time in python... and even if it > would be possible, then we cannot expect from the introspection > interface users (developers) to tweak their code in a way they are not > used to (like making some memory/variable management on their own). Aren't you going to run into the same problem with a GObject-based proxies for these libical objects? The proxies are reference-counted, the libical objects are not, so they may go away before their proxies do. This would leave the proxy with a dangling pointer or (if it somehow tracks the lifetime of the owner of the object) in a state where it is unusable. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Introspection enablement for libecal - huge changes needed?
On Wed, 2014-05-14 at 15:34 +0200, Milan Crha wrote: > Hello, > maybe you noticed that we have a GSoC project for this year, to enable > introspection for calendar part of evolution-data-server (libecal), > which will make it usable for other languages as well. As said already, doing this as add-on of libical and then later taking advantage of it in EDS seems like the right approach. > Will is a student which will do the main work. The current problem is > libical, its icalcomponent, enums and all other structures. I thought > that we will be able to introspect this with simple boxed types, but it > doesn't seem to be possible, thus the only option I can see is to > massively change API of the calendar and define proxies for libical > structures and enums. These proxies would be fully GObject-based, which > might be a plus, I hope. > > I would, for example, define a GObject ICalComponent, which would have > its i_cal_component_* API functions, mimic all the icalcomponent API, > hiding the icalcomponent structure in the background. Would it be possible to keep the current EDS API around and just add the new introspectable API? > Such change will be huge, but more importantly it'll be an earthquake > for anything using libecal (the ECalComponent would not use > icalcomponent anymore, it would start using ICalComponent instead). ECalComponent would use a proxy of the original icalcomponent, right? If the introspection layer in libical has a way to convert to and from the original icalcomponent and the introspectable proxy, then ECalComponent could convert back and forth as needed, depending on which API the EDS client uses. > Because of this earthquake, I would like to hear from others their > opinion. Maybe we overlooked some option in introspection (there is > preferred to create introspection based on code annotations, not to > define them by hand), but I'm afraid the most of the projects like > SyncEvolution, whether it'll be able to handle such change gracefully. I've made my peace with ABI and API changes in EDS ;-} I'm not happy about them, but I can handle them with a combination of ifdefs and compilation of backend shared objects on different Linux distros. Just give me enough and explicit warning beforehand. If an API change can't be avoided, then don't let SyncEvolution hold you back. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] custom labels
On Fri, 2014-04-11 at 12:18 +0200, Patrick Ohly wrote: > Hello! > > Both Google and Apple support custom labels for basically all of a > contacts properties (telephone, email, address, instant messaging, etc). > They use group tags to associate the extra label with the property: > > item4.ADR:;;custom address > item4.X-ABLabel:custom-label > > Does anyone have suggestions for how this could and/or should be handled > by EDS and Evolution? > > Obviously the Evolution UI would need to be modified to actually show > the information. As a first step I would already be happy if the > information didn't get lost during a import/edit/export cycle. > > I tried grouping, but when editing the field (an EMAIL in my test) its > group tag was removed, thus breaking the connection to the custom label. > After editing in Evolution: > > item1.X-ABLabel:foobar > EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:email value modified > > I also tried a custom parameter, but that also got lost: > > EMAIL;X-ABLabel=foobar;X-EVOLUTION-UI-SLOT=1;TYPE=WORK:email value > -> > EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:email value modified > > I tried this with Evolution 3.4 (the one shipped with my current > desktop, Debian Stable). Has perhaps anything been done regarding this > in later versions? > > I am undecided about what EDS and Evolution should use internally to > represent custom labels. I lean towards a custom parameter because > although grouping is supported by EVCard, it hasn't been really used, > has it? The concept of "fields of interest" would still work if the > field also has the custom label as parameter (not that we have special > support for anything other than UID and REV, but at least conceptually > something else could also be supported). The custom parameter has one major drawback: it cannot store arbitrary string values. Line breaks and double quotes are not possible. The EDS vCard parser is also slightly broken and fails for X-ABRELATEDNAMES;X-ABLabel=domestic partner:domestic partner A space without quoting is valid according to http://tools.ietf.org/html/rfc2425#page-5 but e-vcard.c complains "invalid character found in parameter spec" and proceeds at the next line. > On the other hand, deviating from the format of the peers will make > interoperability harder. I'm not sure yet how I would do the conversion > in SyncEvolution between X-ABLabel as parameter and X-ABLabel as > property with group. Importing/exporting vCards manually also is > affected by the choice of the internal format. Even if we used X-ABLabel + grouping, directly exchanging vCards with Apple will still not work properly due to the X-JABBER/AIM/... vs IMPP difference. Further work would be needed either way. I have not tested whether Apple understands X-AIM/JABBER/... instead of IMPP. I'm still undecided. A too complex X-ABLabel property value could be simplified to store it in a X-ABLabel parameter, but that'll drop user data. At this point it really would be good to get feedback from others. Is using groups going to be possible with EContact and Evolution or are we forced to use the simpler parameter approach, despite its limitations? Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Contacts database modifications history
On Thu, 2014-04-24 at 13:51 +, Potrola, MateuszX wrote: > > On Wed, 2014-04-23 at 16:08 -0400, Tristan Van Berkom wrote: > > > This is my prerogative and I can accept that it is not shared with the > > > maintainers of EDS, nevertheless I would still like to caution against > > > opening up the SQL tables owned by EBookSqlite to be shared with > > > plugins (it may take a little more time in the beginning to flesh out > > > a well defined interactive API that satisfies the needs of plugins, > > > but this will pay off in better long term stability, anyway this is my > > > opinion). > > > > I agree with you, plugins should not make assumptions about the content of > > sqlite database. This needs to be documented clearly. > > > > However, the goal is not to let plugins access the existing tables. > > Instead, the goal is to let plugins create their own custom tables and > > update > > them as part of the same transaction that EBookSqlite uses to update its own > > tables. Name clashes need to be avoided, but that is a minor issue. We might > > also want to add hooks for DB updates and locale changes. > I think that we can still create solution where internal tables from > EBookSqlite will be protected from accessing from plugins and all > queries will be run in the same transaction (which is most important > part for us). > Shared transaction across databases may be possible by using sqlite > ATTACH command. Plugins can create their own databases and ask > EBookSqlite to attach them to the one used by EBookSqlite, providing > some names that will identify them (this name will have to be > prepended to each table name when making query). For this to work we still need to grant the plugin access to the sqlite connection of the EBookSqlite database. All that we gain is that sqlite helps a bit with resolving table name conflicts and that data gets separated into more files (not necessarily an advantage). Note that we still need to be careful about name conflicts, because sqlite will pick the most recently attached table first if the table name is not unique (http://www.sqlite.org/draft/lang_attach.html). In other words, a plugin that happens to pick the same name as core EBookSqlite will cause EBookSqlite to use the plugin's table instead. We also loose the option to turn on WAL (http://www.sqlite.org/draft/wal.html) because atomicity is not guaranteed for attached databases. EDS might not use WAL at the moment, but it would be an interesting exercise to run benchmarks with it enabled. Bottom line is that I don't see ATTACH as a viable solution for the problem, in contrast to clearly documenting the table naming convention and the boundary between EBookSqlite and plugin. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Contacts database modifications history
On Wed, 2014-04-23 at 16:08 -0400, Tristan Van Berkom wrote: > This is my prerogative and I can accept that it is not shared with the > maintainers of EDS, nevertheless I would still like to caution against > opening up the SQL tables owned by EBookSqlite to be shared with plugins > (it may take a little more time in the beginning to flesh out a well > defined interactive API that satisfies the needs of plugins, but this > will pay off in better long term stability, anyway this is my opinion). I agree with you, plugins should not make assumptions about the content of sqlite database. This needs to be documented clearly. However, the goal is not to let plugins access the existing tables. Instead, the goal is to let plugins create their own custom tables and update them as part of the same transaction that EBookSqlite uses to update its own tables. Name clashes need to be avoided, but that is a minor issue. We might also want to add hooks for DB updates and locale changes. Does this address your concern? Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Contacts database modifications history
On Wed, 2014-04-16 at 12:23 +0200, Milan Crha wrote: > On Tue, 2014-04-15 at 13:16 +, Potrola, MateuszX wrote: > > I would like to have ability to receive some kind of notifications (or > > store information in some additional database) about modifications > > made to contacts database during synchronization. > > > Hi, > the compose of a "diff" for done changes is probably the hardest thing > on this whole thing you'd like to achieve. There is always (at least) > one place where this "diff" should be computed. I'm not much willing to > add it into evolution-data-server itself, because it has no good use for > it (at least not as of now). I tend to agree. It would be very hard to come up with a general purpose diff that works across a wider range of use cases. If it is configurable, the config language and implementation would have to be unnecessarily flexible (and thus complicated) for what Mateusz probably has in mind. I still think a plugin doing the diff would be okay. > As Patrick pointed out, the previous implementation had sever issues > which made the feature basically unusable. If we cannot make it done for > all backends, then it probably might not be part of the > evolution-data-server. We have a slightly different understanding of EDS here. You see EDS as an abstraction layer that offers just the functionality that can be implemented with all backends. We focus on EDS + local storage and try to make the best use of its capabilities, even if that means adding things to it which won't work with other backends. That's find, I think we can do both. It just means that whatever API gets added that only works with local storage needs to be clearly marked as "local only" in the documentation and/or there has to be a discovery mechanism for determining whether a certain EBookClient instance supports it. > Anyway, I would prefer if you could create a plugin to SyncEvolution > (does it support plugins?), also because it's the software you are > targeting, not evolution-data-server code, Not necessarily. At the moment the focus is on cached PBAP address books that are only written by SyncEvolution, but that may change in the future. Perhaps someone wants to rip out SyncEvolution and do the caching differently, or wants to have address books that can be edited by the user. The change tracking still needs to work in those cases. Another argument for doing it inside EDS is that it can be combined with the actual sqlite database update. To me, that is the only bullet-proof way of ensuring that the change tracking information will be consistent with the actual content. This must work in a car, where the head unit can loose power at any moment. Ensuring that the sqlite database doesn't get corrupted is difficult enough. Ensuring that different processes get a chance to write different files will be even harder. I'd like to mention further use case for allowing hooks during the execution of EDS writes: this can be used to pre-compute data that is later going to be shown by a UI other than Evolution. I think EDS already does some of this for Evolution (like computing a full name) in a way that, strictly speaking, depends on the UI that is going to show the data. One even crazier idea was to maintain an on-disk data structure that can be memory-mapped to access contact data, including strings and their parsed photos ready to be displayed, without going through sqlite and all the string handling. I'm not a terribly big fan of that idea myself, but if there were hooks in EDS which could be used to track DB changes reliably, that would allow implementing this idea without further changes to EDS itself. > Still, all this looks too complicated, adding unnecessary burden, > consider that you specifically target SyncEvolution changes, thus why > not to extend SyncEvolution to write its own backlog of done changes in > some place, where your application may just read them from? SyncEvolution indeed does a read/update/write cycle when updating contacts. This would be a natural place to add a hook for such a plugin. Implementing it will be a bit tricky because it happens under the hood in the Synthesis sync engine, but I guess it could be done, if the alternative (a similar hook in EDS) is not acceptable. > On Wed, 2014-04-16 at 09:02 +, Potrola, MateuszX wrote: > > What is the probability of failure of emitting signals ? > > The probability is close to 0. If it happens, then it's a bug in the > backend (or the related code) and should be fixed as soon as discovered. The more likely failure is that EDS emits the change, but then the computer is shut down before the receiving side gets a chance to handle it => EDS DB was modified, change log in the receiver was not. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hac
Re: [Evolution-hackers] Contacts database modifications history
On Wed, 2014-04-16 at 09:02 +, Potrola, MateuszX wrote: > > > First one is to create new address book backend (which will be > > > inhering from file backend used by Syncevolution) and add there logic > > > for calculating modifications when creating/modifying/removing > > > contacts. These modifications will be stored in additional database, > > > so some additional functions for handling this will need to be added > > > to EBookSqlite class. > > > > > > Here are some pros and cons that I can see: > > > > > > Pros: > > > > > > - If someone don’t want to keep track of modifications history > > > he can still use file backend > > > > > > - We can be sure that contacts database and modifications > > > database will be in sync, by using attach function of sqlite > > > (databases will be separated but transaction for modifying both of > > > them will be treated as one transaction) > > > > Does it have to be a separate sqlite database? You could also use a new > > table > > directly in the sqlite datbase also used for the contact data. > > > No, it don't need to be separate sqlite database, but I was thinking > about case where database will be locked because some operation like > locale update is ongoing, with separate database modifications data > could be still concurrently accessed. Doesn't sqlite allow reading while a write is ongoing? > > Note that the EBookView could also be used as-is by doing the detailed > > check of > > what was modified in the receiving process. For new contacts it is very > > likely that > > the fields of interest were added, and updating contacts happens rarely > > enough > > that the overhead of sending updated data that then completely gets filtered > > out shouldn't matter. > I'm not sure about using EBookView as-is and checking what was > modified in receiving process, because notification will be sent when > data in contacts database will be already changed, so receiving > process should first get all contacts from database before starting > synchronization to be able to do detailed check of what was modified, > which in case of large number of contacts may unnecessarily increase > memory consumption, especially as you've said that updates happens > rarely. This is a valid concern. Both doing the diff inside the EDS backend (as part of the write) or in SyncEvolution would work better. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Contacts database modifications history
On Tue, 2014-04-15 at 13:16 +, Potrola, MateuszX wrote: > In my project I’m using EDS as backend for storing contacts > synchronized using Syncevolution. > > I would like to have ability to receive some kind of notifications (or > store information in some additional database) about modifications > made to contacts database during synchronization. > > What I mean by modification is information about contact id and list > of contact’s fields that were modified (creation and removal of > contact can be special case - list of all contact’s fields that are > set will be returned). > > I know that current implementation of EBookView allows for something > similar - it allows for detecting newly created, removed or modified > contacts, but unfortunately it doesn’t provide details of > modification. For those who were around at the time, there used to be a similar feature which recorded the UIDs of added, modified and removed contacts. It did not record individual changes and was not as configurable as Mateusz wants it to be now. I was on of those who argued in favor of removing that API because it had several design flaws: 1. Reading changes automatically destroyed the recorded changes, so one could not use it to just check whether something had been modified. 2. Changes were recorded for each client asking for it. EDS had no way of knowing whether the client was still interested in the changes and no garbage collection, so it ended up recording changes in perpetuity if the client was not given a chance (or forgot) to unregister. I wouldn't mind seeing a change tracking API getting re-added to EDS if these design flaws get addressed. If there is not enough interest in that, then a solution were such a feature can be added to EDS by a system integrator via plugins or conditional compilation would also satisfy Mateusz needs. > I came up with two slightly different approaches how this could be > achieved. > > First one is to create new address book backend (which will be > inhering from file backend used by Syncevolution) and add there logic > for calculating modifications when creating/modifying/removing > contacts. These modifications will be stored in additional database, > so some additional functions for handling this will need to be added > to EBookSqlite class. > > Here are some pros and cons that I can see: > > Pros: > > - If someone don’t want to keep track of modifications history > he can still use file backend > > - We can be sure that contacts database and modifications > database will be in sync, by using attach function of sqlite > (databases will be separated but transaction for modifying both of > them will be treated as one transaction) Does it have to be a separate sqlite database? You could also use a new table directly in the sqlite datbase also used for the contact data. > Cons: > > - Quite complex implementation, especially if some API needs to > be exposed by EBookClient Perhaps the API for reading the changes could be limited to direct read access, potentially outside of the core libebook? > Second solution is to extend current implementation of EBookView and > add new signals like: contacts_added, contacts_modified, > contacts_removed which will be providing ids and list of modified > fields. Also new function for setting fields of interest will need to > be added. Again in the address book backend implementation of logic > for calculating modifications and emitting this signals will have to > be added (it can be added to current file backend implementation or > like in previous idea, new backend can be created based on file > backend). Note that the EBookView could also be used as-is by doing the detailed check of what was modified in the receiving process. For new contacts it is very likely that the fields of interest were added, and updating contacts happens rarely enough that the overhead of sending updated data that then completely gets filtered out shouldn't matter. > Pros: > > - Way of storing modifications information is up to the client > application, > > - In our case we would like to know what modifications were made > during synchronization, EDS is not aware of which modification of > contacts are part of which synchronization, but client application has > this knowledge. > > Cons: > > - We may end up with out of sync data in case if some signals > will be not emitted. I see that as the most important reason why I would prefer the more integrated approach. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] custom labels
Hello! Both Google and Apple support custom labels for basically all of a contacts properties (telephone, email, address, instant messaging, etc). They use group tags to associate the extra label with the property: item4.ADR:;;custom address item4.X-ABLabel:custom-label Does anyone have suggestions for how this could and/or should be handled by EDS and Evolution? Obviously the Evolution UI would need to be modified to actually show the information. As a first step I would already be happy if the information didn't get lost during a import/edit/export cycle. I tried grouping, but when editing the field (an EMAIL in my test) its group tag was removed, thus breaking the connection to the custom label. After editing in Evolution: item1.X-ABLabel:foobar EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:email value modified I also tried a custom parameter, but that also got lost: EMAIL;X-ABLabel=foobar;X-EVOLUTION-UI-SLOT=1;TYPE=WORK:email value -> EMAIL;TYPE=WORK;X-EVOLUTION-UI-SLOT=1:email value modified I tried this with Evolution 3.4 (the one shipped with my current desktop, Debian Stable). Has perhaps anything been done regarding this in later versions? I am undecided about what EDS and Evolution should use internally to represent custom labels. I lean towards a custom parameter because although grouping is supported by EVCard, it hasn't been really used, has it? The concept of "fields of interest" would still work if the field also has the custom label as parameter (not that we have special support for anything other than UID and REV, but at least conceptually something else could also be supported). On the other hand, deviating from the format of the peers will make interoperability harder. I'm not sure yet how I would do the conversion in SyncEvolution between X-ABLabel as parameter and X-ABLabel as property with group. Importing/exporting vCards manually also is affected by the choice of the internal format. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] WebKit based Evolution composer status and future
On Mon, 2014-03-17 at 09:25 +0100, Tomas Popela wrote: > Regarding the future of WebKit based Evolution composer I'm proposing > this: merge webkit-composer branch into master when the 3.12 release > will be branched and continue to work on it in master branch. This will > hopefully bring more testers for the new composer. When the code will be > in master I will continue to develop it and also porting it to WebKit2 > with the rest of Evolution. While I have no say in how Evolution gets developed, I nonetheless would like to warn against merging unstable and known buggy code into master. What if it turns out that the code can't be stabilized in time for the next release? Can it be reverted or will Evolution have to go out with known regressions? I know that sometimes one has to take the bullet and release something to get bug reports that one would not get otherwise. But IMHO that should be the last resort when all other options of finding those bugs have been exhausted. Just my 2 cents. Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] [Evolution Data Server] Locale change notification delayed
On Wed, 2014-02-12 at 15:27 +, Potrola, MateuszX wrote: > Hi, > > >Regarding your specific issue - it's probably working the same way in > >master as in the branch - my guess is that this is due to D-Bus property > >changes being delayed until the main loop is hit. > > >It may be that a simple call to g_dbus_interface_skeleton_flush() inside > >the loop which sets locale on each book would fix this. > > You are right - it's the same on master branch, but adding > g_dbus_interface_skeleton_flush call after setting locale fixes the issue. > How I can submit patch, should I open new item on bugzilla and send it there? Yes, that's usually the best approach to ensure that patches get included in EDS. Please include patches that apply to master and the gnome-3-8-openismus branches. Please CC me and I can commit them (unless someone beats me to it - I am currently on vacation). Bye, Patrick ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] libecal soname change
On Thu, 2014-01-02 at 08:22 -0500, Matthew Barnes wrote: > On Thu, 2014-01-02 at 11:30 +0100, Patrick Ohly wrote: > > I just noticed that libecal bumped its soname to libecal-1.2.so.16 in > > EDS 3.10. The corresponding commit is: > > > > commit f30ae26320b359666b345c92405bf87f3f43250a > > Author: Matthew Barnes > > Date: Tue Jul 16 06:34:02 2013 -0400 > > > > Bump libecal / libedata-cal soname for API breaks. > > > > Matthew, can you elaborate what that break is? > > The libdata-cal bump was to cover the preceding commits: [...] > The libecal bump looks like it may have been a mistake on my part. I > must have been under the impression the client-side API changed as well. Indeed, it looks like the soname change was unnecessary. It's too late to fix now, though. Can we perhaps agree on a different procedure for soname changes? If a patch requires a soname change, then alert the list here. Then interested developers can double-check whether a change really is necessary and/or whether a different solution can be found. In addition, the email will serve as warning about upcoming soname changes for developers (like me) who want to provide pre-compiled binaries. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] libecal soname change
Hello! I just noticed that libecal bumped its soname to libecal-1.2.so.16 in EDS 3.10. The corresponding commit is: commit f30ae26320b359666b345c92405bf87f3f43250a Author: Matthew Barnes Date: Tue Jul 16 06:34:02 2013 -0400 Bump libecal / libedata-cal soname for API breaks. Matthew, can you elaborate what that break is? I've looked at a diff of the header files, but most of it is just reformatting. If there is an API break in there, then I missed 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Plan to support and use vCard 4.0 (RFC 6350) in evolution-data-server
Hello! I only saw this email now - please copy me directly on emails that you want me to see. On Fri, 2013-05-17 at 15:34 +0200, Milan Crha wrote: > Hello all, > there was a little discussion about vCard 4.0, aka [RFC6350], which > adds, among other things, a valuable KIND attribute, which is used to > differentiate what the vCard is for. That would get rid of the > evolution's X-EVOLUTION-LIST attribute, and maybe more. Here's a list of > changes between 3.0 and 4.0 vCard [2]. The RFC is not that old yet, it's > only from August 2011, but I guess it's not a problem. > > I'd like to know from others their opinion, whether it would be > appreciated to switch to vCard 4.0 by default in evolution > (-data-server). The plan is to keep backward compatibility with 3.0, > even be able to save in 3.0 format (probably by some combo in a save > dialog to pick the format user prefers), but otherwise switch internally > into vCard 4.0. That way specialized editors could be created (as you > know, there are currently only two, one for general contact, the other > for contact list; maybe it'll make sense to introduce new contact editor > for organization, room, and so on). In general I consider that a good thing. vCard 4.0 is a step forward, but it is not going to be adapted unless someone starts. But being the first to move forward will also mean that there will be problems exchanging data with peers that are still on vCard 3.0, simply because some information cannot be stored there. This is nothing new and SyncEvolution is prepared for such kind of format mismatches, it just will be some work to adapt it. In Evolution it would be good to have the ability to mark an address book as "only supports vCard 3.0 properties". Such a flag should disable all UI features which rely on vCard 4.0. Then address books which mirror a remote vCard 3.0 address book will be usable without surprises for the user. > I also do not know how will other programs work, when user would try to > import a 4.0 vCard, while the program only knows 3.0. I believe the > reading should not be strict on versions, the version only gives a > "hint" what the card can, and what it should not, contain, right? One would hope so, but there's nothing in the standard requiring that. A client might also decide to play it safe and reject vCard 4.0 because it simply cannot know whether the vCard 3.0 decoding rules still apply. For example, in an early draft of vCard 4.0 the set of characters which needed quoting was defined differently than in 3.0. This was reverted back to the rules from 3.0 later on to enhance backward compatibility, but it might as well have stayed in the spec. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] UI interaction from book/calendar backends
On Thu, 2012-11-22 at 13:04 -0500, Matthew Barnes wrote: > Actually I don't think evolution-source-registry requires GTK+. If it's > the password dialog you're thinking of, we only link to gcr-base-3 which > speaks via D-Bus to the process actually showing the password dialog. > > I'm not sure if evolution-source-registry is ultimately the right place > for user interfaces, but I can't think of a better solution at present. > > I have a few requests, though: > > 1) Write this as a ESourceRegistryServer extension, and just link to >GTK+ from the extension module. That way it's easily removable if >the Tizen folks don't want it, or they want to implement their own >version using Qt. Qt is so old-fashioned MeeGo - HTML5 rulez in Tizen! >:-> Thanks for keeping this in mind. I thing it is just good engineering and will hopefully also turn out to be useful outside of Tizen, which - just to avoid any confusion - does not use EDS at the moment and might never do. I'm bringing it up as an example of a distro where compiling EDS is hard at the moment because of the GTK dependency - there might be other, similiary limited platforms. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Addresbook query result ordering
On Mon, 2012-10-15 at 20:59 +0900, Tristan Van Berkom wrote: > Secondly, I see three potential ways of approaching this problem and I > wonder which way would be preferable. > > These are the possible avenues I'm aware of: > > a) Extend EBookQuery API > > This seems the most natural way to go about it, as I see it a query > can contain multiple query terms and a single 'sort order' for the > results; i.e. something like: > >e_book_query_set_order (query, EContactField, AscendingOrDescending); > > However, the EBookQuery does not contain multiple terms, instead it > contains... multiple queries, which would make the API quite awkward > since it would never make sense to have a query with more than one > order defined for it's results. > > Modifying the query also has other drawbacks, it seems some backends > like to send the serialized query expressions over the network, > perhaps backwards compatibility would become a serious issue here > (I've not investigated very far into this). > > b) A new set of query apis > > This might be the right choice considering drawbacks of 'a', in this > case we could introduce an entirely new set of apis concerned with > getting results from the addressbook. > > so methods such as: > >e_book_client_get_contacts (book, query, ...) > > would receive a complimentary: > >e_book_client_get_sorted_contacts (book, query, sort_field, ASCEND...) > > possibly deprecating all of the orderless apis which preceded > these new apis along the way. > > This has the obvious disadvantage of implementation difficulty, > i.e. EDS has still not been ported to use the new gdbus-codegen > tool so all of that d-bus stuff would need to be done by hand > (possibly less overall effort to just port to gdbus-codegen first). > > One upside to this is that backends which don't yet support the > sorted results could fall back on existing APIs, possibly by > introducing an EBookBackendSortable interface or such > (question still would remain... how to advertise at runtime > whether the sorting is supported by the active backend, perhaps > via the 'capabilities' backend property). > > c) The least attractive and easiest way would be to simply add an > api to EBookClient telling it "in what order should you sort > the addressbook" (or, an ESourceExtension configuring the sort > order). There is a forth option: d) add sorting to EBookView and leave e_book_client_get_contacts() as it is. Extending EBookView might be easier than extending EBookQuery. Note that when adding sorting, it also makes sense to add a "maximum number of contacts" parameter. Without sorting, that value is useless because is not predictable which contacts will be included in the result. With sorting, it becomes possible to populate a fixed-size view without having to receive all results. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Regarding ESource and ESourceExtension
On Mon, 2012-10-08 at 14:42 -0400, Matthew Barnes wrote: > Not sure if this fully addresses your question or not. I'm still at a > bit of loss as to your use case. One example would be SyncEvolution, configuring and using additional features in EDS if and only if they are supported. If that cannot be detected at runtime, then one of several much less attractive workarounds must be used: * configure option in SyncEvolution - which users will get wrong * detect version of EDS and its backend (not even sure whether that works with enough granularity) and map to hard-coded list of capabilities - will be difficult to keep up-to-date -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Regarding API breakage and lost test cases
On Tue, 2012-10-02 at 13:30 +0900, Tristan Van Berkom wrote: > I'm still trying to find my footing here, the migration guide > and new documentation on ESourceRegistry don't seem to outline > how a new addressbook is actually created. > > i.e. if I wanted to test a specific addressbook backend, > I would need a way to create one and populate it inside some sandbox > directory. The location of the database files is fixed for the file backend. It has to be in ~/.local/share/evolution and is derived from the UUID. I agree that an explicit path property for the file backend would be nice. > I'm getting as far as: > > // Create scratch source > e_source_new_with_uid(); > > // Add it to the registry > e_source_registry_new(); > e_source_registry_add_sources(); > > // Fetch the new source from the registry (no longer 'scratch') > e_source_registry_ref_source(); > > // Create the book now that we have a valid source (... or so we > // suspected) > e_book_new (); > > Problems I'm encountering are: > > a.) It seems that e_source_registry_ref_source() only finds the > newly created source the next time I startup the test app > (so this may indicate a bug, that the local registry is not > updated after adding a new source). Not sure about that one. To some extend the API depends on processing D-Bus events in the main event loop. Perhaps that's not happening in your test app? > b.) e_book_new() complains that the new source has no backend: > >"e-book.c:2944: cannot get book from factory: > No backend name in source 'Unnamed'" You need to add the right extensions to the new source: source = e_source_new(); ESourceBackend *backend = e_source_get_extension(source, E_SOURCE_EXTENSION_ADDRESS_BOOK); e_source_backend_set_backend_name(backend, "local"); > I'm sure I've missed something ridiculous here, while observing > the sources e-data-book-factory.c, it seems that if the source > were given an addressbook extension with the intended backend > name... that it would happily go ahead and create a book. > > But, while we do have: > e_source_get_extension() & e_source_has_extension() > > I'm not seeing: > e_source_add_extension() _get_extension() adds the extension implicitly. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Update on the composer port
On Sun, 2012-09-02 at 00:16 +0200, Dan Vratil wrote: > Behavior of the editor is almost identical to GtkhtmlEditor, except for HTML > mode -> Plain text switching. GtkhtmlEditor is simply switching renderers on > top of a single DOM document, but we can't do this with WebKit. I looked how > others do it and ended up with a "Switching to plain text will lose all > formatting, OK?" dialog. Please excuse my ignorance, but how will plain text mode work in the future? Will it still be possible to define item lists, indent blocks, etc.? I edit and send only plain text emails, but I use these formatting helps from Evolution, including saving of drafts with this markup. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ evolution-hackers mailing list evolution-hackers@gnome.org To change your list options or unsubscribe, visit ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] e_cal_client_check_timezones() + e_cal_client_tzlookup() + Could not retrieve calendar time zone: Invalid object
Hello! I've started testing a version of SyncEvolution which uses the EBook/CalClient API. The import test fails for a VEVENT which uses a custom timezone: BEGIN:VCALENDAR PRODID:-//Ximian//NONSGML Evolution Calendar//EN VERSION:2.0 BEGIN:VTIMEZONE TZID:EST/EDT BEGIN:STANDARD TZOFFSETFROM:-0400 TZOFFSETTO:-0500 TZNAME:EST DTSTART:19671029T02 RRULE:FREQ=MONTHLY;INTERVAL=12;BYDAY=-1SU END:STANDARD BEGIN:DAYLIGHT TZOFFSETFROM:-0500 TZOFFSETTO:-0400 TZNAME:EDT DTSTART:19870405T02 RRULE:FREQ=MONTHLY;INTERVAL=12;BYDAY=1SU END:DAYLIGHT END:VTIMEZONE BEGIN:VEVENT UID:20060416T205224Z-4272-727-1-251@gollum DTSTAMP:20060416T205224Z DTSTART;TZID=EST/EDT:20060406T14 DTEND;TZID=EST/EDT:20060406T143000 TRANSP:OPAQUE SEQUENCE:2 SUMMARY:timezone New York with custom definition for 2006 CLASS:PUBLIC CREATED:20060416T205301Z LAST-MODIFIED:20120531T085920Z END:VEVENT END:VCALENDAR e_cal_client_check_timezones() fails with "Could not retrieve calendar time zone: Invalid object". This comes from the attempt to look up "EST/EDT" in an empty calendar: icaltimezone * e_cal_client_tzlookup (const gchar *tzid, gconstpointer ecalclient, GCancellable *cancellable, GError **error) { ... if (e_cal_client_get_timezone_sync (cal_client, tzid, &zone, cancellable, &local_error)) { g_warn_if_fail (local_error == NULL); return zone; } if (g_error_matches (local_error, E_CAL_CLIENT_ERROR, E_CAL_CLIENT_ERROR_OBJECT_NOT_FOUND)) { /* We had to trigger this error to check for the * timezone existance, clear it and return NULL. */ g_clear_error (&local_error); } ... It's normal that e_cal_client_get_timezone_sync() fails. But it fails with an unexpected error: signal sender=:1.3 -> dest=(null destination) serial=94 path=/org/gnome/evolution/dataserver/Calendar/1636/9; interface=org.gnome.evolution.dataserver.Calendar; member=get_timezone_done uint32 18 string "org.gnome.evolution.dataserver.Calendar.InvalidObject" string "Could not retrieve calendar time zone: Invalid object" string "" Therefore the error is not cleared and e_cal_client_check_timezones() aborts instead of accepting the "EST/EDT" time zone as it is. This works in Evolution itself, probably because it let's the backend itself do the e_cal_client_check_timezones() with a different lookup function. What is the preferred way of fixing this? We could define that e_cal_client_get_timezone*() must return the E_CAL_CLIENT_ERROR_OBJECT_NOT_FOUND error when the time zone doesn't exist and fix it to do so. Or we could make the error check in e_cal_client_tzlookup() more liberal and ignore all E_CAL_CLIENT_ERRORs. The latter might be easier, in particular considering that multiple different calendar backends might need fixing to reliably return E_CAL_CLIENT_ERROR_OBJECT_NOT_FOUND. -- 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 ... https://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] Evolution module renames
On Fri, 2012-05-11 at 09:52 +0200, Milan Crha wrote: > It's a general issue, because you never know what you'll have pulled in, > and what steps you should do before you actually pull those changes in. > I do not know if we can anyhow instruct autotools to not do this hard > dependency of .am and .ac files, but I'm afraid we cannot do anything > with this. --disable-maintainer-mode if you have AM_MAINTAINER_MODE in configure.ac. In general I don't trust "make uninstall", in any project. It's usually not available when needed again, probably untested, or will fail for some other reason (like the one you found). I rather install into a directory that I can wipe out entirely, or use checkinstall. -- 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] PIM server synchronization and Evolution online/offline state
On Wed, 2012-04-04 at 13:32 +0200, Christian Hilberg wrote: > Which is the long-term vision for Evolution in this regard? Lack of proper offline support has been my main motivation for developing SyncEvolution. I know from others that they, too, would love to see this supported natively in EDS+Evolution, without the need for an external application. If there is sufficient interest, then I would be open for discussions about how SyncEvolution could be integrated seamlessly into Evolution. This would bring offline support for CalDAV, CardDAV, ActiveSync and might even add support for SyncML peers (client or server). But note that the current code is in C++ and depends on additional libraries that are not currently part of the GNOME stack (libsynthesis, libneon for offline CalDAV/CardDAV). Rewriting it in pure C+GNOME would be a lot of work. -- 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] valgrind error in evolution-addressbook-factory + libdb
On Tue, 2012-03-06 at 14:33 +0100, Patrick Ohly wrote: > Hello! > > Checking the EDS daemons from master under valgrind found this: > > # ==23596== Conditional jump or move depends on uninitialised value(s) > # ==23596==at 0x9A404E7: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x9A41CB1: __log_put (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x9A42F2D: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x9A43D9F: __log_put_record (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x9986B1B: __ham_add_el (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x997E1FD: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x99F5548: __dbc_iput (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x99F314F: __db_put (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x9A04EBA: __db_put_pp (in > /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > # ==23596==by 0x103CC953: do_create (e-book-backend-file.c:915) > # ==23596==by 0x103CCBFC: e_book_backend_file_create_contacts > (e-book-backend-file.c:988) > # ==23596==by 0x4E4AC31: e_book_backend_sync_create_contacts > (e-book-backend-sync.c:85) > # ==23596==by 0x4E4D3D7: book_backend_create_contacts > (e-book-backend-sync.c:493) > # ==23596==by 0x4E4ED2A: e_book_backend_create_contacts > (e-book-backend.c:465) > # ==23596==by 0x4E513D3: operation_thread (e-data-book.c:157) > # ==23596==by 0x83E9D07: ??? (in > /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) > # ==23596==by 0x83E77E5: ??? (in > /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) > # ==23596==by 0x8678B4F: start_thread (pthread_create.c:304) > # ==23596==by 0x896690C: clone (clone.S:112) > # ==23596== Uninitialised value was created by a stack allocation > # ==23596==at 0x994DB80: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) > > That is with libdb 5.1.29-1 from Debian Testing, in 64 bit mode. > > At first glance it doesn't look like EDS can be causing this. I don't > have time to investigate by compiling libdb from source right now, but > at least I wanted to point out that there might be a cause for random > crashes. It's not bad as I thought. Am I too pessimistic? ;-) I've tracked it down and filed a bug against Debian: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=662917 >From the report: - I've tracked the root cause in __ham_add_el() in src/hash/hash_page.c: if (is_databig) { doff.type = H_OFFPAGE; ===>UMRW_SET(doff.unused[0]); ===>UMRW_SET(doff.unused[1]); ===>UMRW_SET(doff.unused[2]); UMRW_SET is a NOP unless --enable-umrw is used during configure. Compiling with --enable-umrw avoids the valgrind warning. The unitialized value is used in a comparison of a checksum with 0. I can convince myself that this is probably okay, because it doesn't matter whether some of the data the checksum was calculated over was uninitialized as long as it doesn't change later on. But nevertheless, having to deal with such random valgrind errors while debugging other code isn't nice. Please consider using --enable-umrw when compiling libdb. -- 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
[Evolution-hackers] valgrind error in evolution-addressbook-factory + libdb
Hello! Checking the EDS daemons from master under valgrind found this: # ==23596== Conditional jump or move depends on uninitialised value(s) # ==23596==at 0x9A404E7: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x9A41CB1: __log_put (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x9A42F2D: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x9A43D9F: __log_put_record (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x9986B1B: __ham_add_el (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x997E1FD: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x99F5548: __dbc_iput (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x99F314F: __db_put (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x9A04EBA: __db_put_pp (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) # ==23596==by 0x103CC953: do_create (e-book-backend-file.c:915) # ==23596==by 0x103CCBFC: e_book_backend_file_create_contacts (e-book-backend-file.c:988) # ==23596==by 0x4E4AC31: e_book_backend_sync_create_contacts (e-book-backend-sync.c:85) # ==23596==by 0x4E4D3D7: book_backend_create_contacts (e-book-backend-sync.c:493) # ==23596==by 0x4E4ED2A: e_book_backend_create_contacts (e-book-backend.c:465) # ==23596==by 0x4E513D3: operation_thread (e-data-book.c:157) # ==23596==by 0x83E9D07: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) # ==23596==by 0x83E77E5: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) # ==23596==by 0x8678B4F: start_thread (pthread_create.c:304) # ==23596==by 0x896690C: clone (clone.S:112) # ==23596== Uninitialised value was created by a stack allocation # ==23596==at 0x994DB80: ??? (in /usr/lib/x86_64-linux-gnu/libdb-5.1.so) That is with libdb 5.1.29-1 from Debian Testing, in 64 bit mode. At first glance it doesn't look like EDS can be causing this. I don't have time to investigate by compiling libdb from source right now, but at least I wanted to point out that there might be a cause for random crashes. -- 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] libebook master: obsolete vCard used despite updated properties
On Sat, 2012-03-03 at 13:26 +0100, Patrick Ohly wrote: > It removes the check for evc->priv->attributes. Adding that check back > fixes the problem. Attached is the patch. I must admit that I'm not up-to-date about release plans. From Matthew's email I gathered that there will be a release on Monday?! Please include this patch. I think it is obviously correct, or at least as correct as the original code was before introducing the vCard 2.1 support - see below. > Question: why does the removed comment only mention UID? Isn't any > contact modification detected by checking evc->priv->attributes? I'm still wondering about that, primarily because I want to be sure that the code really works in all cases. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. >From 956c91eb6e8ed89d3847738768d53752d2e2d7a4 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Sun, 4 Mar 2012 14:24:03 + Subject: [PATCH] EContact: fix "parse vcard + update contact + commit" The commit which introduced vCard 2.1 encoding support (cca25e) removed the "evc->priv->attributes" check. Without that check the encoder will always return a cached vCard, even if the EContact was modified in the meantime. Found when updating of a contact with SyncEvolution's --update operation failed because the UID set after parsing the vCard was ignored and thus committing the update wasn't possible. Might also break quite a lot of other functionality! Fixed by addding the "evc->priv->attributes" in both cases (vCard 2.1 and 3.0). --- addressbook/libebook/e-vcard.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/addressbook/libebook/e-vcard.c b/addressbook/libebook/e-vcard.c index 1ff8af2..375a9d1 100644 --- a/addressbook/libebook/e-vcard.c +++ b/addressbook/libebook/e-vcard.c @@ -1251,12 +1251,14 @@ e_vcard_to_string (EVCard *evc, switch (format) { case EVC_FORMAT_VCARD_21: - if (evc->priv->vcard && strstr_nocase (evc->priv->vcard, CRLF "VERSION:2.1" CRLF)) + if (evc->priv->vcard && evc->priv->attributes == NULL && + strstr_nocase (evc->priv->vcard, CRLF "VERSION:2.1" CRLF)) return g_strdup (evc->priv->vcard); return e_vcard_to_string_vcard_21 (evc); case EVC_FORMAT_VCARD_30: - if (evc->priv->vcard && strstr_nocase (evc->priv->vcard, CRLF "VERSION:3.0" CRLF)) + if (evc->priv->vcard && evc->priv->attributes == NULL && + strstr_nocase (evc->priv->vcard, CRLF "VERSION:3.0" CRLF)) return g_strdup (evc->priv->vcard); return e_vcard_to_string_vcard_30 (evc); -- 1.7.2.5 ___ 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] PHOTO uri + escaping
Hello! I am seeing an issue where the Synthesis engine doesn't parse a vCard with PHOTO;VALUE=uri the same way as libebook's e_contact_inline_local_photo() does. The vCard has: PHOTO;VALUE=uri:file:///home/nightly/.local/share/evolution/addressbook/130 3826927.6946.21@mob-sync2/photos/pas_id_4F511ADB000F_photo-file0.image %252FGIF The actual file name is: /home/nightly/.local/share/evolution/addressbook/1303826927.6946.21@mob-sync2/photos/pas_id_4F521E7A000F_photo-file0.image%2FGIF Note that %2FGIF was encoded as %252FGIF in the vCard. The Synthesis engine does no decoding of the %25 before trying to read the file. I think this is a bug, and I take full credit, eh, blame for it as I added that too simplistic URI handling code ;-) However, why does the file name end in %2FGIF in the first place? It would be much more readable to just use .GIF. That notwithstanding, of course I intend to fix the bug in the Synthesis engine. -- 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
[Evolution-hackers] libebook master: obsolete vCard used despite updated properties
Hello! I tested SyncEvolution against current EDS master and found a problem: setting a UID before committing an updated contact has no effect. The sequence of events is this: 1. e_contact_new_from_vcard() where the vCard contains no UID. 2. e_contact_set(E_CONTACT_UID, ) -> g_object_set() 3. e_book_commit_contact() -> e_vcard_to_string() -> D-Bus In step 3, e_vcard_to_string() returns the cached evc->priv->vcard without the UID although it has a UID in its attributes (evc->priv->attributes). I think the culprit is this commit here: commit cca25e98a71d8c06f9ce1b53658ec4675f2dd5c2 Author: Bartosz Szatkowski Date: Tue Oct 18 12:29:07 2011 +0200 Bug #656603 - Add support for generating vCard 2.1 in libebook [...] @@ -1016,17 +1246,16 @@ e_vcard_to_string (EVCard *evc, { g_return_val_if_fail (E_IS_VCARD (evc), NULL); - /* If the vcard is not parsed yet, and if we don't have a UID in priv->attributes - return priv->vcard directly */ - /* XXX: The format is ignored but it does not really matter -* since only 3.0 is supported at the moment */ - if (evc->priv->vcard != NULL && evc->priv->attributes == NULL) - return g_strdup (evc->priv->vcard); - switch (format) { case EVC_FORMAT_VCARD_21: + if (evc->priv->vcard && strstr_nocase (evc->priv->vcard, CRLF "VERSION:2.1" CRLF)) + return g_strdup (evc->priv->vcard); + return e_vcard_to_string_vcard_21 (evc); case EVC_FORMAT_VCARD_30: + if (evc->priv->vcard && strstr_nocase (evc->priv->vcard, CRLF "VERSION:3.0" CRLF)) + return g_strdup (evc->priv->vcard); + return e_vcard_to_string_vcard_30 (evc); default: g_warning ("invalid format specifier passed to e_vcard_to_string"); It removes the check for evc->priv->attributes. Adding that check back fixes the problem. Question: why does the removed comment only mention UID? Isn't any contact modification detected by checking evc->priv->attributes? -- 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
[Evolution-hackers] libecal: memory leak
Hello! I've updated one of the chroots for SyncEvolution's nightly testing to current Debian Testing. Now I am seeing a memory leak in libecal 3.2.2: ==23530== 597 bytes in 15 blocks are definitely lost in loss record 4,401 of 4,550 ==23530==at 0x4C27673: malloc (vg_replace_malloc.c:263) ==23530==by 0x7D8AC02: g_malloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) ==23530==by 0x7DA13BD: g_strdup (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) ==23530==by 0x13BF5CCF: gdbus_proxy_async_method_done (e-gdbus-templates.c:1016) ==23530==by 0x78F7803: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x7909789: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x7912E10: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x7912FB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x13BF5438: e_gdbus_proxy_emit_signal (e-gdbus-templates.c:600) ==23530==by 0xC5BE7BB: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==23530==by 0xC5BE236: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.5.0.10) ==23530==by 0x78F7CC6: g_cclosure_marshal_generic (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x78F7803: g_closure_invoke (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x79095BE: ??? (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x7912E10: g_signal_emit_valist (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0x7912FB1: g_signal_emit (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.3000.2) ==23530==by 0xB82F0FE: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.3000.2) ==23530==by 0xB81D9AD: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.3000.2) ==23530==by 0x7D840CE: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) ==23530==by 0x7D848C7: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) ==23530==by 0x7D84A98: g_main_context_iteration (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.2) ==23530==by 0x13BF36EA: gdbus_proxy_call_sync (e-gdbus-templates.c:1442) ==23530==by 0x13BF72B8: e_gdbus_proxy_call_sync_string__string (e-gdbus-templates.c:1608) ==23530==by 0x1650896A: e_gdbus_cal_call_create_object_sync (e-gdbus-cal.c:571) ==23530==by 0x164DEE15: e_cal_create_object (e-cal.c:3932) ==23530==by 0xFA0B881: SyncEvo::EvolutionCalendarSource::insertItem(std::string const&, std::string const&, bool) (EvolutionCalendarSource.cpp:446) Does that look familiar to anyone? -- 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] e_cal/book_open() + only_if_exists
On Tue, 2012-02-14 at 08:45 +0100, Milan Crha wrote: > On Mon, 2012-02-13 at 21:52 +0100, Patrick Ohly wrote: > > * A second invocation of SyncEvolution finds the definition of the > > system address book via e_book_get_addressbooks() and tries to > > open it with e_book_open(only_if_exists=false), which then fails > > with various errors, depending on the EDS version ("Cannot open > > book: db error 0x2 (No such file or directory)" in 3.2, > > "DB_RUNRECOVERY" in 3.3). > > > > The problem can be avoided by always passing only_if_exists=true to > > the open calls. The way how the API is defined and/or implemented, > > creating an database definition and creating the actual database files > > are two different steps. > > Hi, > that's rather a bug in the backend. Calling with only_if_exists=false > might mean that the backend should create a new book/calendar, in case > it's not available yet. Otherwise, if you call with only_if_exists=true, > the backend should claim an error when the requested book/calendar > cannot be opened. Sorry, typo in my email. I meant "the problem can be avoided by always passing only_if_exist=false". In other words, never use only_if_exists=true because it serves no useful purpose. Is that correct or is there some legitimate usage for only_if_exists=true? -- 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
[Evolution-hackers] e_cal/book_open() + only_if_exists
Hello! With EDS 3.2 and 3.3 I've seen the following problem on a clean account (no databases, no gconf keys): * An invocation of SyncEvolution to list databases calls e_book_new_system_addressbook(), which creates a gconf /apps/evolution/addressbook/sources entry for the system address book. * A second invocation of SyncEvolution finds the definition of the system address book via e_book_get_addressbooks() and tries to open it with e_book_open(only_if_exists=false), which then fails with various errors, depending on the EDS version ("Cannot open book: db error 0x2 (No such file or directory)" in 3.2, "DB_RUNRECOVERY" in 3.3). The problem can be avoided by always passing only_if_exists=true to the open calls. The way how the API is defined and/or implemented, creating an database definition and creating the actual database files are two different steps. Therefore a client can never be sure whether a database really exists, and thus it always to pass only_if_exists=true. If that is the case, then why have it at all? We might as well ignore the value in the current implementations and mark it as deprecated in the APIs. Or do I miss something? -- 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] History is gone after wip/gsettings branch merge
On Mon, 2011-11-28 at 10:59 +0100, Milan Crha wrote: > On Mon, 2011-11-28 at 10:15 +0100, Patrick Ohly wrote: > > On Mon, 2011-11-28 at 08:27 +0100, Milan Crha wrote: > > > Hi, > > > I just realized a very sad thing, the history of certain files is gone > > > after the merge of wip/gsettings branch into evolution master. I tried > > > both gitk and git log on mail/e-mail-session.c and the history is ending > > > on October 6th, 2010. > > > > I had a copy of the git repo before that merge and the history of that > > file also ended on that date. I doubt that this has anything to do with > > any git merge. > > Hrm, is it? Actually I now doubt that my git repo was old enough. I thought you talked about the account settings, which is the one which I hadn't pulled yet. Either way, a git merge doesn't rewrite history. > I know it's a new file, but I thought it's older than > October 6th, 2010. It seems that a06e4484b on October 6th, 2010 renamed and modified the file (mail-session.c -> e-mail-session.c) at the same time. That defeats git's --follow and --find-renames heuristics. That's actually an argument in favor of doing merges instead of just applying the final, resulting patch: the file renaming can be committed in a single commit together with the related changes (autotools, include statements) without changing the file content itself. Then in a separate commit, make the actual changes to the content. That is easier to review (because the diffs will be much smaller) and I bet that "git log" would be able to show the full history for the renamed file. Very interesting discussion :-) SyncEvolution almost never accepts new features via merging, instead applying the final, squashed patch. The argument was that a linear development history is easier to read. But it does have this disadvantage of not handling rename+modification and also looses valuable history about the patch itself, or at least makes it harder to find. -- 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] History is gone after wip/gsettings branch merge
On Mon, 2011-11-28 at 08:27 +0100, Milan Crha wrote: > Hi, > I just realized a very sad thing, the history of certain files is gone > after the merge of wip/gsettings branch into evolution master. I tried > both gitk and git log on mail/e-mail-session.c and the history is ending > on October 6th, 2010. I had a copy of the git repo before that merge and the history of that file also ended on that date. I doubt that this has anything to do with any git merge. > The question is: Can this be fixed? Very hard. Most likely the initial conversion from svn was incomplete. One could redo the conversion and/or rewrite the history, but that implies all commits done later and all trees that other people currently work with become invalid and need to be rebased. IMHO it would be better to add the missing history as independent branches. > P.S.: I never trusted the git merge, it breaks many things, from commit > times/dates Huh? How that? "git merge" only adds one commit. It does not rewrite existing commits from any of the branches that get merged. > to lost work from others if the merger is not careful > enough. Now that is indeed a valid concern. A "git merge" is like applying a single, fairly complex patch (assuming that the branch which gets merged is large). To be on the safe side, a merge could be prepared, pushed to a separate branch and then get reviewed before fast-forwarding the real branch. The downside is that any change on the real branch in the meantime requires redoing the merge or accepting some kind of unreviewed merging again: merge A+B = C gets reviewed, A' + C does not, which might be okay if the delta between A and A' is small and does not lead to further conflicts with C. -- 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] UID in vCard
On Fri, 2011-11-18 at 17:27 +0100, Christian Hilberg wrote: > Am Freitag 18 November 2011, um 16:53:38 schrieb Patrick Ohly: > > On Fri, 2011-11-18 at 11:23 +0100, Christian Hilberg wrote: > > > > That is the whole point of this mail thread: a vCard UID may have a > > > > meaning outside of the storage in which it currently exists. EDS cannot > > > > know whether that is the case. Currently it assumes that the UID has no > > > > meaning and throws it away when adding contacts. > > > > > > Not globally true. The file backend may do so, but it is the backend > > > implementation > > > deciding whether re-writing a UID or not. E-D-S cannot decide that, since > > > it does > > > not know what a given backend is dealing with. > > > > What I meant is the Evolution/EDS API expectation that adding a contact > > will never fail because of a UID conflict. Whether the backend > > implements that by always overwriting the UID (as the file backend does) > > or by keep it when possible and overwriting otherwise (as in the Kolab > > backend) is indeed a backend implementation detail. > > From evolution-kolab's point of view, it would be simple to return a > "*mp* UID already exists, try again"-error to E-D-S in that case, > provided the E-D-S API for that existed. As Matthew said, the Evolution UI itself is probably not able to handle such an error at the moment. So at the end of the day, it might be the client which has to tell EDS+backend and/or libebook which behavior it wants: preserve UID at all costs (including throwing errors) or emulate the current semantic (don't throw errors, overwrite UID if necessary). > > But it has the same effect: a libebook user cannot reliably detect an > > add<->add UID conflict. It can check for a contact with the UID first > > (by assuming that ID in the libebook API == UID in vCard), but then > > there's still a race condition between that check and creating that > > contact. > > Again, in Kolab context, the user of the calendar lib or addressbook lib > would still get a vague indication only. The race condition could still > occur, since there is (a) no transactional system provided by the Kolab server > for PIM object creation and (b) any Kolab client is required to fully work > in offline mode. This leads to another pet peeve of mine: all backends should be able to work in read/write offline mode. This implies syncing between local and remote data when going online, which should be provided as a core feature of EDS instead of being replicated in every single backend. Akonadi provides such a feature. I have some ideas how SyncEvolution could be used for that inside EDS/Evolution, but that's a topic for another day and mail thread... > AFAICS, the following may be a good start: > * have E-D-S generate good UUIDs > * give the backends the chance to report if a UUID already exists > (if the error does not pop up instantly, it does not mean everything > is well, but *if* it does, certainly E-D-S can try again and generate > another UID, and notify the user) > * encourage all backend implementors not to overwrite existing UIDs, > if at all possible Agreed. -- 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] UID in vCard
On Fri, 2011-11-18 at 11:23 +0100, Christian Hilberg wrote: > Am Mittwoch 16 November 2011, um 15:55:54 schrieb Patrick Ohly: > > Hello! > > [...] > > On Di, 2011-11-15 at 15:01 +0100, Christian Hilberg wrote: > > > If a new UID is to be created, it is the responsibility of the Kolab > > > client to > > > assign one. The Kolab server itself is unaware to object UIDs and will > > > not touch > > > them (no read/write/anything). > > With "client", I meant an EDS client here (= the application using > > libebook). That there is a Kolab client and server involved is of course > > important for you, but not so much for a user of the abstract libebook > > API ;-) > > While the E-D-S client (like Evo) might not be interested whether it is > a Kolab backend being used, there is still one thing you may wish to consider > here. > We could of course map between Kolab PIM object UIDs and E-D-S UUIDs in our > backend. That should never be necessary. As you said, having such two different ids doesn't buy the user much. Only a radical step away from "do what you want with UID" to "UID must be UUID and preserved" will - not realistic anytime soon, but at least a proof-of-concept would be nice. > > That is the whole point of this mail thread: a vCard UID may have a > > meaning outside of the storage in which it currently exists. EDS cannot > > know whether that is the case. Currently it assumes that the UID has no > > meaning and throws it away when adding contacts. > > Not globally true. The file backend may do so, but it is the backend > implementation > deciding whether re-writing a UID or not. E-D-S cannot decide that, since it > does > not know what a given backend is dealing with. What I meant is the Evolution/EDS API expectation that adding a contact will never fail because of a UID conflict. Whether the backend implements that by always overwriting the UID (as the file backend does) or by keep it when possible and overwriting otherwise (as in the Kolab backend) is indeed a backend implementation detail. But it has the same effect: a libebook user cannot reliably detect an add<->add UID conflict. It can check for a contact with the UID first (by assuming that ID in the libebook API == UID in vCard), but then there's still a race condition between that check and creating that contact. -- 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] UID in vCard
Hello! Before I reply to Christian, let me elaborate a bit more on why overwriting the UID is bad for syncing. I thought it was obvious, but probably not ;-} What I have in mind is a system where contact data moves freely and ad-hoc between peers, without being forced to go through a central server, stick to a fixed topology or use a specific database technology. This cannot be done with today's technology because avoiding duplicates becomes unreliable. CouchDB comes close, but is a closed system. You also cannot create two independent CouchDB instances and then merge them (at least that is my understanding - I might be wrong on that particular aspect). A creator-assigned global UID would be the right way of solving this. Storages or systems which do create and preserve such a UID cannot participate in that system. I'd like to make sure that the EDS file backend can be part of it, even if it is just for experiments. For more details, see the TODOs on "ad-hoc synchronization" in http://syncevolution.org/blogs/pohly/2011/state-union-version-12 On Di, 2011-11-15 at 15:01 +0100, Christian Hilberg wrote: > Am Dienstag 15 November 2011, um 11:03:24 schrieb Patrick Ohly: > > On Di, 2011-11-15 at 10:50 +0100, Christian Hilberg wrote: > > [...] > > > Just adding a few bits on how the situation is for the Kolab groupware > > > server. > > > > > > The evolution-kolab backend cannot ask the Kolab server for a UID (since > > > there > > > is no API for that) nor does the server enforce certain UIDs on a PIM > > > object (but, > > > of course, that there be one). The only requirement is that the PIM > > > object's UID > > > be unique in a given PIM folder. If the UID is globally unique, all the > > > better. > > > > That's the same situation as with the file backend then: a client could > > decide to set a UID in the vCard before creating the contact, and the > > Kolab backend+server would use that UID instead of creating their own. > > Good :-) > > If a new UID is to be created, it is the responsibility of the Kolab client to > assign one. The Kolab server itself is unaware to object UIDs and will not > touch > them (no read/write/anything). With "client", I meant an EDS client here (= the application using libebook). That there is a Kolab client and server involved is of course important for you, but not so much for a user of the abstract libebook API ;-) > > How does the backend work at the moment? Does it always overwrite an > > existing UID like the file backend does or does it already work as I > > proposed? > > Existing UIDs are (and must be) preserved. This is a requirement for Kolab > client > interop, since they all rely on the object's UID to reference it (especially > regarding > changes to the contents of the PIM object). In Kolab, there is no way to > correctly reference an object other than using its UID. > > > If it does, do you throw a > > E_BOOK_ERROR_CONTACT_ID_ALREADY_EXISTS when the existing UID is not > > unique? > > Eeewww. :-) evolution-kolab presently sits on Evo/EDS 2.30.3 (which some like > to call "just plain old" here =). No error message in that case. If the UID > already exists, it gets rewritten. It was a tradeoff here - an existing Kolab > object and its UID superseedes a new one. Imported objects are regarded as > "new" (being assigned a new UID), should the UID they carry already exist in > the given PIM folder. The original UID would do no good in the Kolab context > if another object with that same UID already exists, since other groupware > clients do have an idea about the object this UID refers to already. Trying > to find out whether the imported object could actually be an update for an > already existing one seemed too complex and out-of-scope for the initial > evolution-kolab implementation. We're now porting to current Evo/EDS git > master, > but I would still keep the current implementation unchanged when it comes > to how to interpret UIDs of imported objects. That is the whole point of this mail thread: a vCard UID may have a meaning outside of the storage in which it currently exists. EDS cannot know whether that is the case. Currently it assumes that the UID has no meaning and throws it away when adding contacts. Consider sending around vCard with globally unique UID. A user might import that vCard once into Kolab using Kolab client A. Then when he tries again with Kolab client B, the client can warn him reliably that this particular contact was already imported. > > > PIM objects already residing on the Kolab server do carry a UID, created > > > by the > > > client which created th
Re: [Evolution-hackers] UID in vCard
On Di, 2011-11-15 at 10:50 +0100, Christian Hilberg wrote: > Hi everyone, > > Am Montag 14 November 2011, um 11:22:57 schrieb Milan Crha: > > On Mon, 2011-11-14 at 10:00 +0100, Patrick Ohly wrote: > > > So I suggest to pursue the first approach instead. I think it is > > > possible for the file backend. > > > > > > Is it also possible for other backends? Or are some unable to store > > > the UID and look up contacts (efficiently) by it? In that case we will > > > have to relax the semantic of the API and accept that some backends > > > still use their own local ID. "Supports UID" should be defined as a > > > capability of the backend so that clients can take it into account. > > > > Hi, > > I wouldn't call it "local UID", it's rather "backend's UID" and mostly > > not, this is not doable for groupware backends which are using > > server-supplied unique identifiers for contacts/calendars, like message > > IDs in evolution-mapi, which talks to Exchange servers. It's easier, and > > makes sense, to use server-side IDs, which are meant to be unique. > > It's also a reason, from my point of view, why > > e_data_book_respond_create_contacts() passes UIDs of newly created > > objects back to the client. > > [...] > > Just adding a few bits on how the situation is for the Kolab groupware server. > > The evolution-kolab backend cannot ask the Kolab server for a UID (since there > is no API for that) nor does the server enforce certain UIDs on a PIM object > (but, > of course, that there be one). The only requirement is that the PIM object's > UID > be unique in a given PIM folder. If the UID is globally unique, all the > better. That's the same situation as with the file backend then: a client could decide to set a UID in the vCard before creating the contact, and the Kolab backend+server would use that UID instead of creating their own. Good :-) How does the backend work at the moment? Does it always overwrite an existing UID like the file backend does or does it already work as I proposed? If it does, do you throw a E_BOOK_ERROR_CONTACT_ID_ALREADY_EXISTS when the existing UID is not unique? > PIM objects already residing on the Kolab server do carry a UID, created by > the > client which created the object (evolution-kolab, Kontact, Horde, Outlook with > a Kolab connector, Thunderbird via Lightning/SyncKolab, ...). Do you attempt to make the UID globally unique, for example by using a UUID? > When it comes to importing a PIM object, it is not possible to retain its > UID in the cases where the same UID exists on the server already for another > PIM > object (unlikely, but possible, since Kolab object UIDs are not required to be > globally unique). As long as we're in offline mode, we may at first succeed > retaining > the object's UID, but when going online any syncing with the server, find that > a new UID must be set on the object. What happens during syncing? Do you resolve the add<->add conflict by duplicating the item, merging them or discarding one copy? -- 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] UID in vCard
On Mo, 2011-11-14 at 11:22 +0100, Milan Crha wrote: > On Mon, 2011-11-14 at 10:00 +0100, Patrick Ohly wrote: > > So I suggest to pursue the first approach instead. I think it is > > possible for the file backend. > > > > Is it also possible for other backends? Or are some unable to store > > the UID and look up contacts (efficiently) by it? In that case we will > > have to relax the semantic of the API and accept that some backends > > still use their own local ID. "Supports UID" should be defined as a > > capability of the backend so that clients can take it into account. > > Hi, > I wouldn't call it "local UID", it's rather "backend's UID" I wouldn't use the term UID at all for this local (or backend) ID. UID has a specific meaning for both iCalendar 2.0 (where it is well-defined) and vCard (where it is less well-defined). Introducing yet another flavor of UID just leads to confusion, in particular when the same contact has both the original vCard UID and a local backend "UID". > and mostly > not, this is not doable for groupware backends which are using > server-supplied unique identifiers for contacts/calendars, like message > IDs in evolution-mapi, which talks to Exchange servers. It's easier, and > makes sense, to use server-side IDs, which are meant to be unique. > It's also a reason, from my point of view, why > e_data_book_respond_create_contacts() passes UIDs of newly created > objects back to the client. So we do need the concept of local IDs which are not the same as the creator-assigned UID. What about a CardDAV server? It has server-supplied IDs (the path to the resource) and a UID as part of the vCard stored there? How is that handled at the moment? And what does that mean for the file backend? I still think it would be good if the file backend supported creator-assigned UIDs. That other backends can't do that doesn't mean that file backend is not allowed to do it, right? > For example with Google calendar, which is served by CalDAV backend, the > backend fetches newly added event from the server only to present UI > with real object from the server, because the server can modify the > event on its own when creating it (it adds default alarms, if set in > Google's calendar preferences on the Web). Are you saying that Google Calendar EDS does *not* report the original iCalendar 2.0 UID? -- 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
[Evolution-hackers] UID in vCard
Hello! I'd like to write down some thoughts on vCard UID handling in EDS. Andrew mentioned it on IRC, so I'm copying him. Let me define the terms to make the difference clear: * UID: part of the contact properties. Ideally set once when a new contact is created in such a way that it is globally unique (same semantic as iCalendar 2.0 UID, not just unique inside the current address book). Can and should be passed around and preserved when exporting/importing contacts. * local ID: local meta data about a contact, only relevant inside an address book to look up contacts. The current situation is that EDS uses the vCard UID property for its own local ID. When trying to create a new contact with UID already set, that UID will be overwritten. This is problematic for interoperability (one cannot store a vCard as-is) and syncing (which would benefit considerably from a creator-assigned, fixed UID). This is partly an API and partly an implementation issue. The libebook<->e_addressbook_factory D-Bus API just passes vCards, so a vCard property has to be used for any local ID, and UID is the one that is used. e_book_add_contact() doesn't have an explicit "local ID" retval. Clients have to use e_contact_get_const(contact, E_CONTACT_UID) to retrieve the assigned local ID. Other APIs avoid that. For example, e_book_add_contact_async() + EBookIdAsyncCallback return the ID out-of-band. The more recent EBookClient API always uses separate ID retvals. However, in contrast to the EBook API it specifies that these are the UID of the created contact and not some kind of local ID. The older API left that undefined and just talked of "id". I see several ways forward: 1. Accept that a contact ID as used in the EBook API is the vCard UID and make it possible to set that UID when creating a new contact. That implies for adding a contact: * remove the code which overwrites the UID * add check for existence of the UID in the address book and a corresponding error code if it does 2. Use the local ID in the EBook API and support storing the vCard UID separately. Support lookup of a contact by UID. The advantage of the second approach is that it is potentially more efficient. I myself took advantage of that in the EDS<->QtContacts bridge code. But it never was a full solution (depended on 32 bit local IDs, which is okay for the file backend, but not all of them, and required patching EDS). The disadvantage of the second approach is an API change and/or extension. It has some potential advantages, but as the EDS<->QtContacts bridge is not used anymore, there is little incentive to do the extra work right away. So I suggest to pursue the first approach instead. I think it is possible for the file backend. Is it also possible for other backends? Or are some unable to store the UID and look up contacts (efficiently) by it? In that case we will have to relax the semantic of the API and accept that some backends still use their own local ID. "Supports UID" should be defined as a capability of the backend so that clients can take it into account. -- 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] SyncEvolution + EClient API + EXDATE regression (Bug #655253)
On Mi, 2011-09-14 at 08:28 +0200, Milan Crha wrote: > yes, I agree, it wasn't the right change, I'm sorry for that. Please > revert the commit and reopen the bug. I reverted the EXDATE part of the commit, but couldn't reopen the bug (Bugzilla does't offer me that option). Can you reopen 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] SyncEvolution + EClient API + EXDATE regression (Bug #655253)
On Di, 2011-09-13 at 17:59 +0200, Patrick Ohly wrote: > On Mo, 2011-09-12 at 09:09 +0200, Patrick Ohly wrote: > > On Mo, 2011-09-12 at 07:56 +0200, Milan Crha wrote: > > > On Fri, 2011-09-09 at 10:32 +0200, Patrick Ohly wrote: > > > > Milan, can you shed some light on why the patch solves #655253? I fail > > > > to see what e_cal_backend_file_modify_object() has to do with deleting > > > > one occurrence of a repeatable event. > > > > > > > > If the EXDATE was really necessary to avoid having the original and the > > > > detached recurrence show up, then IMHO adding the EXDATE only works > > > > around the real problem. The real problem is more likely to be in the > > > > matching against RECURRENCE-ID. > > > > > > Hi, > > > sure, the thing why I added it there is that when you move one instance > > > of a recurring event to another hour, then you are asked whether you > > > want to change time for all instances or only this instance. Moving only > > > this instance should create a detached instance, and create an exception > > > in the master object. > > > > No, creating the exception is not necessary. > > > > Suppose you have a VEVENT with RRULE which expands to a regular start > > time of one recurrence at, say, 20110912T09Z. Then the detached > > recurrence must have RECURRENCE-ID:20110912T09Z and it will > > *replace* the regular recurrence without having to add an EXDATE to the > > parent. That's part of the iCalendar 2.0 semantic. > > The root cause of the bug is that the detached recurrences, if created > with current Evolution master, do not get the same UID as the original > recurring event. I've verified that by looking at the resulting .ics > file. > > It works correctly in 2.32.4. > > Does anyone know where in the 3.x cycle this broke? Darn, I can no longer reproduce the problem. I was able to reproduce with the source from current master (thus my statement above about the UIDs not matching), but after removing the EXDATE code it all works fine: same UID, no duplicates shown even after restart. Even after restoring the original source it still works (same UID, EXDATE added). Milan, do you agree that the e_cal_util_remove_instances() call can and should be removed? Either way, it only works around whatever is causing the UID issue. Adding the EXDATE just pampers over that problem and still doesn't fix things like "remove all recurrences". Patch is here: diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c index 2dc9a90..e1cfbb3 100644 --- a/calendar/backends/file/e-cal-backend-file.c +++ b/calendar/backends/file/e-cal-backend-file.c @@ -2391,9 +2391,6 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, priv->comp = g_list_remove (priv->comp, recurrence); obj_data->recurrences_list = g_list_remove (obj_data->recurrences_list, recurrence); g_hash_table_remove (obj_data->recurrences, rid); - } else if (obj_data->full_object) { - /* add exception for the modified instance */ - e_cal_util_remove_instances (e_cal_component_get_icalcomponent (obj_data->full_object), icalt } /* add the detached instance */ -- 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] SyncEvolution + EClient API + EXDATE regression (Bug #655253)
On Mo, 2011-09-12 at 09:09 +0200, Patrick Ohly wrote: > On Mo, 2011-09-12 at 07:56 +0200, Milan Crha wrote: > > On Fri, 2011-09-09 at 10:32 +0200, Patrick Ohly wrote: > > > Milan, can you shed some light on why the patch solves #655253? I fail > > > to see what e_cal_backend_file_modify_object() has to do with deleting > > > one occurrence of a repeatable event. > > > > > > If the EXDATE was really necessary to avoid having the original and the > > > detached recurrence show up, then IMHO adding the EXDATE only works > > > around the real problem. The real problem is more likely to be in the > > > matching against RECURRENCE-ID. > > > > Hi, > > sure, the thing why I added it there is that when you move one instance > > of a recurring event to another hour, then you are asked whether you > > want to change time for all instances or only this instance. Moving only > > this instance should create a detached instance, and create an exception > > in the master object. > > No, creating the exception is not necessary. > > Suppose you have a VEVENT with RRULE which expands to a regular start > time of one recurrence at, say, 20110912T09Z. Then the detached > recurrence must have RECURRENCE-ID:20110912T09Z and it will > *replace* the regular recurrence without having to add an EXDATE to the > parent. That's part of the iCalendar 2.0 semantic. The root cause of the bug is that the detached recurrences, if created with current Evolution master, do not get the same UID as the original recurring event. I've verified that by looking at the resulting .ics file. It works correctly in 2.32.4. Does anyone know where in the 3.x cycle this broke? -- 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] SyncEvolution + EClient API + EXDATE regression (Bug #655253)
On Mo, 2011-09-12 at 07:56 +0200, Milan Crha wrote: > On Fri, 2011-09-09 at 10:32 +0200, Patrick Ohly wrote: > > Milan, can you shed some light on why the patch solves #655253? I fail > > to see what e_cal_backend_file_modify_object() has to do with deleting > > one occurrence of a repeatable event. > > > > If the EXDATE was really necessary to avoid having the original and the > > detached recurrence show up, then IMHO adding the EXDATE only works > > around the real problem. The real problem is more likely to be in the > > matching against RECURRENCE-ID. > > Hi, > sure, the thing why I added it there is that when you move one instance > of a recurring event to another hour, then you are asked whether you > want to change time for all instances or only this instance. Moving only > this instance should create a detached instance, and create an exception > in the master object. No, creating the exception is not necessary. Suppose you have a VEVENT with RRULE which expands to a regular start time of one recurrence at, say, 20110912T09Z. Then the detached recurrence must have RECURRENCE-ID:20110912T09Z and it will *replace* the regular recurrence without having to add an EXDATE to the parent. That's part of the iCalendar 2.0 semantic. > Note the issue wasn't shown when you did it, but only when you restart > evolution and e-calendar-factory, because the backend sends you the > master object without exceptions and a detached instance. I still think that the recurrence generation is really at fault. This seems to be a regression, the bug does not occur in Evolution 2.32.4. > Do you think there is a better solution for this? I would have to investigate some more. I compiled Evolution master in a chroot, but I haven't tried to actually run it yet - this is my main work machine. No promise that I'll get around to it soon. I want to release SyncEvolution 1.2 or at least the final release candidate this week before going on vacation end of the week. -- 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
[Evolution-hackers] SyncEvolution + EClient API + EXDATE regression (Bug #655253)
Hello! I have started integrating a patch by Chris Dumez which switches the SyncEvolution Evolution API from the legacy API to the new EClient API in EDS 3.2. I'm currently testing it in the nightly testing, with EDS compiled from source on Debian Unstable. Backend is the normal file backend. I see one regression: when adding a detached recurrence with e_cal_client_modify_object_sync(CALOBJ_MOD_THIS), an EXDATE is added to the parent event. While semantically correct as far as I know (the detached recurrence overrides the EXDATE, instead of the EXDATE canceling out the detached recurrence), it is a side effect which makes it hard to undo the addition of the detached recurrence. Calling e_cal_remove_object_with_mod(CALOBJ_MOD_ONLY_THIS) won't be enough, the caller would also have to know that it has to remove the EXDATE. It also breaks SyncEvolution's automated testing: http://syncev.meego.com/latest/head-unstable-amd64/5-evolution/Client_Source_eds_event_LinkedItems_0_testLinkedItemsParentChild.log This seems to come from this code: e_cal_backend_file_modify_object() 8631a8f2 (Milan Crha 2011-09-02 13:21:07 +0200 2394) } else if (obj_data->full_object) { 8631a8f2 (Milan Crha 2011-09-02 13:21:07 +0200 2395) /* add exception for the modified instance */ 8631a8f2 (Milan Crha 2011-09-02 13:21:07 +0200 2396) e_cal_util_remove_instances (e_cal_component_get_icalcomponent (obj_data->full_object), icaltime_from_string (rid), CALOBJ_MOD_THIS); commit 8631a8f2e0c1ca71a48aeca5a44a11506ac77e33 Author: Milan Crha Date: Fri Sep 2 13:21:07 2011 +0200 Bug #655253 - Delete of one occurrence of a repeatable event don't work This was added in 3.1.91. Milan, can you shed some light on why the patch solves #655253? I fail to see what e_cal_backend_file_modify_object() has to do with deleting one occurrence of a repeatable event. If the EXDATE was really necessary to avoid having the original and the detached recurrence show up, then IMHO adding the EXDATE only works around the real problem. The real problem is more likely to be in the matching against RECURRENCE-ID. -- 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
[Evolution-hackers] regression in e_cal_remove_object()
Hello! I'm have to report that the MOD_ONLY_THIS improvements (included in master, GNOME 3.0 and 2.32 branches, not released as .tar.gz as far as I know) caused a regression in e_cal_remove_object(): it no longer removes all VEVENTs with the given UID. That's because it uses MOD_THIS, instead of MOD_ALL. The backend works as intended. IMHO MOD_THIS is wrong for the intended and/or traditional semantic of e_cal_remove_object(). This regression in combination with another bug led to internal corruption in the file backend which shows up in SyncEvolution as "Object not found" errors. Patches here: https://bugzilla.gnome.org/show_bug.cgi?id=655986 Chenthill, can you review please? -- 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] improve PHOTO support in libebook/EDS
On Fri, 2011-07-08 at 14:45 -0400, Tristan Van Berkom wrote: > However the EContact passed to e_book_commit_contact() > will not be modified by the call to the backend (the > gdbus call simply queues a job in the addressbook thread > and returns)... so for the caller to get the real uri > one must further call e_book_get_contact() or have an > EBookView in play to receive it. I don't think EDS ever made the promise that an items would be saved exactly as sent. Apps which need an exact copy of the stored item indeed should use a view to get the real content of the database after requesting changes. > > > Perhaps the backend needs to claim that it supports a list of > > > protocols for staging data (such as 'file','http','ftp' etc) ? > > > > The staging dir would be, by definition, local. I don't think we should > > support anything else. > > > > Ah I had imagined it the other way around (that the addressbook > would take care of downloading any data from a client's directory). This is also what I had in mind. > But it seems more practical to leave file uploading to the client, We can design the API to make that possible, but I don't think we should worry to much about it now. > All in all then the added code I think would be: > >o An api for a client to get a staging directory > (which can return NULL) > > const gchar *e_book_get_staging_uri (); > > The api would be implemented with a new GDBus > method call or property which would in turn > be handled by EDataBook. > > EDataBook would implement it in a backend > specific way and return NULL if the backend > does not implement the new ->get_staging_uri() > vfunc. > >o EBookBackendFile (the BDB addressbook backend) will then > manage a staging directory and implement the new vfunc: > EBookBackendClass->get_staging_uri() to report the > file://path/to/staging/directory/ > > "Managing" the staging directory will only consist of creating > it at book creation time, removing it at book remove time and > emptying it out every time the book is freshly opened (just in > case). Do we want one staging directory per address book, or one staging directory per client? My proposal was to create one per client, because then client's don't have to worry about file name collisions (they are the only ones writing into it) and files can be deleted once the client exits. If we have a long-running user of the address book (like Evolution) and a short-lived one (like SyncEvolution) and only one staging directory, then files created and not properly committed by SyncEvolution cannot be cleaned until also Evolution quits. It might also be advantageous to have one directory per client process once MeeGo adds separate privileges for processes running under the same user ID. This is currently a pie in the sky, though. Note that the "private staging directory" is a bit at odds with allowing arbitrary URIs. While such a guarantee is easy for local directories, it might make less sense for other URIs (like HTTP resources + PUT). This could be resolved by making the guarantee only for file:// URIs. >o The local file backend will then move files from the staging > directory to the 'photos' directory and update the uris > in the stored vCards (whenever receiving vCard updates that > refer to files in the staging uri). > > If the addressbook receives uris in the staging directory which > have failed to upload or do not exist for any reason that will > be considered an error (the client needs to finish uploading > successfully before sending vcards holding staged uris of course). > > Clients can then try to get a staging uri, if they receive a staging > uri then they should check if they support the protocol. > > If the backend returns an ssh:// staging directory and the client cannot > deal with that, then the client should fall back to sending binary > blobs (whether or not there is a staging directory available, clients > can continue to send photo data as binary blobs reliably anyway). > > In theory clients can deal with the addressbook owned staging uri > pretty conveniently using GIO to check if the staging uri is supported > and to upload data. > > Perhaps this is not so very complex after all ? I think it is doable. -- 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] calendar events with category list - Evolution 2.32
On Fri, 2011-07-08 at 14:25 +0100, Valluri, Amarnath wrote: > When see this event in Evolution-2.32 UI(Import Assistant), only the > last(Business) category is shown. This does not only affect importing or syncing. The easiest way to reproduce the problem is to create an event in the Evolution UI with two categories selected. Only one category is visible in the UI after saving the event. I have not checked whether searching works; it might also be affected. > I could see the same issue reported by Patrick on Evolution-2.10 but > the issue is still present in 2.32. Not quite: the old issue was related to broken comma handling in libical, which caused two categories to be stored as CATEGORIES:abc\,xyz although the RFC specifies a simple comma as separator. Later libical was changed, which now seems to cause the current problem. Has anyone noticed this before? -- Patrick Ohly ___ 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] improve PHOTO support in libebook/EDS
On Thu, 2011-07-07 at 20:39 -0400, Tristan Van Berkom wrote: > I now have a much simpler patch up on the openismus-work branch[0]. > (it's also in patch form on the bug[1]). That looks a lot saner than the previous solution. Removing all the attempts to keep clients perfectly informed about file removal has paid off. I haven't done a detailed code review. I'd prefer to have the EDS experts do that. But perhaps one change first: this code seems useful for a variety of local backends. I think the bulk of it should be in the shared libedata-book library. > Also there is a strange use case worth pointing out: > > You are not allowed to use an addressbook generated uri as the uri > for another field of the same contact or another contact. > > So you are allowed to set multiple contacts photo fields > to point to "http://hostyouravatar.com/buster.png";, but you > are not allowed to do: > >/* Get that contact named "Elvis Presley" */ >PhotoContact *photo = e_contact_get (contact_a, E_CONTACT_PHOTO); > >/* Use Elvis's face on this new contact I'm creating */ >PhotoContact *new_photo = create_photo (photo->data.uri); > >e_book_commit_contact (new_photo); > > Not sure if that's an acceptable rule or not, if not then we > either need to add some complex code to implement internal > refcounting on the uris in the backend... or perhaps we > could trap the incoming shared uris and create copies > on disk instead... I would trap the incoming URI and create a hardlink under a different name. That'll share the data without requiring us to implement refcounting in the backend. > In any case, I think we could try to close this first and > then look at the staging directory feature separately, seeing > that this code will very easily scale from: > a.) "If the incoming photo is a binary blob" to... > b.) "If the incoming photo is a binary blob or > a uri inside the staging directory..." I agree. We can always do another iteration if the D-Bus overhead for storing photos becomes important. -- 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] improve PHOTO support in libebook/EDS
On Thu, 2011-07-07 at 20:54 -0400, Tristan Van Berkom wrote: > It's possible but will need to be conditional, probably depending on > whether there is a staging directory available or not. > > I'll start thinking about how this staging directory could be > implemented, I suppose the client needs to create one somehow > while opening the EBook, and only on the condition that the > backend can handle incoming data from a staging directory. The EDS daemon needs to create the directory. That's required for the cleanup rules to work. I would do it in the backend, possibly with some server-side utility code that can be used by multiple backends. > Possibly at book creation time one needs to specify an > actual directory for this (or has the option to specify > a directory)... The backend should choose. That way it can pick something that is suitable (like on the same filesystem) without exposing implementation details to the client. > How do remote backends handle uris from the local staging dir ? They'll simply refuse to create a staging directory and thus request that the client continues to inline data. > perhaps the staged uris would be sent as ftp:// to the remote > backend but locally stored in a file:// uri... does the backend > need to participate in the formatting of the uris that will > be sent to it ? That is one possibility. Don't forget that the local file backend also has to rewrite the URI (from staging dir to final location). > Perhaps the backend needs to claim that it supports a list of > protocols for staging data (such as 'file','http','ftp' etc) ? The staging dir would be, by definition, local. I don't think we should support anything else. -- Best Regards Patrick Ohly Senior Software Engineer Intel GmbH Open Source Technology Center Pützstr. 5 Phone: +49-228-2493652 53129 Bonn Germany ___ 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] improve PHOTO support in libebook/EDS
On Fri, 2011-07-08 at 10:42 +0100, Burton, Ross wrote: > On 7 July 2011 09:48, Patrick Ohly wrote: > >> At any rate, if it's judged that the performance gain of using > >> a staging directory is worth the added complexity then let's do it. > > > > I'd like to hear some other opinions about this. Ross? > > It does sound like a lot of extra complexity and complication for the > performance gain... So what do you suggest? Inline the data when storing a photo and strip it out in the backend? -- 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] improve PHOTO support in libebook/EDS
On Thu, 2011-07-07 at 15:09 +0530, Chenthill wrote: > I like the staging directory approach. Would it be possible for > implementing the logic to convert the inline data ->file inside > e_book_client_add_contact so that the clients need to worry about > changing the code ? "need *not* worry", right? Yes, I can imagine that this would work. Should we allow to enable or disable this client side? In other words, should we force the separate storing of photo data in all cases or is there a legitimate situation where a client might want to force the data to be stored inline? I can't think of one myself. Note that once we introduce this mechanism (whether it is mandatory or not), all clients must be prepared to deal with PHOTO URIs. But this is not new, because there might already be contacts in current EDS with URIs. The only change would be that clients get an URI for their *own* contacts even if those had inline data. -- 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] improve PHOTO support in libebook/EDS
On Wed, 2011-07-06 at 16:16 -0400, Tristan Van Berkom wrote: [staging directory for out-of-band photo data transmission] > This alternative proposal is strictly regarding the photo data for > which the server must take ownership of I assume, correct ? Yes. > I might not have been clear enough in my proposal but I only > intend for new or modified photos to be sent to the addressbook. Understood. I personally don't mind the D-Bus overhead in these cases because I consider them rare. But there are people who run massive write benchmarks against EDS anyway and do flag low performance in them as a problem. So if we can find a solution that also optimizes the write performance, then it would be better. > The staging directory you propose if I understand it correctly > would also be used under only the same circumstances, I suppose > the questions are: > o What is faster: writing a file to a staging directory and > then moving it to a permanent location or sending the blob > once over D-Bus and saving it to the permanent location. If the photo data needs to be stored as file and moving the file into its permanent location can be done with a rename, then the staging directory approach is faster. I expect writing the file to be equally fast, regardless of who does it. Sending inline data adds D-Bus overhead (considerable for large chunks of data!) and encoding/decoding overhead. > o What is more reliable ? (it seems that there are less > points of failure when sending the blob but I could be wrong). Agreed. I tried to mitigate that with the rules around cleaning up the staging directory. > Perhaps the staging directory is a little faster when batching lots > of photo modifications during a 'sync' operation ? (the client > gets to write to disk while waiting for the server to be ready > for the next batch of contact additions perhaps ?) > > At any rate, if it's judged that the performance gain of using > a staging directory is worth the added complexity then let's do it. I'd like to hear some other opinions about this. Ross? > I think the more important issue at hand is that to offer > a reliable api for EBookView then the backend has to track > the views which have received notification of the contact > modifications. > > In other words when I have an EBookView and I have been > notified that a contact exists with a photo at a given uri path, > then until I receive notification that that uri is invalid, > removed or replaced with a new one... I should be able to > access that file. I don't see this as a problem. When a process fails to load a photo because the contact was already removed on the server together with that photo, then shortly after the attempt to read the photo the process will get the notification that the contact is gone and thus the process no longer needs the photo. I also think that this is a very rare situation. Definitely doesn't justify adding complex code to the D-Bus interface between libebook and EDS. > Another hard limitation is regarding storing uris for which > you don't want the addressbook to assume ownership of. > > The clean way as far as I can see is whoever is responsible > for adding an 'alien uri' to an addressbook is responsible > for removing that entry from the addressbook at the time > that the uri might be deleted. Agreed. But I don't think there is a clean and efficient way of solving this. It'll be much simpler to accept that there will be URIs that point to deleted files. The intention is to not use URIs pointing to arbitrary local files. Therefore we don't need to provide a solution that assists apps who want to do that. > Well... perhaps we should just swallow the ugly race and do nothing > about it and say that: > "a uri might be invalid from time to time directly before your view >receives notification of the removal" ? > > Would that be acceptable ? Yes. -- 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
[Evolution-hackers] improve PHOTO support in libebook/EDS
Hello! It was pointed out before that support for PHOTO data in EDS is sub-optimal because the data has to be encoded as B64 data in the vCard and then gets transmitted like that via D-BUs and stored in Berkley DB. A high-level description of the problem is here: http://wiki.meego.com/Architecture/planning/evolution-data-server/eds-improvements#Contacts:_Store_PHOTO_Data_as_Plain_Files Photos should be transferred over D-Bus via a filepath to a local file rather than transferring the actual photo data over D-Bus. The VCard format already allows this option (See the PHOTO property). We must be careful to keep these local files in sync and to remove them when contacts are removed, and to restrict access to the files from other processes. There should be utility code available to libebook users to convert between the different representations. Goals: 1. Apps should be able to create photo files without ever passing the data to libebook. 2. External PHOTO file and corresponding contact must be kept in sync: 1. If a contact is deleted, the photo also needs to be removed. 2. If a photo file is created and then storing the contact fails, who removes the file? 3. Access to photo data must be limited to processes which have the right permissions. --- There also was a proposal for how to solve this, but I am no longer entirely happy with it. Openismus is working on this at the moment, see https://bugzilla.gnome.org/show_bug.cgi?id=652178 Before diving further into prototyping or implementing a solution, I would like to come to an agreement about the client visible behavior and a rough outline how to implement it. First let me clarify that we need to distinguish two different use cases: 1. The photo app owns and manages a set of photos. A link is added to a contact. If the contact is deleted, the photo must remain because it is part of a slide show. If the photo is moved or renamed, the link remains but is obviously no longer useful ("dangling link" as in a symbolic link in a file system). 2. Photo data is attached to a contact. EDS becomes the owner for the data and must ensure that it is stored and deleted together with the contact. Apps get access to the photo via its URI, exactly as in the first case. The extension that we are discussing addresses the second point. It must not break the first one. Tristan's plan seems to be to pass the PHOTO data inline in the vCard to the file backend and to strip it out there. Note that this is not optimal in terms of performance: if the data is already available separately (as it is in syncing, for example), then it has to be inlined and transferred via D-Bus (slow). This is acceptable because contacts are read a lot more often than written. But that doesn't stop people from running write benchmarks ;-) I'd only go down that route if it really makes the implementation a lot simpler. From the description in 652178 and private email it doesn't sound like it necessarily does. How about a different approach: * Each EBookClient can request a "staging" directory. If the backend supports this mechanism, it is created for this client by EDS. If not, the client has to inline PHOTO data as before. * The content of the staging directory is ephemeral. When a client crashes or disconnects, EDS removes the directory and everything inside it. When EDS crashes, it cleans up all staging directories when it restarts. * If a client wants EDS to take ownership of the photo, it creates a file in the staging directory with an arbitrary name and sets the PHOTO URI such that it points to it with an absolute path. * The client submits the contact. At that point it stops being responsible for the file. * The EDS backend checks for such special PHOTO URIs. If it finds one, it copies or moves the file to its permanent location and updates the URI before storing the contact and notifying clients. Note that backends are free to do that however they want. For example, a CardDAV backend might upload the file data to a resource on the HTTP server and set the PHOTO URI to a http:// URL. The file backend chooses a staging directory in the same file system as the permanent photo directory and thus can simply move the file instead of having to copy it. libebook might offer some additional utility code to simplify the handling of such a staging directory and PHOTO URIs. API to be decided, also for the "request a staging directory" part. -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ ___ evolution-hackers
Re: [Evolution-hackers] Categories
On Do, 2011-06-23 at 07:14 -0400, Adam Tauno Williams wrote: > When saving a task or event over a CalDAV collection Evolution sends a > CATEGORIES attribute like: > > CLASS:PUBLIC > CATEGORIES:Favorites\,Gifts\,Goals/Objectives > PERCENT-COMPLETE:0 > > Looking at RFC2445 I'm pretty sure this is wrong. Upstream libical used to have a bug around that. It should be fixed in 0.44. I don't know whether Evolution itself still has it wrong somewhere. -- 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-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 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] [EDS PATCHES] ecal item semantic (was: [Fwd: Re: e_cal_remove_object_with_mod() + empty rid: semantic?])
On Mo, 2011-06-06 at 08:00 +0530, chen wrote: > Can you please attach these patches on to bugzilla ? I will put the > comments there. I can do it myself, but there is no option to specify > the author, so it would be better if you can do it.. Sure. See https://bugzilla.gnome.org/show_bug.cgi?id=651970 -- 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] Issues with new EBook dbus implementation
On Fr, 2011-05-27 at 11:59 -0400, Matthew Barnes wrote: > On Fri, 2011-05-27 at 16:49 +0200, Milan Crha wrote: > > Maybe it can pump them, but it doesn't deliver them, as far as my tests > > showed, because they are usually delivered on idle, which never happen > > with blocked main loop. That's the reason why I added there this loop. > > Only notice that this loop is running for "sync" calls, and only if it's > > done from the main thread. The async calls and sync calls from a > > dedicated thread doesn't do any such thing. You may not use sync calls > > in your application, preferably. (Same as evolution shouldn't, which is > > subject to change, to use the EBOok/CalClient APIs directly.) > > Can't we just consider it a programming error for a D-Bus method to be > called synchronously from the main thread? I don't see any reason to > support this case since it would be by definition a bug. I beg to differ. A command line tool like SyncEvolution is perfectly usable with blocking the main thread in the synchronous API calls. Or rather, it works as good as the synchronous API implementation. One problem are timeouts. The other is that the calls don't detect when the server dies because the corresponding D-Bus signal is not delivered (seen in older EDS releases, not sure how relevant it is on master). As Alexander said, the synchronous API serves a useful purpose. SyncEvolution is one, simple tests another. +1 for keeping it and fixing it so that it really works. -- 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
[Evolution-hackers] [EDS PATCHES] ecal item semantic (was: [Fwd: Re: e_cal_remove_object_with_mod() + empty rid: semantic?])
Hello Chenthill! You are the calendar maintainer, right? Do you have some time to review these patches? On this occasion let me add some further explanation of the motivation for this patch series. Traditionally, libecal has implemented parts of the semantic typically associated with a UI: for example, it implements "ensure that a recurring event does not occur at this point in time, no matter what it takes to achieve that". I consider this a high-level API. The low-level API would be "remove/add/modify this VEVENT". The libecal API *looks* like it supports that and according to our previous discussion is meant to (see the comments about e_cal_remove_object_with_mod()), but the actual implementation differs. This is a problem for sync operations, where that semantic is implemented elsewhere and the calendar storage has to mirror the operations done there (CalDAV/SyncML server in SyncEvolution; KCal in the MeeGo architecture). Therefore this patch series adds the missing operations. This is done so that the file backend can replace other local storages that EDS is being compared against. I understand that the vision of EDS goes beyond just being a local storage, but that's irrelevant in the context of such comparisons (unfortunately). -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ --- Begin Message --- On Mo, 2011-05-16 at 18:06 +0200, Patrick Ohly wrote: > I'll do it as [\n] so that entries without an rid continue to > look exactly like the current ones. Looks good. I ran my KCal-EDS test program which adds, modifies and removes events, including parent and child (= detached recurrence) in various orders. I am now seeing all ECalView signals that I expect and need. I also ran Evolution in parallel to the test and saw it react to all signals, but not quite as I would have liked. Evolution seems to interpret uid + empty rid as "all recurrences removed", which IMHO is wrong. It should mean "parent removed", because otherwise there is no way of expressing that change. For "child removed, parent not updated (= no EXDATE added)" I would expect Evolution to show the original, unmodified recurrence again. Instead it only removes the recurrence (as if EXDATE had been added). I consider this an Evolution bug which can and should be fixed separately. Please understand that it is currently lower priority for me, my main goal has to be to get EDS working as intended in EKCal-EDS, without Evolution. Please have a look at the attached patch series. They were tested with the gnome-2-32 branch. Except for the very last one, all apply to "master". Does this look okay? May I submit to "master" (after fixing the last patch), gnome-3-0 and gnome-2-32? -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ >From 2e128c63a4a9dd64a3a327262498deb869a119f7 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 11 May 2011 16:59:51 +0200 Subject: [PATCH 1/8] calendar file backend: support removing parent event with CALOBJ_MOD_THIS It was possible to create a meeting series with just a detached event (RECURRENCE-ID set) by importing a meeting invitation for that single recurrence. It was not possible to arrive at that same state after adding the parent event (the one with the RRULE) because e_cal_remove_object_with_mod() removed all instances for CALOBJ_MOD_THIS and empty rid. This contradicts the intended semantic of e_cal_remove_object_with_mod(): "By using a combination of the @uid, @rid and @mod arguments, you can remove specific instances. If what you want is to remove all instances, use e_cal_remove_object instead." This patch implements the desired semantic: - e_cal_backend_file_remove_object(CALOBJ_MOD_THIS) now always calls remove_instance(). - remove_instance() was extended to also work for the parent event. - That call removes the complete object if nothing is left after removing the instance. This case must be handled by the caller. The return value is the original object (if it still exists) and NULL if not. - Because the uid pointer into the object may become invalid as part of the removal, a more permanent pointer has to be provided by the caller. --- calendar/backends/file/e-cal-backend-file.c | 134 ++- 1 files changed, 88 insertions(+), 46 deletions(-) diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c index 33bab50..7a8c450 100644 --- a/calendar/backends/file/e-cal-backend-file.c +++ b/calendar/backends/file/e-cal-backend-file.c @@ -2629,47 +2629,94 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const g_static_rec_mutex_unlock (&priv->idle_save_rmutex); } -static void -remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *rid) +/** + * Remove one and only one instance. The object may be em
Re: [Evolution-hackers] [Reminder] Evolution community meeting at #evolution-meet channel - May 18 - 3:30 PM UTC
On Di, 2011-05-17 at 08:41 +0530, chen wrote: > * Project updates > * Discussion on queries/decisions > * Individual updates I had to take a phone call and couldn't provide my individual update. Let me add it via email: I consider the libecal API fixes ready for review and merging. Someone please have a look and provide feedback. -- 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] 32 bit IDs in contact file backend
On Mi, 2011-05-18 at 07:34 +0200, Milan Crha wrote: > On Tue, 2011-05-17 at 17:19 +0200, Patrick Ohly wrote: > > On Di, 2011-05-17 at 16:25 +0200, Milan Crha wrote: > > > On Tue, 2011-05-17 at 15:59 +0200, Patrick Ohly wrote: > > > I'm not sure if I got it right, but such workarounds are just > > > wrong from my point of view. You cannot force servers to use > > > certain types of IDs because of constraints given by application > > > which tries to connect to it. > > > > We can't force a server. But we can adapt the file backend. > > That's what I didn't get fully from your conversation with David. What > is the gain to change file backend when there are all other backends > which will be unusable with QtContacts because they cannot do 32-bit int > UIDs? Once we have the mapping, it is an optimization. Without the mapping, it is the only way to get the system working. > Was it "just" to have it done "natively" in the most common > backend and all the rest will use mappings in the QtContacts<->EDS > interface? Yes. > It might be harder to spot bugs in the first or the second > part of the code (with or without intermediate mappings), isn't it? This argument can be turned around: because we don't need the mapping with the 32 bit patch applied, we can focus on the rest of the code, test in a working system, and later extend 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] 32 bit IDs in contact file backend
On Di, 2011-05-17 at 16:25 +0200, Milan Crha wrote: > On Tue, 2011-05-17 at 15:59 +0200, Patrick Ohly wrote: > I'm not sure if I got it right, but such workarounds are just wrong from > my point of view. You cannot force servers to use certain types of IDs > because of constraints given by application which tries to connect to > it. We can't force a server. But we can adapt the file backend. > Your interface between QtContacts and eds may hide this QtContacts > implementation detail, to be able to support any book backend, current > or future. This might not be done in eds backend itself, from my point > of view. "might not" or "must not"? Anyway, the direction is clear: add the mapping code to QtContacts-EDS, as soon as time allows. We are juggling resources between various tasks at the moment, so I'd say as long as we have a solution that works for the time being, I'd rather see other problems addressed first. While we do that, the 32 bit patch should remain outside upstream EDS. Once we have the mapping code, we can revisit that question by doing a direct comparison with and without 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] 32 bit IDs in contact file backend
On Di, 2011-05-17 at 18:49 +0530, Chenthill wrote: > On Tue, 2011-05-17 at 14:51 +0200, Patrick Ohly wrote: > > | Further work if you agree in principle: > > | * let clients query whether all contacts have the simplified ID - > > |could be done with the dynamic capabilities that I mentioned in > > |the e-client API thread; avoids reading all contact (UIDs) > This sounds like a better solution than adding constraints on eds end. I prefer to think of it as an additional promise to the libebook user, not a constraint. > One possibility which came to my mind [requires more thoughts] was to > bring the uid generation (e_book_backend_file_create_next_uid) function > into EBookBackend as a virtual function. Then you could have a external > backend for qtcontacts subclassing EBookFileBackend and over-riding the > create_next_uid function alone. > > I haven checked if other backend's would need this virtual function > though. Maybe webdav might use it ? And then what? Build and maintain out-of-tree MeeGo versions of all backends? Quite frankly, patching the existing backends sounds less risky to me. Of course, including these changes upstream would be best. -- 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] 32 bit IDs in contact file backend
On Di, 2011-05-17 at 13:27 +0100, David Woodhouse wrote: > On Tue, 2011-05-17 at 14:04 +0200, Patrick Ohly wrote: > > On Di, 2011-05-17 at 12:38 +0100, David Woodhouse wrote: > > > Even if we *didn't* have immediate plans to use other back ends like EWS > > > with this setup, that would be entirely the wrong thing to do, surely? > > > > I'm not so sure. We are pitching EDS as an alternative for other storage > > solutions that are highly optimized (= limited!) for specific use cases. > > What you are suggesting is that any attempt to add optimizations for a > > specific combination of app + EDS + backend is wrong and should be > > avoided. My feeling is that EDS will simply not be used at all unless > > such optimization are acceptable. > > [EDS upstream] > > I have no objection to an *optimisation*. You seemed to be describing a > *fix*, not an optimisation. > > An *optimisation* allows things to work faster or more efficiently, when > they were already working before. > > So if you expose an extra '32bit-numeric-uid' in your static > capabilities for the back end, and the user can make use of that to > operate more efficiently by bypassing the permanent uidstring<->integer > mapping, then I'm happy with that. That was the plan. From the original email: | Further work if you agree in principle: | * let clients query whether all contacts have the simplified ID - |could be done with the dynamic capabilities that I mentioned in |the e-client API thread; avoids reading all contact (UIDs) > But *only* if it really is an > optimisation, and designed such that the code still works (via the > mapping) without it. I can't promise that the code will work without it right away because the mapping hasn't been implemented yet due to lack of time. See also: http://lists.meego.com/pipermail/meego-dev/2011-May/483078.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] 32 bit IDs in contact file backend
On Di, 2011-05-17 at 12:38 +0100, David Woodhouse wrote: > On Thu, 2011-04-28 at 15:16 +0200, Patrick Ohly wrote: > > As part of wrapping QtContacts around EDS [1] I ran into the same issue > > that Nokia already encountered in their Maemo 5 [2] backend: EDS uses > > strings as ID, QtContacts 32 bit integers. > > > > Nokia solved that by setting up an in-memory hash which maps the UID > > string to its CRC-16. I don't see any checks for the (possible) hash > > collisions. IMHO a proper solution has to keep a permanent mapping on > > disk, otherwise the 32 bit IDs won't be stable. > > > > Overall not a nice solution. My attempt to make it nicer at least in > > combination with the file backend (the main goal for QtContacts-EDS) > > focused on simplifying the problem instead: with 32 bit IDs in the > > storage, the mapping becomes easy. > > While I understand what you're saying when you say 'nicer', this seems > to be a fundamentally *wrong* approach. > > You're suggesting that a user of the EDS API should rely on internal > implementation details of a single back end, which don't even apply to > other back ends. > > Even if we *didn't* have immediate plans to use other back ends like EWS > with this setup, that would be entirely the wrong thing to do, surely? I'm not so sure. We are pitching EDS as an alternative for other storage solutions that are highly optimized (= limited!) for specific use cases. What you are suggesting is that any attempt to add optimizations for a specific combination of app + EDS + backend is wrong and should be avoided. My feeling is that EDS will simply not be used at all unless such optimization are acceptable. > Or is this hack planned to be *extremely* limited for MeeGo 1.2, and > dropped in some way (perhaps by *fixing* the QtContacts API) by 1.3? In > which case, perhaps it really would make more sense to do it with a > persistent mapping in your wrapper? There are no plans to fix the QtContacts API. I agree that adding a mapping to QtContacts-EDS is useful and should be done - eventually. Right now, the 32 bit EDS patch is the easiest (and only) solution that we have for the problem. If there is no interest in it upstream, then I would at least like to use it in MeeGo. -- 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] 32 bit IDs in contact file backend
On Do, 2011-04-28 at 15:16 +0200, Patrick Ohly wrote: > Attached the resulting patch. Note that with the patch applied, all new > contacts in a Berkley DB get the simpler IDs, unconditionally. Older > contacts continue to use their existing IDs. Would something like this > be acceptable upstream? Any thoughts on this? -- 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] e_cal_remove_object_with_mod() + empty rid: semantic?
On Mo, 2011-05-16 at 18:06 +0200, Patrick Ohly wrote: > I'll do it as [\n] so that entries without an rid continue to > look exactly like the current ones. Looks good. I ran my KCal-EDS test program which adds, modifies and removes events, including parent and child (= detached recurrence) in various orders. I am now seeing all ECalView signals that I expect and need. I also ran Evolution in parallel to the test and saw it react to all signals, but not quite as I would have liked. Evolution seems to interpret uid + empty rid as "all recurrences removed", which IMHO is wrong. It should mean "parent removed", because otherwise there is no way of expressing that change. For "child removed, parent not updated (= no EXDATE added)" I would expect Evolution to show the original, unmodified recurrence again. Instead it only removes the recurrence (as if EXDATE had been added). I consider this an Evolution bug which can and should be fixed separately. Please understand that it is currently lower priority for me, my main goal has to be to get EDS working as intended in EKCal-EDS, without Evolution. Please have a look at the attached patch series. They were tested with the gnome-2-32 branch. Except for the very last one, all apply to "master". Does this look okay? May I submit to "master" (after fixing the last patch), gnome-3-0 and gnome-2-32? -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ >From 2e128c63a4a9dd64a3a327262498deb869a119f7 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 11 May 2011 16:59:51 +0200 Subject: [PATCH 1/8] calendar file backend: support removing parent event with CALOBJ_MOD_THIS It was possible to create a meeting series with just a detached event (RECURRENCE-ID set) by importing a meeting invitation for that single recurrence. It was not possible to arrive at that same state after adding the parent event (the one with the RRULE) because e_cal_remove_object_with_mod() removed all instances for CALOBJ_MOD_THIS and empty rid. This contradicts the intended semantic of e_cal_remove_object_with_mod(): "By using a combination of the @uid, @rid and @mod arguments, you can remove specific instances. If what you want is to remove all instances, use e_cal_remove_object instead." This patch implements the desired semantic: - e_cal_backend_file_remove_object(CALOBJ_MOD_THIS) now always calls remove_instance(). - remove_instance() was extended to also work for the parent event. - That call removes the complete object if nothing is left after removing the instance. This case must be handled by the caller. The return value is the original object (if it still exists) and NULL if not. - Because the uid pointer into the object may become invalid as part of the removal, a more permanent pointer has to be provided by the caller. --- calendar/backends/file/e-cal-backend-file.c | 134 ++- 1 files changed, 88 insertions(+), 46 deletions(-) diff --git a/calendar/backends/file/e-cal-backend-file.c b/calendar/backends/file/e-cal-backend-file.c index 33bab50..7a8c450 100644 --- a/calendar/backends/file/e-cal-backend-file.c +++ b/calendar/backends/file/e-cal-backend-file.c @@ -2629,47 +2629,94 @@ e_cal_backend_file_modify_object (ECalBackendSync *backend, EDataCal *cal, const g_static_rec_mutex_unlock (&priv->idle_save_rmutex); } -static void -remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *rid) +/** + * Remove one and only one instance. The object may be empty + * afterwards, in which case it will be removed completely. + * + * @uidpointer to UID which must remain valid even if the object gets + * removed + * @ridNULL, "", or non-empty string when manipulating a specific recurrence; + * also must remain valid + * @return modified object or NULL if it got removed + */ +static ECalBackendFileObject * +remove_instance (ECalBackendFile *cbfile, ECalBackendFileObject *obj_data, const gchar *uid, const gchar *rid) { gchar *hash_rid; ECalComponent *comp; struct icaltimetype current; - if (!rid || !*rid) - return; + /* only check for non-NULL below, empty string is detected here */ + if (rid && !*rid) + rid = NULL; - if (g_hash_table_lookup_extended (obj_data->recurrences, rid, (gpointer *)&hash_rid, (gpointer *)&comp)) { - /* remove the component from our data */ + if (rid) { + /* remove recurrence */ + if (g_hash_table_lookup_extended (obj_data->recurrences, rid, + (gpointer *)&hash_rid, (gpointer *)&comp)) { + /* remove the component from our data */ + icalcomponent_remove_component (cbfile->priv->icalcomp, + e_cal_component_get_icalcomponent (comp)); + cbfile->priv->comp = g_list_remove (cbfile->priv->comp, comp); + obj_data->recurrences_list = g_list_rem
Re: [Evolution-hackers] e_cal_remove_object_with_mod() + empty rid: semantic?
On Do, 2011-05-12 at 13:17 +0200, Milan Crha wrote: > On Thu, 2011-05-12 at 12:44 +0200, Patrick Ohly wrote: > > I found this in e-data-cal-view.c notify_remove(): > > > > 280 /* TODO: store ECalComponentId instead of just uid*/ > > 281 uid = g_strdup (id->uid); > > 282 g_array_append_val (priv->removes, uid); > > > > In other words, removes always come without rid, even if the actual > > event that was removed was not the parent but a detached recurrence. > > ... > > It'll lead to a change of the D-Bus API. For "master" that shouldn't > > be a problem, but I also want this in MeeGo for KCal-EDS, based on > > 2.32. I guess we have to bite the bullet and maintain a MeeGo > version > > with a different API than regular 2.32.x. > > Hi, > I noticed it too, recently (at the beginning of this week, actually). > I suggest to not change the D-Bus API, rather encode the string pair > into the string, like "uid\nrid". I was planning to do so in the > master branch, but didn't do that yet. That's still a D-Bus API change: a libecal which receives a UID entry in that format won't know what to do with it. But it'll be easier to implement that way, so I'll give it a try first. I'll do it as [\n] so that entries without an rid continue to look exactly like the current ones. -- 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] libdb performance issue? (was: Re: libebook: errors when using asynchronous contact addition/removal functions)
On Mo, 2011-05-16 at 13:05 +0200, Milan Crha wrote: > On Mon, 2011-05-16 at 11:35 +0200, Patrick Ohly wrote: > > And it works beautifully. One word added to the source code and the > > stress test passes reliably and quickly :-) > > > > Patch attached. Okay to submit into master (not tested there, though) > > and gnome-2-32 branches (which is where I have tested it)? > > Yes please, master, gnome-3-0 and if you want then gnome-2-32 too. > Thanks. Okay, done. -- 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] libdb performance issue? (was: Re: libebook: errors when using asynchronous contact addition/removal functions)
[Milan, can you please keep other people on CC? I know that you personally prefer to not be CCed on mailing list emails and I try to remember that in my own replies, but others are not on the list and may depend on being CCed. For example, I don't know whether Chris is subscribed.] On Mo, 2011-05-16 at 11:12 +0200, Patrick Ohly wrote: > It might be easier to set DB_INIT_CDB, which enforces multiple > reads/single writer access without deadlocks. I'll give that a try. And it works beautifully. One word added to the source code and the stress test passes reliably and quickly :-) Patch attached. Okay to submit into master (not tested there, though) and gnome-2-32 branches (which is where I have tested it)? -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ >From 2b4cff9230a049d5ad7fe86873aba9a7bba35af1 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 16 May 2011 11:21:04 +0200 Subject: [PATCH] addressbook file backend: libdb must be initialized for concurrent read/write Very bad performance (100% CPU load, several minutes run time) were seen for multiple concurrent writes. gdb shows that libdb is apparently busy polling while writing. The libdb API docs for DB_ENV->open() imply that either DB_INIT_CDB or DB_INIT_LOCK must be used in apps which are not read-only, like EDS. This patch adds DB_INIT_CDB because it is simple and fixes the performance problem. In some rare cases, DB_INIT_LOCK might provide better performance by allowing concurrent writes of independent data, but that seems too complicated for not enough gain right now (must check for deadlocks). --- addressbook/backends/file/e-book-backend-file.c | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c index d21c527..372ade5 100644 --- a/addressbook/backends/file/e-book-backend-file.c +++ b/addressbook/backends/file/e-book-backend-file.c @@ -1146,7 +1146,18 @@ e_book_backend_file_load_source (EBookBackend *backend, (gpointer (*)(gpointer , gsize))g_try_realloc, g_free); - db_error = (*env->open) (env, NULL, DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE | DB_THREAD, 0); + /* + * We need either DB_INIT_CDB or DB_INIT_LOCK, because we will have + * multiple threads reading and writing concurrently without + * any locking above libdb. + * + * DB_INIT_CDB enforces multiple reader/single writer by locking inside + * the database. It is used instead of DB_INIT_LOCK because DB_INIT_LOCK + * may deadlock, which would have to be called in a separate thread. + * Considered too complicated for not enough gain (= concurrent writes) + * at this point. + */ + db_error = (*env->open) (env, NULL, DB_INIT_CDB | DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE | DB_THREAD, 0); if (db_error != 0) { env->close(env, 0); g_warning ("db_env_open failed with %s", db_strerror (db_error)); -- 1.7.2.5 ___ 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] libdb performance issue? (was: Re: libebook: errors when using asynchronous contact addition/removal functions)
On Mo, 2011-05-16 at 08:11 +0200, Milan Crha wrote: > On Fri, 2011-05-13 at 15:44 +0200, Patrick Ohly wrote: > > In libebook, I get a "Timeout was reached" because the asynchronous > > operation doesn't complete quickly enough. Same for the attempt to > > delete the contacts. The gError->code is 24, which is indeed > > E_BOOK_ERROR_NOT_SUPPORTED. > > Hi, > as long as it's GError, you cannot compare only GError::code, because > the correct way to identify the error is using the whole pair > GError::domain and GError::code. Your domain for "Timeout was reached" > is most likely G_IO_ERROR with a code G_IO_ERROR_TIMED_OUT. Indeed, that explains it. In principle I know that, I just forgot to check in this case ;-} > The new eclient stuff will not suffer of this (unless some backend will > block main thread for too long). The EClient may not time out, but taking several minutes of 100% CPU time just to safe a handful of contacts just can't be right. > I've no idea about libdb, thus only referring to the error. I have one > notice though, the EBook calls are served and processed mostly as soon > as they come, and they can come "in parallel" too. The backend itself > may make sure that it'll not make any trouble, like by some busy lock. libdb is supposed to be thread-safe, so in theory shouldn't need such a lock. Either libdb is broken or EDS is not using it correctly (wrong environment, for example). Looking at the DB API [1] I see this flag for opening the environment: DB_INIT_LOCK Initialize the locking subsystem. This subsystem should be used when multiple processes or threads are going to be reading and writing a Berkeley DB database, so that they do not interfere with each other. If all threads are accessing the database(s) read-only, locking is unnecessary. When the DB_INIT_LOCK flag is specified, it is usually necessary to run a deadlock detector, as well. See db_deadlock and DB_ENV->lock_detect() for more information. EDS does not pass this flag, although it has concurrent reads *and* writes. The behavior in that case is undefined as far as I can tell. The downside of using DB_INIT_LOCK is that deadlocks are possible and need to be checked by regularly calling lock_detect(). It might be easier to set DB_INIT_CDB, which enforces multiple reads/single writer access without deadlocks. I'll give that a try. [1] http://download.oracle.com/docs/cd/E17076_02/html/api_reference/C/envopen.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] e_cal_remove_object_with_mod() + empty rid: semantic?
On Fr, 2011-05-13 at 12:48 +0100, Ross Burton wrote: > On 12 May 2011 11:44, Patrick Ohly wrote: > > Can you perhaps comment? You wrote the TODO items below... > [snip] > > I can fix these TODOs. Any objections or concerns? > > None whatsoever, those are embarrassingly leftin from the very early > porting where large chunks of code were copied from the contacts code > that just had an UID... Did the previous code transmit UID+RECURRENCE-ID? I am wondering why Evolution manages to keep views properly in sync despite the lack of the RECURRENCE-ID - and what'll happen when it starts receiving them. I guess I'll find out... -- 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
[Evolution-hackers] libdb performance issue? (was: Re: libebook: errors when using asynchronous contact addition/removal functions)
On Fr, 2011-05-13 at 12:42 +0100, Christophe Dumez wrote: > On my machine some of the insertions often fail with error: > E_BOOK_ERROR_NOT_SUPPORTED. What is the error explanation in that case? See gError->message. When I run the test program, I see that e-addressbook-factory starts consuming 100% CPU time. "strace -p " shows that not a single system call is done. Attaching with gdb and a "thread apply all bt" shows that there are roughly 15 threads, most of them in libdb inside libthread and/or __os_yield(). Can you confirm similar behavior? My hypothesis is that we either run into a real bug in libdb (no progress made executing multiple concurrent writes) or a corner case where progress is extremely slow. Something like this made EDS look bad in the tracker<->EDS comparison: http://mail.gnome.org/archives/tracker-list/2011-March/msg00035.html In libebook, I get a "Timeout was reached" because the asynchronous operation doesn't complete quickly enough. Same for the attempt to delete the contacts. The gError->code is 24, which is indeed E_BOOK_ERROR_NOT_SUPPORTED. Running dbus-monitor, I see that removeContacts is called, but there's no reply because the D-Bus server is busy. I tried the LD_PRELOAD=libeatmydata.so workaround suggested in the mail above and it does avoid the problem. Is there anyone around who understand libdb well enough to shed some light on this? What is a proper fix? -- 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
[Evolution-hackers] NO_THISANDFUTURE/THISANDPRIOR and file backend
Hello! Why does the file backend claim that it doesn't support CAL_STATIC_CAPABILITY_NO_THISANDFUTURE and CAL_STATIC_CAPABILITY_NO_THISANDPRIOR? There is code for these in remove_object_instance_cb(), e_cal_backend_file_modify_object() and e_cal_backend_file_remove_object(). Is all of that dead code that was never used and tested? I don't need this feature, I just wonder. -- 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] e_cal_remove_object_with_mod() + empty rid: semantic?
Hello Ross! Can you perhaps comment? You wrote the TODO items below... On Di, 2011-05-03 at 18:04 +0200, Patrick Ohly wrote: > I also wonder about the "objects-removed" signal in ECalView. If there > are two events, one with RRULE and one with RECURRENCE-RULE, and both > get removed, should there be two entries in "objects-removed"? I found this in e-data-cal-view.c notify_remove(): 280 /* TODO: store ECalComponentId instead of just uid*/ 281 uid = g_strdup (id->uid); 282 g_array_append_val (priv->removes, uid); In other words, removes always come without rid, even if the actual event that was removed was not the parent but a detached recurrence. There's another, similar TODO in send_pending_removes(): 192 /* TODO: send ECalComponentIds as a list of pairs */ 193 e_gdbus_cal_view_emit_objects_removed (view->priv->gdbus_object, (const gchar * const *) priv->removes->data); And sure enough, the actual D-Bus API uses an array of strings instead of an array of string pairs. I can fix these TODOs. Any objections or concerns? It'll lead to a change of the D-Bus API. For "master" that shouldn't be a problem, but I also want this in MeeGo for KCal-EDS, based on 2.32. I guess we have to bite the bullet and maintain a MeeGo version with a different API than regular 2.32.x. One more question about e_cal_backend_notify_object_removed(): it takes iCalendar 2.0 object strings as parameter in addition to the id. If both are set, it acts like "object_updated". If the new "object" is NULL, it checks whether the "old_object" matches the view. Is that really necessary? There is a second check whether the ID is in the view in e_data_cal_view_notify_objects_removed(): 701 for (l = ids; l; l = l->next) { 702 ECalComponentId *id = l->data; 703 if (g_hash_table_lookup (priv->ids, id)) 704 notify_remove (view, id); 705 } At least that is my understanding. So is it safe to pass only the ID and old_object = object = NULL? -- 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] e_cal_remove_object_with_mod() + empty rid: semantic?
On Di, 2011-05-10 at 10:23 +0200, Milan Crha wrote: > On Wed, 2011-05-04 at 17:31 +0200, Patrick Ohly wrote: > > On Mi, 2011-05-04 at 09:41 +0200, Patrick Ohly wrote: > > > On Mi, 2011-05-04 at 07:50 +0200, Milan Crha wrote: > > > > I would expect that with CALOBJ_MOD_THIS it may remove only exact > > > > component, for uid + NULL-rid the master object (which implies also all > > > > generated instances) and keep all detached instances, > > > > > > Okay, then we agree on the desired semantic. > > > > I ran into another oddity: suppose there is a recurring event without > > exceptions and a detached recurrence gets added. The libecal API does > > not support undoing that operation. > > ... > > Hi, > it sounds like you might find useful an addition of a reverse function > for > gboolean e_cal_client_get_object_sync ( > ECalClient *client, > const gchar *uid, const gchar *rid, > icalcomponent **icalcomp, > GCancellable *cancellable, GError **error); > which can return either one component, or a VCALENDAR component. > > I'm thinking of something like: >e_cal_client_replace_object (..., const gchar *uid, > /* const */ icalcomponent *icalcomp, ...); > Note it doesn't take the 'rid' parameter as the get_object function. Yes, there might be cases where that is better than the current API. But I see it as complementary to enhancing the existing, single-event focused APIs. -- 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] [PATCH 1/2] e_cal_new_system_foo() should create corresponding source in GConf
On Di, 2011-05-10 at 09:34 +0100, David Woodhouse wrote: > On Tue, 2011-05-10 at 10:19 +0200, Patrick Ohly wrote: > > It seems that a similar problem exists in libebook if no address books > > were created already by Evolution. Chris is seeing such an issue with > > 2.32.3 in MeeGo. > > Oh, tits. I hate the fact that all this code is so *gratuitously* > separate. Yeah, me too. 3.2 will be better, but will still have separate libebook/libecal libraries. > > We probably need to add the "create GConf entry for local:system" part > > to libebook in the gnome-2-32 branch. Is that something that is still > > of interest for Trunk, given that EClient API will obsolete it for > > 3.2? > > It's still relevant for now, so let's do it. We *could* make a case for > not bothering, and just fixing it in 3.0 and 2.32 instead. But our > policy is to backport only fixes from master, and I prefer not to set a > counter-precedent unless I absolutely have to. It doesn't *hurt* to fix > it in master first. > > Do you have a patch? Sorry, no. And no time either :-/ -- 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] [PATCH 1/2] e_cal_new_system_foo() should create corresponding source in GConf
Hello! It seems that a similar problem exists in libebook if no address books were created already by Evolution. Chris is seeing such an issue with 2.32.3 in MeeGo. We probably need to add the "create GConf entry for local:system" part to libebook in the gnome-2-32 branch. Is that something that is still of interest for Trunk, given that EClient API will obsolete it for 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] [PATCH 1/2] e_cal_new_system_foo() should create corresponding source in GConf
On Di, 2011-05-10 at 11:40 +0100, Dumez, Christophe wrote: > I have tested the patch but it does not seem to help. I don't know what the > reason is yet. If you have never run Evolution, there will be no gconf entries for addressbook. The second part of the fix was to have libecal create this entry when creating a new system address book. -- 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] (summarize ][) New 'eclient' branch in eds
On Mo, 2011-05-09 at 17:00 +0200, Milan Crha wrote: > On Wed, 2011-05-04 at 14:37 +0200, Milan Crha wrote: > > So here left basically three things, > >a) merging some API in utils, > >b) getting well-known properties, > >c) setting well-known properties > > Hi, > I just did a commit into the eclient branch with a fix for b) and c). > There is a little change, I named those functions: >e_client_get_backend_property_sync >e_client_set_backend_property_sync > with their async versions too. > > Well-known properties are: > * for a book: > BOOK_BACKEND_PROPERTY_LOADED > BOOK_BACKEND_PROPERTY_ONLINE > BOOK_BACKEND_PROPERTY_READONLY > BOOK_BACKEND_PROPERTY_CACHE_DIR > BOOK_BACKEND_PROPERTY_CAPABILITIES > BOOK_BACKEND_PROPERTY_REQUIRED_FIELDS > BOOK_BACKEND_PROPERTY_SUPPORTED_FIELDS > BOOK_BACKEND_PROPERTY_SUPPORTED_AUTH_METHODS > >* for a calendar: > CAL_BACKEND_PROPERTY_LOADED > CAL_BACKEND_PROPERTY_ONLINE > CAL_BACKEND_PROPERTY_READONLY > CAL_BACKEND_PROPERTY_CACHE_DIR > CAL_BACKEND_PROPERTY_CAPABILITIES > CAL_BACKEND_PROPERTY_CAL_EMAIL_ADDRESS > CAL_BACKEND_PROPERTY_ALARM_EMAIL_ADDRESS > CAL_BACKEND_PROPERTY_DEFAULT_OBJECT Why duplicate the LOADED/ONLINE/READONLY/CACHE_DIR/CAPABILITIES properties? They could be defined as common E_CLIENT_BACKEND_ properties. -- 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] e_cal_remove_object_with_mod() + empty rid: semantic?
On Mi, 2011-05-04 at 09:41 +0200, Patrick Ohly wrote: > On Mi, 2011-05-04 at 07:50 +0200, Milan Crha wrote: > > I would expect that with CALOBJ_MOD_THIS it may remove only exact > > component, for uid + NULL-rid the master object (which implies also all > > generated instances) and keep all detached instances, > > Okay, then we agree on the desired semantic. I ran into another oddity: suppose there is a recurring event without exceptions and a detached recurrence gets added. The libecal API does not support undoing that operation. e_cal_remove_object_with_mod(rid=, CALOBJ_MOD_THIS) will remove the recurrence but it will *also* add an EXDATE for that recurrence. So the calendar is not in the same state as it was before adding the detached recurrence. This behavior may make sense for a UI ("remove this recurrence" implies adding the EXDATE), but not for syncing. May I add a CALOBJ_MOD_REALLY_REALLY_REMOVE_THIS_ONLY mode? > If a backend doesn't support the semantic, then there's not much that > can be done. But the file backend should be able to to handle it, > therefore I consider it a bug that it removes all instances in this > case. > > I'll look into fixing this. I have a patch ready. Will continue to test and send when the rest also passes my tests. > > > I also wonder about the "objects-removed" signal in ECalView. If there > > > are two events, one with RRULE and one with RECURRENCE-RULE, and both > > > get removed, should there be two entries in "objects-removed"? > > > > I've no idea on this. I'm sorry. > > First things first ;-) Let me fix the removal, then look into this > problem here. ECalView is indeed odd. My initial understanding is that the file backend will never send information about removed detached recurrences, so rid is always empty. What I get instead when removing a detached recurrence is a "master modified" with EXDATE set. I suspect that the Evolution UI uses that to remove the corresponding detached recurrence. Does anyone know whether that is the case? If I introduce the CALOBJ_MOD_REALLY_REALLY_REMOVE_THIS_ONLY, then EXDATE won't be set and a real "object-removed" with rid set will be needed. -- 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] (summarize ][) New 'eclient' branch in eds
On Mi, 2011-05-04 at 14:37 +0200, Milan Crha wrote: > Hi, > I'm getting slightly lost what is left and what is under discussion, > thus please let me summarize things here: > > 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. > > It seems strange to me to have EClientSourceType defined in e-client.h > and its main usage in other file (remember of compiler dependency > issue), thus I suggest to keep e-client/e-book-client/e-cal-client as > they are and rename e-client-authentication.h/.c (which resides in > libedataserverui) to e-client-utils.h/.c and do common "interfaces" > here. This is including also e_client_open_new() being defined here. > This is for client-side code merging. I still think that "list and open databases" are common operations and that having different libraries and code for contacts and calendar is a historic artifact. But let's not get hung up on that and move on without changing it. > > 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". > > Here was suggested a common e_client_get_property_sync (EClient *client, > const gchar *in_name, gchar **out_value, ...); function (with its async > equivalent) (the e_client_get_backend_property_sync is better > descriptive, less confusing with GObject properties, I guess, though > it's quite long name). I'm fine with that, I can add it. Thanks. > > 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). > > This breaks abstraction, from my point of view. The client may not > know/expect anything about backends, That's not quite true in all cases. On an embedded system, a client might know exactly that it is going to be installed together with a specific backend and may want to influence that backend without having to maintain a non-upstream API for it. > so any property setting from client > side is not needed. Another situation would be a writable property that is supported by one or more backends. Writing in a backend which doesn't support should trigger an error, but in others it may make sense. For example, setting the color of a calendar is possible in CalDAV. A read/write property would make sense to expose that to clients. > You can ask for "change" tag, or "last modified" > tag, but you should not be able to change it from client side in other > than _add/_modify/_remove way. "change" and "last modified" would be read-only. But other properties may be writable. Even if there are none predefined now, adding the API now is necessary to make it future-proof. -- 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] e_cal_remove_object_with_mod() + empty rid: semantic?
On Mi, 2011-05-04 at 07:50 +0200, Milan Crha wrote: > I would expect that with CALOBJ_MOD_THIS it may remove only exact > component, for uid + NULL-rid the master object (which implies also all > generated instances) and keep all detached instances, Okay, then we agree on the desired semantic. > though this > operation doesn't seem to have much sense to me. It's certainly not a common operation, but the API should allow it nevertheless, because otherwise correct synchronization between a database where such an operation took place (for whatever reason) and EDS is hard. > The problem is that > each backend has its own implementation of such operation (also note > that groupwise backend has no master object (it tells it in > capabilities)) and for example file backend behaves with rid=NULL > mod=CALOBJ_MOD_THIS the same like with mod=CALOBJ_MOD_ALL, thus it drops > also detached instances. If a backend doesn't support the semantic, then there's not much that can be done. But the file backend should be able to to handle it, therefore I consider it a bug that it removes all instances in this case. I'll look into fixing this. > > I also wonder about the "objects-removed" signal in ECalView. If there > > are two events, one with RRULE and one with RECURRENCE-RULE, and both > > get removed, should there be two entries in "objects-removed"? > > I've no idea on this. I'm sorry. First things first ;-) Let me fix the removal, then look into this problem here. -- 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
[Evolution-hackers] e_cal_remove_object_with_mod() + empty rid: semantic?
Hello! I'm running into an old nemesis of mine: how do I remove the parent event (the one with RRULE) without also removing child events (= detached recurrences, exceptions)? This is not something that is needed by the Evolution UI, but IMHO it is a valid operation. Definitely can happen during syncing or when using a different front end. It is possible to create a calendar which only has the child event in Evolution by importing a meeting invitation for just one instance of a recurring event. I consider such a calendar valid, but there is no clean way to get to that state after the parent has been added, as far as I know. e_cal_remove_object_with_mod() looks promising, but I can't get it to work: /** * e_cal_remove_object_with_mod: * @ecal: A calendar client. * @uid: UID og the object to remove. * @rid: Recurrence ID of the specific recurrence to remove. * @mod: Type of removal. * @error: Placeholder for error information. * * This function allows the removal of instances of a recurrent * appointment. By using a combination of the @uid, @rid and @mod * arguments, you can remove specific instances. If what you want * is to remove all instances, use e_cal_remove_object instead. * * If not all instances are removed, the client will get a "obj_modified" * signal, while it will get a "obj_removed" signal when all instances * are removed. * * Returns: TRUE if the operation was successful, FALSE otherwise. */ gboolean e_cal_remove_object_with_mod (ECal *ecal, const gchar *uid, const gchar *rid, CalObjModType mod, GError **error) The documentation does not explain which combination of the parameters does what. Previously, passing a NULL or empty rid (don't remember exactly, but I probably tried both at that time) confused the Exchange connector backend. Therefore it wasn't possible to remove just the parent event. In SyncEvolution I added weird hacks for this, like first retrieving all children, removing the whole series with e_cal_remove_object(), then re-adding the children. I was hoping to avoid this with more recent EDS, the local file backend and simply calling e_cal_remove_object_with_mod() regardless whether it has an rid or not. But it turns out that e_cal_remove_object_with_mod(rid="") simply calls the removeObject D-Bus method with an empty string for the rid, just like e_cal_remove_object() does, and thus all events sharing the same UID get deleted. rid=NULL doesn't make any difference. What is the intended semantic of e_cal_remove_object_with_mod()? If "rid empty or NULL, then remove only parent"? I also wonder about the "objects-removed" signal in ECalView. If there are two events, one with RRULE and one with RECURRENCE-RULE, and both get removed, should there be two entries in "objects-removed"? I might be misreading my debug logging, but that doesn't seem to happen. Before I dig deeper, I'd like to get some feedback on the expected semantic. -- 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 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
[Evolution-hackers] 32 bit IDs in contact file backend
Hello! As part of wrapping QtContacts around EDS [1] I ran into the same issue that Nokia already encountered in their Maemo 5 [2] backend: EDS uses strings as ID, QtContacts 32 bit integers. Nokia solved that by setting up an in-memory hash which maps the UID string to its CRC-16. I don't see any checks for the (possible) hash collisions. IMHO a proper solution has to keep a permanent mapping on disk, otherwise the 32 bit IDs won't be stable. Overall not a nice solution. My attempt to make it nicer at least in combination with the file backend (the main goal for QtContacts-EDS) focused on simplifying the problem instead: with 32 bit IDs in the storage, the mapping becomes easy. Attached the resulting patch. Note that with the patch applied, all new contacts in a Berkley DB get the simpler IDs, unconditionally. Older contacts continue to use their existing IDs. Would something like this be acceptable upstream? Further work if you agree in principle: * let clients query whether all contacts have the simplified ID - could be done with the dynamic capabilities that I mentioned in the e-client API thread; avoids reading all contact (UIDs) * optional: migrate database to simplified IDs on request I'm not sure whether the second point really needs to be in EDS. Obviously it should only be done when the impact on the rest of the system is understood. For example, in a database that is synchronized with SyncEvolution, such a rewriting of IDs will result in removing and re-adding all contacts in the synchronized peers because that is what it'll look like to other EDS clients. This is also how the migration can be implemented without changes in EDS: just remove and re-add all contacts. [1] https://meego.gitorious.org/meego-middleware/qtcontacts-eds [2] https://www.qt.gitorious.org/qt-mobility/contacts/trees/master/plugins/contacts/maemo5 -- Bye, Patrick Ohly -- patrick.o...@gmx.de http://www.estamos.de/ >From 65438bd3c4b706535f83304d94b3c018c8899a5d Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Fri, 18 Mar 2011 16:07:23 +0100 Subject: [PATCH] addressbook file backend: use 32 bit integers as IDs The motiviation for this change is primarily that it'll simplify wrapping EDS in QtContacts, which only supports 32 bit integers for the storages that it wraps. Previously the ID was generated as a combination of time and running counter, without checking for conflicts. This patch changes this such that the ID is only a running counter. ID conflicts are detected during an attempt to save, leading to retries with the next counter value. The "for future use" members of _EBookBackendFilePrivate are removed because they serve no useful purpose in a structure which is allocated and used only locally. --- addressbook/backends/file/e-book-backend-file.c | 48 +++--- 1 files changed, 33 insertions(+), 15 deletions(-) diff --git a/addressbook/backends/file/e-book-backend-file.c b/addressbook/backends/file/e-book-backend-file.c index dc6a703..d21c527 100644 --- a/addressbook/backends/file/e-book-backend-file.c +++ b/addressbook/backends/file/e-book-backend-file.c @@ -77,11 +77,7 @@ struct _EBookBackendFilePrivate { DB *file_db; DB_ENV *env; EBookBackendSummary *summary; - /* for future use */ - gpointer reserved1; - gpointer reserved2; - gpointer reserved3; - gpointer reserved4; + guint id; }; G_LOCK_DEFINE_STATIC (global_env); @@ -169,13 +165,13 @@ build_summary (EBookBackendFilePrivate *bfpriv) } static gchar * -e_book_backend_file_create_unique_id (void) +e_book_backend_file_create_next_id (EBookBackendFile *bf) { - /* use a 32 counter and the 32 bit timestamp to make an id. - it's doubtful 2^32 id's will be created in a second, so we - should be okay. */ - static guint c = 0; - return g_strdup_printf (PAS_ID_PREFIX "%08lX%08X", time(NULL), c++); + /* use a 32 counter, avoid 0; collision checks must be done by caller */ + ++bf->priv->id; + if (!bf->priv->id) + bf->priv->id = 1; + return g_strdup_printf (PAS_ID_PREFIX "%04X", bf->priv->id); } static void @@ -210,7 +206,8 @@ do_create(EBookBackendFile *bf, g_assert (vcard_req); g_assert (contact); - id = e_book_backend_file_create_unique_id (); + retry: + id = e_book_backend_file_create_next_id (bf); string_to_dbt (id, &id_dbt); @@ -224,9 +221,10 @@ do_create(EBookBackendFile *bf, string_to_dbt (vcard, &vcard_dbt); - db_error = db->put (db, NULL, &id_dbt, &vcard_dbt, 0); + db_error = db->put (db, NULL, &id_dbt, &vcard_dbt, DB_NOOVERWRITE); g_free (vcard); + g_free (id); if (0 == db_error) { db_error = db->sync (db, 0); @@ -234,12 +232,16 @@ do_create(EBookBackendFile *bf, g_warning ("db->sync failed with %s", db_strerror (db_error)); } } else { - g_warning (G_STRLOC ": db->put failed with %s
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 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] Fedora builds with 2.32.2+ patches
On Fr, 2011-04-08 at 16:31 +0100, David Woodhouse wrote: > On Fri, 2011-04-08 at 15:31 +0200, Patrick Ohly wrote: > > On Do, 2011-04-07 at 11:33 +0100, David Woodhouse wrote: > > > Once this passes muster, I'll push these patches (probably *without* the > > > NTLM bits, if you're looking closely at what I included) to the > > > gnome-2-32 branches and perhaps start doing a 'final call' for 2.32.3 > > > candidate bugs/patches. > > > > Please consider backporting the fixes for e_cal_new_system_*(). They are > > unusable in 2.32.x but I intended to use them soon in MeeGo. > > > > I'm not sure which fixed from the master branch are all needed, I hope > > Matthew and Milan can provide a list. > > I am trying hard not to bother Matthew and Milan; I don't want the > continued maintenance of the gnome-2-32 branch to be a burden to them in > any way. I know, but Matthew had already kindly offered some help here. > I have cherry-picked the patches from master which look relevant; please > could you test what is currently in > > http://git.infradead.org/evolution-data-server-2.32.git > git://git.infradead.org/evolution-data-server-2.32.git Listing sources shows local:/system instead of local:system and opening "local:system" also still fails. Milan pointed to the history of e-source.c, but I don't see which (other?) commits are needed to fix these issues. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ 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] Fedora builds with 2.32.2+ patches
On Do, 2011-04-07 at 11:33 +0100, David Woodhouse wrote: > Once this passes muster, I'll push these patches (probably *without* the > NTLM bits, if you're looking closely at what I included) to the > gnome-2-32 branches and perhaps start doing a 'final call' for 2.32.3 > candidate bugs/patches. Please consider backporting the fixes for e_cal_new_system_*(). They are unusable in 2.32.x but I intended to use them soon in MeeGo. I'm not sure which fixed from the master branch are all needed, I hope Matthew and Milan can provide a list. -- 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] e_cal_new_system_calendar() -> creates a new calendar each time?
On Do, 2011-04-07 at 10:47 +0200, Patrick Ohly wrote: > Hello! > > I noticed that in 2.32/MeeGo, e_cal_new_system_calendar() always creates > a new calendar, although there is already one. > > It is defined in gconf as: > > base_uri="local:" readonly="no"> uid="1300454894.7178.4@pohly-mobl1" name="Personal" > relative_uri="system" color_spec="#BECEDD"/> > > The sequence of events is this: > 1. e_cal_new_system_calendar() > 2. e_cal_new_from_uri("local:system", ... > 3. get source list > 4. search_known_sources() by comparing e_source_peek_absolute_uri() > against "local:system" > 5. no source found, create anew > > Step 4 fails because there is no absolute URI: > > (gdb) p source->priv->absolute_uri > $11 = (gchar *) 0x0 > (gdb) p source->priv->relative_uri > $12 = (gchar *) 0x8079450 "system" > > Therefore e_source_peek_absolute_uri() returns NULL and the comparison > fails. > > What is the root cause for this issue, and how should it be fixed? absolute_uri is taken from the "uri" property, so adding that to the gconf sources works around the issue. Perhaps dump_common_to_xml_node() and the code after /* do not store absolute uris for local:system sources */ is to blame? Or if "uri" is correctly not set, then e_cal_new_system_calendar()/e_cal_new_from_uri() must be fixed to cope with that? One more oddity: on the client side, e_source_get_uri() for the system calendar returns "local:/system". What is the correct absolute URI for the local system calendar? -- 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