Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Status

Calendar
Internal Components
VERIFIED FIXED
10 years ago
6 years ago

People

(Reporter: Michiel van Leeuwen (email: mvl+moz@), Assigned: dbo)

Tracking

Dependency tree / graph
Bug Flags:
wanted-calendar1.0 +

Details

Attachments

(7 attachments, 1 obsolete attachment)

There is a new version of libical (0.27). We should update to it.
Need to figure out if we update to latest cvs or to the release.

There is information on how to update on the wiki: http://wiki.mozilla.org/Calendar:Updating_libical

libical can be found at http://sourceforge.net/projects/freeassociation/

It might be hard to get a patch attached to this bug, and i'm not sure if reviewing has any use. Can I get blanket review?
(Reporter)

Updated

10 years ago
Blocks: 386516
(Assignee)

Comment 1

10 years ago
(In reply to comment #0)
> It might be hard to get a patch attached to this bug, and i'm not sure if
> reviewing has any use. Can I get blanket review?
A "cvs diff -u8p -rLIBICAL_0_24RC_CVS_20050215 -rMOZILLA_1_8_BRANCH" shows me that about 1/3 of the patch are code changes. I have no problem granting a blanket review to you since you've some experience with updating libical, but if you like, I volunteer to have a further look at the applied changes. IMO we should add the bug-id+title into code comments this time (if easily possible), since the cvs log doesn't reflect that anymore after an upgrade.
Finally, IMO the upgrade should be done post 0.7, because we don't know much about regressions in libical in the meantime.
(Reporter)

Comment 2

10 years ago
The procdure sketches at the wiki page i pointed to should update libical while keeping our changes. It's basically a merge. So we should not need to manually apply our changes. I'm not sure what it does to cvs blame, though.

The patch i was talking about is about the overall libical changes, not about any private changes.
(Assignee)

Comment 3

10 years ago
(In reply to comment #2)
> The procdure sketches at the wiki page i pointed to should update libical while
> keeping our changes. It's basically a merge. So we should not need to manually
> apply our changes. I'm not sure what it does to cvs blame, though.
Yes, if cvs doesn't detect a conflict, we could be pretty sure the merge doesn't hurt. However if you are facing the need to resolve a conflict, I volunteer to review that. 

> The patch i was talking about is about the overall libical changes, not about
> any private changes.
I agree that doesn't make sense.
(Reporter)

Comment 4

10 years ago
Created attachment 290901 [details] [diff] [review]
changes between libical trunk and mozilla's version

This diff is what changed between LIBICAL_0_24RC_CVS_20050215 and current trunk of libical. That means that our local changes should not show up.
There is a lot of noise in the patch, like timezone definitions. I don't think that they will affect us. I already stripped the makefile changes, since we don't use that build system. I also stripped a bunch of timezone changes, to get the patch small enough for bugzilla to accept it. The sample that's left is still enough for review, i'd think.


daniel, can you take a quick look if there is anything that might break?

(I'm planning on doing the update using cvs import, not by applying this patch. I hope that that will make future stuff easier)



note to self: this is how to get a clean libical tree

cvs -d:pserver:anonymous@freeassociation.cvs.sourceforge.net:/cvsroot/freeassociation login 
cvs -z3 -d:pserver:anonymous@freeassociation.cvs.sourceforge.net:/cvsroot/freeassociation co -P libical
mkdir libical_clean
cd libical_clean
cp -ar AUTHORS ChangeLog design-data/ LICENSE NEWS README scripts/ src/ test-data/ THANKS TODO ylwrap zoneinfo/ ../libical_clean
cd ../libical_clean/
find -iname CVS | xargs rm -rf
Attachment #290901 - Flags: review?(daniel.boelzle)
(Assignee)

Comment 5

10 years ago
Comment on attachment 290901 [details] [diff] [review]
changes between libical trunk and mozilla's version

No review, just skin-deep look; r=dbo. Let's give it a try.
Attachment #290901 - Flags: review?(daniel.boelzle) → review+
(Assignee)

Comment 6

10 years ago
Created attachment 291011 [details] [diff] [review]
local changes (FWIW, just for reference)
(Reporter)

Comment 7

10 years ago
Ok, it all went wrong. First, I ran cvs import in the wrong directory, causing all kinds of not-wanted files to be imported. But we could have lived with that.
The second problem was that somebody at libical thought it was a great idea to put icalvalue.* and a couple of other files in .cvsignore, while those files are very much needed. Thus, those files were not imported.
After a whole lot of messing around, i had to revert to some manual solution. I manually applied the patch from comment 6 to a clean libical dir, and commited the result. It should build now.
But I forgot to think about future upgrades. Not sure how we can make those possible now :(
(Assignee)

Comment 8

10 years ago
Would be great to have a clean tag for the vanilla libical, to diff out our local changes in the future.

Comment 9

10 years ago
mvl, is there a reason why this was only checked into the trunk? Does our cross-commit policy not apply here?
Assignee: nobody → mvl
(Reporter)

Comment 10

10 years ago
I never said that I was finished with updating libical, so please have some patience.
cvs import can't be combined with cross-commit. So I had to do branch first. And when that went kind-of wrong, i decided to not do branch until i had a green trunk. Now I just need some time to check in to branch.

Updated

10 years ago
Blocks: 272411
(Assignee)

Comment 11

10 years ago
Side-Note: Bug 415987 has applied a fix for libical in the meantime. The fix is checked in for both HEAD and MOZILLA_1_8_BRANCH.
Status: NEW → ASSIGNED
(Assignee)

Comment 12

9 years ago
If 0.8 is out, updating libical shouldn't be postponed any longer.
Flags: wanted-calendar0.9+

Comment 13

9 years ago
I believe the checkin on TRUNK caused one unit test (test_recur.js) to fail on the second test.

Comment 14

9 years ago
Now that 0.8 is released when will this be merged from Trunk to Branch?
Good news... parts of attachment#291011 [details] [diff] [review] seem to have found their way into libical trunk (http://freeassociation.svn.sourceforge.net/viewvc/freeassociation?view=rev&revision=812).
(Assignee)

Comment 16

9 years ago
I think we should refrain from updating libical at this stage, unless there's a compelling reason to do so (I don't know of any urging bug, and it would only break our unit tests). We should bundle our resources to get the wanted/blocking bugs fixed.
Flags: wanted-calendar0.9+ → wanted-calendar0.9-

Comment 17

9 years ago
i've made myself home and merged your patches into upstream libical.
(SVN Rev 812) 
I've changed one of your patches to maintain a stable api to the other libical applications (SVN Rev 814):
  icaltime_compare_date_only (classic)
  icaltime_compare_date_only_tz (mozilla style with timezone to use)

Please feel welcome to join the SF project, me or Art Cancro will add you.
Created attachment 323909 [details] [diff] [review]
Changes between our branch and libical 2008-06-05 trunk (svn)

Wilfried, thanks for merging our changes back in. I'm happy to hear about this motion to un-fork libical, I think it makes things easier for all sides.

I took a look at what changed between what we have in branch and the current libical svn trunk and stripped out most unimportant things (like rcs id changes). This might help for diagnosing things that break.

Just an ida, but maybe after 0.9 we can think about pulling libical directly from sf.net, given we have a fair amount of regression testing and monitor libical checkins. I think that would make life much easier, libical updates seem to be quite a pain.
(Assignee)

Updated

9 years ago
Flags: wanted-calendar0.9- → wanted-calendar1.0+
(Assignee)

Comment 19

9 years ago
Created attachment 342226 [details] [diff] [review]
fix for empty property values (bug 458828 comment #4)

If it's correct, we should propose it for upstream libical.
(Assignee)

Comment 20

9 years ago
Created attachment 344812 [details] [diff] [review]
our changes to 0.40

I've done an update to libical 0.40:
a) I've stripped down the stock 0.40 to what's really necessary, *without* our local changes. => This patch is too big to be filed here, and is presumably unreviewable, too.
b) Our local changes to 0.40; attached here.
Assignee: mvl → daniel.boelzle
Attachment #342226 - Attachment is obsolete: true
Attachment #344812 - Flags: review?(philipp)
(Assignee)

Comment 21

9 years ago
Created attachment 344814 [details] [diff] [review]
fix to allow reading empty properties

The RFC seems to allow empty properties, thus we should allow reading them, although they won't be serialized out.
Attachment #344814 - Flags: review?(philipp)
Comment on attachment 344812 [details] [diff] [review]
our changes to 0.40

I spoke this through with daniel at the moz eu camp and we agreed on r+
Attachment #344812 - Flags: review?(philipp) → review+
Comment on attachment 344814 [details] [diff] [review]
fix to allow reading empty properties

r=philipp
Attachment #344814 - Flags: review?(philipp) → review+
(Assignee)

Comment 24

9 years ago
Pushed changes for stripped stock 0.40 to comm-central <http://hg.mozilla.org/comm-central/rev/3e3f139788da>. This should serve as a reference to sum up our changes.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
(Assignee)

Comment 25

9 years ago
Pushed our changes to comm-central <http://hg.mozilla.org/comm-central/rev/4072d2cac1df>

-> FIXED
(Assignee)

Comment 26

9 years ago
Allow empty properties: Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/92a426263440>
(Assignee)

Comment 27

9 years ago
updated <https://wiki.mozilla.org/Calendar:Updating_libical>

Comment 28

9 years ago
This broke the win32 build servers:
http://tinderbox.mozilla.org/showlog.cgi?log=Sunbird/1225042396.1225042625.16323.gz#err0

/libical/src/libical/icaltimezone.c(346) : warning C4013: 'strcasecmp' undefined; assuming extern returning int

/libical/src/libical/icaltimezone.c(1376) : error C2275: 'icalarray' : illegal use of this type as an expression

    /libical/src/libical/icalarray.h(36) : see declaration of 'icalarray'

/libical/src/libical/icaltimezone.c(1376) : error C2065: 'builtin_timezones' : undeclared identifier

/libical/src/libical/icaltimezone.c(1402) : error C2223: left of '->num_elements' must point to struct/union

/libical/src/libical/icaltimezone.c(1403) : warning C4047: 'function' : 'icalarray *' differs in levels of indirection from 'int'

/libical/src/libical/icaltimezone.c(1403) : warning C4024: 'icalarray_element_at' : different types for formal and actual parameter 1

/libical/src/libical/icaltimezone.c(1461) : error C2275: 'icalarray' : illegal use of this type as an expression

    /libical/src/libical/icalarray.h(36) : see declaration of 'icalarray'

/libical/src/libical/icaltimezone.c(1466) : error C2223: left of '->num_elements' must point to struct/union

/libical/src/libical/icaltimezone.c(1470) : warning C4047: 'function' : 'icalarray *' differs in levels of indirection from 'int'

/libical/src/libical/icaltimezone.c(1470) : warning C4024: 'icalarray_element_at' : different types for formal and actual parameter 1

/libical/src/libical/icaltimezone.c(1900) : warning C4013: 'snprintf' undefined; assuming extern returning int

/libical/src/libical/icaltimezone.c(1989) : warning C4013: 'S_ISDIR' undefined; assuming extern returning int

NEXT ERROR make[7]: *** [icaltimezone.obj] Error 2
(Assignee)

Comment 29

9 years ago
Yes, I kind of expected that, though didn't hope for it. I'll take care of it tomorrow...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 30

9 years ago
Created attachment 344882 [details] [diff] [review]
win32 bustage fix

r=me; pushed: <http://hg.mozilla.org/comm-central/rev/785c708554f7>
Attachment #344882 - Flags: review+
(Assignee)

Updated

9 years ago
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED

Comment 31

9 years ago
libical 0.40 offers cmake which should compile on windows...
does the above patch fix configure for the same thing? ;-)
(Assignee)

Comment 32

9 years ago
Created attachment 346637 [details] [diff] [review]
cleanup shared_makefiles.sh
Attachment #346637 - Flags: review?(mschroeder)
Comment on attachment 346637 [details] [diff] [review]
cleanup shared_makefiles.sh

r=mschroeder
Attachment #346637 - Flags: review?(mschroeder) → review+
(Assignee)

Comment 34

9 years ago
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/404408453da0>

-> FIXED
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1

Updated

6 years ago
Depends on: 557782
You need to log in before you can comment on or make changes to this bug.