Re: [Evolution-hackers] Move authentication of backends back to the client (3.13.90)

2015-02-27 Thread Patrick Ohly
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)

2015-02-18 Thread Patrick Ohly
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

2014-09-08 Thread Patrick Ohly
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

2014-09-04 Thread Patrick Ohly
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

2014-05-26 Thread Patrick Ohly
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

2014-05-25 Thread Patrick Ohly
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

2014-05-23 Thread Patrick Ohly
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?

2014-05-20 Thread Patrick Ohly
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?

2014-05-15 Thread Patrick Ohly
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

2014-05-02 Thread Patrick Ohly
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

2014-04-24 Thread Patrick Ohly
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

2014-04-24 Thread Patrick Ohly
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

2014-04-16 Thread Patrick Ohly
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

2014-04-16 Thread Patrick Ohly
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

2014-04-15 Thread Patrick Ohly
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

2014-04-11 Thread Patrick Ohly
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

2014-03-17 Thread Patrick Ohly
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

2014-02-20 Thread Patrick Ohly
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

2014-01-02 Thread Patrick Ohly
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

2014-01-02 Thread Patrick Ohly
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

2013-06-30 Thread Patrick Ohly
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

2012-11-23 Thread Patrick Ohly
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

2012-10-16 Thread Patrick Ohly
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

2012-10-08 Thread Patrick Ohly
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

2012-10-02 Thread Patrick Ohly
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

2012-09-02 Thread Patrick Ohly
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

2012-05-31 Thread Patrick Ohly
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

2012-05-14 Thread Patrick Ohly
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

2012-04-10 Thread Patrick Ohly
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

2012-03-07 Thread Patrick Ohly
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

2012-03-06 Thread Patrick Ohly
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

2012-03-04 Thread Patrick Ohly
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

2012-03-03 Thread Patrick Ohly
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

2012-03-03 Thread Patrick Ohly
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

2012-02-17 Thread Patrick Ohly
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

2012-02-14 Thread Patrick Ohly
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

2012-02-13 Thread Patrick Ohly
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

2011-11-28 Thread Patrick Ohly
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

2011-11-28 Thread Patrick Ohly
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

2011-11-20 Thread Patrick Ohly
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

2011-11-18 Thread Patrick Ohly
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

2011-11-16 Thread Patrick Ohly
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

2011-11-15 Thread Patrick Ohly
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

2011-11-14 Thread Patrick Ohly
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

2011-11-14 Thread Patrick Ohly
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)

2011-09-14 Thread Patrick Ohly
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)

2011-09-13 Thread Patrick Ohly
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)

2011-09-13 Thread Patrick Ohly
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)

2011-09-12 Thread Patrick Ohly
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)

2011-09-09 Thread Patrick Ohly
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()

2011-08-09 Thread Patrick Ohly
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

2011-07-09 Thread Patrick Ohly
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

2011-07-08 Thread Patrick Ohly
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

2011-07-08 Thread Patrick Ohly
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

2011-07-08 Thread Patrick Ohly
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

2011-07-08 Thread Patrick Ohly
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

2011-07-07 Thread Patrick Ohly
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

2011-07-07 Thread Patrick Ohly
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

2011-07-06 Thread Patrick Ohly
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

2011-06-24 Thread Patrick Ohly
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

2011-06-14 Thread Patrick Ohly
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

2011-06-14 Thread Patrick Ohly
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?])

2011-06-05 Thread Patrick Ohly
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

2011-06-02 Thread Patrick Ohly
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?])

2011-06-02 Thread Patrick Ohly
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

2011-05-19 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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

2011-05-17 Thread Patrick Ohly
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?

2011-05-17 Thread Patrick Ohly
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?

2011-05-16 Thread Patrick Ohly
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)

2011-05-16 Thread Patrick Ohly
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)

2011-05-16 Thread Patrick Ohly
[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)

2011-05-16 Thread Patrick Ohly
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?

2011-05-13 Thread Patrick Ohly
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)

2011-05-13 Thread Patrick Ohly
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

2011-05-12 Thread Patrick Ohly
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?

2011-05-12 Thread Patrick Ohly
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?

2011-05-10 Thread Patrick Ohly
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

2011-05-10 Thread Patrick Ohly
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

2011-05-10 Thread Patrick Ohly
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

2011-05-10 Thread Patrick Ohly
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

2011-05-09 Thread Patrick Ohly
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?

2011-05-04 Thread Patrick Ohly
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

2011-05-04 Thread Patrick Ohly
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?

2011-05-04 Thread Patrick Ohly
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?

2011-05-03 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-28 Thread Patrick Ohly
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

2011-04-11 Thread Patrick Ohly
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

2011-04-08 Thread Patrick Ohly
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?

2011-04-07 Thread Patrick Ohly
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


  1   2   3   >