Closed Bug 394902 Opened 17 years ago Closed 16 years ago

update libical

Categories

(Calendar :: Internal Components, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: mvl, Assigned: dbo)

References

Details

Attachments

(7 files, 1 obsolete file)

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?
Blocks: 386516
(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.
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.
(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.
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)
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+
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 :(
Would be great to have a clean tag for the vanilla libical, to diff out our local changes in the future.
mvl, is there a reason why this was only checked into the trunk? Does our cross-commit policy not apply here?
Assignee: nobody → mvl
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.
Blocks: 272411
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
If 0.8 is out, updating libical shouldn't be postponed any longer.
Flags: wanted-calendar0.9+
I believe the checkin on TRUNK caused one unit test (test_recur.js) to fail on the second test.
Now that 0.8 is released when will this be merged from Trunk to Branch?
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-
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.
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.
Flags: wanted-calendar0.9- → wanted-calendar1.0+
If it's correct, we should propose it for upstream libical.
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)
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+
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
Pushed our changes to comm-central <http://hg.mozilla.org/comm-central/rev/4072d2cac1df>

-> FIXED
Allow empty properties: Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/92a426263440>
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
Yes, I kind of expected that, though didn't hope for it. I'll take care of it tomorrow...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
libical 0.40 offers cmake which should compile on windows...
does the above patch fix configure for the same thing? ;-)
Attachment #346637 - Flags: review?(mschroeder)
Comment on attachment 346637 [details] [diff] [review]
cleanup shared_makefiles.sh

r=mschroeder
Attachment #346637 - Flags: review?(mschroeder) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/404408453da0>

-> FIXED
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
Depends on: 557782
You need to log in before you can comment on or make changes to this bug.