Re: [Evolution-hackers] SyncEvolution + EClient API + EXDATE regression (Bug #655253)

2011-09-14 Thread Milan Crha
On Tue, 2011-09-13 at 18:44 +0200, Patrick Ohly wrote:
 On Di, 2011-09-13 at 17:59 +0200, Patrick Ohly wrote:
  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.

Hi,
yes, I agree, it wasn't the right change, I'm sorry for that. Please
revert the commit and reopen the bug. Thanks for the pointer on this.
Bye,
Milan

___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] 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-14 Thread Milan Crha
On Wed, 2011-09-14 at 09:50 +0200, Patrick Ohly wrote:
 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?

Hi,
thanks. I see Akhil took care of the reopen. And now just find the
proper solution (and possibly when it got broken at the first place).
I believe your investigation about newly set UID is correct, I also
noticed that they do not match, but I thought it's not relevant, until
you corrected my thoughts.
Bye,
Milan


___
evolution-hackers mailing list
evolution-hackers@gnome.org
To change your list options or unsubscribe, visit ...
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] 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-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