Closed
Bug 394902
Opened 17 years ago
Closed 16 years ago
update libical
Categories
(Calendar :: Internal Components, defect)
Calendar
Internal Components
Tracking
(Not tracked)
VERIFIED
FIXED
1.0b1
People
(Reporter: mvl, Assigned: dbo)
References
Details
Attachments
(7 files, 1 obsolete file)
726.27 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
35.16 KB,
patch
|
Details | Diff | Splinter Review | |
186.59 KB,
patch
|
Details | Diff | Splinter Review | |
14.89 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
1.27 KB,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
7.27 KB,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
655 bytes,
patch
|
mschroeder
:
review+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•17 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•17 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•17 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•17 years ago
|
||
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•17 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•17 years ago
|
||
Reporter | ||
Comment 7•17 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•17 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•17 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•17 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.
Assignee | ||
Comment 11•16 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.
Updated•16 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 12•16 years ago
|
||
If 0.8 is out, updating libical shouldn't be postponed any longer.
Flags: wanted-calendar0.9+
Comment 13•16 years ago
|
||
I believe the checkin on TRUNK caused one unit test (test_recur.js) to fail on the second test.
Comment 14•16 years ago
|
||
Now that 0.8 is released when will this be merged from Trunk to Branch?
Comment 15•16 years ago
|
||
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•16 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•16 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.
Comment 18•16 years ago
|
||
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•16 years ago
|
Flags: wanted-calendar0.9- → wanted-calendar1.0+
Assignee | ||
Comment 19•16 years ago
|
||
If it's correct, we should propose it for upstream libical.
Assignee | ||
Comment 20•16 years ago
|
||
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•16 years ago
|
||
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 22•16 years ago
|
||
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 23•16 years ago
|
||
Comment on attachment 344814 [details] [diff] [review] fix to allow reading empty properties r=philipp
Attachment #344814 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 24•16 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
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Assignee | ||
Comment 25•16 years ago
|
||
Pushed our changes to comm-central <http://hg.mozilla.org/comm-central/rev/4072d2cac1df> -> FIXED
Assignee | ||
Comment 26•16 years ago
|
||
Allow empty properties: Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/92a426263440>
Assignee | ||
Comment 27•16 years ago
|
||
updated <https://wiki.mozilla.org/Calendar:Updating_libical>
Comment 28•16 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•16 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•16 years ago
|
||
r=me; pushed: <http://hg.mozilla.org/comm-central/rev/785c708554f7>
Attachment #344882 -
Flags: review+
Assignee | ||
Updated•16 years ago
|
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Comment 31•16 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•16 years ago
|
||
Attachment #346637 -
Flags: review?(mschroeder)
Comment 33•16 years ago
|
||
Comment on attachment 346637 [details] [diff] [review] cleanup shared_makefiles.sh r=mschroeder
Attachment #346637 -
Flags: review?(mschroeder) → review+
Assignee | ||
Comment 34•16 years ago
|
||
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/404408453da0> -> FIXED
Updated•16 years ago
|
Status: RESOLVED → VERIFIED
Updated•14 years ago
|
Target Milestone: 1.0 → 1.0b1
You need to log in
before you can comment on or make changes to this bug.
Description
•