[From nobody Mon Jun 9 05:25:01 2014 Return-Path: <freeassociation-devel-bounces@lists.sourceforge.net> Received: from zeitsenke.de ([unix socket]) by zeitsenke.de (Cyrus v2.2.13-Debian-2.2.13-14+lenny4) with LMTPA; Tue, 16 Aug 2011 13:03:51 +0200 X-Sieve: CMU Sieve 2.2 Received: from localhost (localhost [127.0.0.1]) by zeitsenke.de (Postfix) with ESMTP id 00CCBC012271D for <patrick@estamos.de>; Tue, 16 Aug 2011 13:03:51 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at zeitsenke.de X-Spam-Flag: NO X-Spam-Score: -2.098 X-Spam-Level: X-Spam-Status: No, score=-2.098 required=5 tests=[AWL=-0.185, BAYES_00=-2.599, SPF_NEUTRAL=0.686] Received: from zeitsenke.de ([127.0.0.1]) by localhost (zeitsenke.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hK87vLZV-lRS for <patrick@estamos.de>; Tue, 16 Aug 2011 13:03:42 +0200 (CEST) Received: from mx0.gmx.net (mx0.gmx.net [213.165.64.100]) by zeitsenke.de (Postfix) with SMTP id B9663C012263A for <patrick@estamos.de>; Tue, 16 Aug 2011 13:03:42 +0200 (CEST) Received: (qmail 18808 invoked by alias); 16 Aug 2011 11:03:42 -0000 Delivered-To: GMX delivery to patrick.ohly@gmx.de Received: (qmail invoked by alias); 16 Aug 2011 11:03:41 -0000 Received: from lists.sourceforge.net (EHLO lists.sourceforge.net) [216.34.181.88] by mx0.gmx.net (mx070) with SMTP; 16 Aug 2011 13:03:41 +0200 Received: from localhost ([127.0.0.1] helo=sfs-ml-1.v29.ch3.sourceforge.com) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from <freeassociation-devel-bounces@lists.sourceforge.net>) id 1QtHQp-000506-8e; Tue, 16 Aug 2011 11:03:39 +0000 Received: from sog-mx-3.v43.ch3.sourceforge.com ([172.29.43.193] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from <patrick.ohly@gmx.de>) id 1QtHQn-0004zx-So for freeassociation-devel@lists.sourceforge.net; Tue, 16 Aug 2011 11:03:37 +0000 Received-SPF: pass (sog-mx-3.v43.ch3.sourceforge.com: domain of gmx.de designates 213.165.64.23 as permitted sender) client-ip=213.165.64.23; envelope-from=patrick.ohly@gmx.de; helo=mailout-de.gmx.net; Received: from mailout-de.gmx.net ([213.165.64.23]) by sog-mx-3.v43.ch3.sourceforge.com with smtp (Exim 4.76) id 1QtHQm-0000ph-KV for freeassociation-devel@lists.sourceforge.net; Tue, 16 Aug 2011 11:03:37 +0000 Received: (qmail invoked by alias); 16 Aug 2011 11:03:30 -0000 Received: from p5B37AE47.dip.t-dialin.net (EHLO [192.168.1.128]) [91.55.174.71] by mail.gmx.net (mp063) with SMTP; 16 Aug 2011 13:03:30 +0200 X-Provags-ID: V01U2FsdGVkX1+vULdBySGv6PErUtzbpdPgLlsdCS0uvtSUzFUJz4 QIG3Lo8zxjkdtS From: Patrick Ohly <patrick.ohly@gmx.de> To: Allen Winter <winter@kde.org> In-Reply-To: <201108122023.48753.winter@kde.org> References: <1313056347.4723.263.camel@pohly-mobl1.ikn.intel.com> <201108111921.36370.winter@kde.org> <1313131782.4723.294.camel@pohly-mobl1.ikn.intel.com> <201108122023.48753.winter@kde.org> Content-Type: multipart/mixed; boundary="=-/1vtPu98jDPE+SOEP0I6" Date: Tue, 16 Aug 2011 13:03:29 +0200 Message-ID: <1313492609.10938.44.camel@pohly-mobl1.ikn.intel.com> Mime-Version: 1.0 X-Mailer: Evolution 2.32.4 X-Headers-End: 1QtHQm-0000ph-KV Cc: freeassociation-devel@lists.sourceforge.net Subject: Re: [Freeassociation-devel] libical: TZID prefix X-BeenThere: freeassociation-devel@lists.sourceforge.net X-Mailman-Version: 2.1.9 Precedence: list List-Id: <freeassociation-devel.lists.sourceforge.net> List-Unsubscribe: <https://lists.sourceforge.net/lists/listinfo/freeassociation-devel>, <mailto:freeassociation-devel-request@lists.sourceforge.net?subject=unsubscribe> List-Archive: <http://sourceforge.net/mailarchive/forum.php?forum_name=freeassociation-devel> List-Post: <mailto:freeassociation-devel@lists.sourceforge.net> List-Help: <mailto:freeassociation-devel-request@lists.sourceforge.net?subject=help> List-Subscribe: <https://lists.sourceforge.net/lists/listinfo/freeassociation-devel>, <mailto:freeassociation-devel-request@lists.sourceforge.net?subject=subscribe> Errors-To: freeassociation-devel-bounces@lists.sourceforge.net X-GMX-Antispam: 0 (eXpurgate); Detail=5D7Q89H36p6zdxTM5oHCjQPZvS8GJdZWjY1PGV3T30aFhJg3RnuG9yoGLOH+VOXBZXa1i L8BdNiuR47o/WIYMDgftCXcpDgKyeYeaJvSMPWV+gPS96LwCG6rQVsFLBIAA458VdEjoTqrTFA7y qfJKTjfodjETv1P3IBfSSzSKZl9LMoq7AMsTzPljbpDw4PruY2s7nUTlJY1cueY4KLuRRns1qrOo w2DvjAiEdmHXYM=V1; X-Resent-By: Forwarder <forwarder@gmx.de> X-Resent-For: patrick.ohly@gmx.de X-Resent-To: patrick@estamos.de X-Evolution-Source: imap://patrick@zeitsenke-imaps/ --=-/1vtPu98jDPE+SOEP0I6 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On Fri, 2011-08-12 at 20:23 -0400, Allen Winter wrote: > On Friday 12 August 2011 2:49:42 AM Patrick Ohly wrote: > > On Do, 2011-08-11 at 19:21 -0400, Allen Winter wrote: > > > On Thursday 11 August 2011 5:52:27 AM Patrick Ohly wrote: > > > > Hello Allen! > > > > > > > > I'm having some issue with the libical compiled from source which might > > > > be related to your following commit: > > > > > > > Ok, reverted in SVN commit r1083 > > > > > > Please try now and let me know if things work better. > > > > I can't test that particular commit because the nightly test machine > > went down and I cannot reset it remotely myself. But I had already > > tested with a version where I reverted the commit myself. I can confirm > > that this fixes the problem for me. > > > > > regression test fails now btw. I'll try to fix that once Patrick reports back. > > > > Please continue. > > > > I think I fixed the regression test. > There was a bug in icaltimezone_get_builtin_timezone_from_tzid() > that didn't skip past the "/Tzfile" when extracting the location from the TZID. Your fix is: commit a65a0894ce7cf9672f9837dd38a95bbcba84b49b Author: awinterz <awinterz@b29e25b1-4845-0410-9c6e-9608a45a5811> Date: Sat Aug 13 00:11:14 2011 +0000 in icaltimezone_get_builtin_timezone_from_tzid(), skip past the 4th '/' character to find the location from the tzid, not the 3rd '/' character. This is needed because there has been a "/Tzfile" encoded into the tzid as of late. fixes the 'test_convenience' regression test. But that is not correct for the TZID as produced by the latest libical. In the test program from my initial email, I get TZID /freeassociation.sourceforge.net/Tzfile/Europe/Berlin. With the commit above, icaltimezone_get_builtin_timezone_from_tzid() skips forward to the "Berlin" part, which then fails to be found. I fail to see how the commit relates to the 'test_convenience'. Isn't that the one which uses icaltimezone_get_builtin_timezone("Europe/Rome")? Is icaltimezone_get_builtin_timezone_from_tzid() used indirectly in that test? [talking to myself while looking at the source...] Yes, that's it. The tests call icaltimezone_get_builtin_timezone_from_tzid() like this: #0 icaltimezone_get_builtin_timezone_from_tzid ( tzid=0x62d1f0 "/softwarestudio.org/Olson_20010626_2/Tzfile/Europe/Rome") at /home/nightly/freeassociation/libical/src/libical/icaltimezone.c:1468 #1 0x00007ffff777c3c8 in icalcomponent_get_datetime (comp=<value optimized out>, prop=<value optimized out>) at /home/nightly/freeassociation/libical/src/libical/icalcomponent.c:1587 #2 0x00007ffff777c41c in icalcomponent_get_dtstart (comp=0x6288f0) at /home/nightly/freeassociation/libical/src/libical/icalcomponent.c:1615 #3 0x0000000000407d79 in test_convenience () at /home/nightly/freeassociation/libical/src/test/regression.c:2282 #4 0x0000000000416f5a in test_run (test_name=<value optimized out>, test_fcn=0x406c40 <test_convenience>, do_test=0, headeronly=<value optimized out>) at /home/nightly/freeassociation/libical/src/test/regression-utils.c:170 #5 0x0000000000406b37 in main (argc=<value optimized out>, argv=0x7fffffffdff8) at /home/nightly/freeassociation/libical/src/test/regression.c:3813 The problem is that "/softwarestudio.org/Olson_20010626_2/Tzfile/Europe/Rome" is not an internal TZID for which icaltimezone_get_builtin_timezone_from_tzid() is meant to work - at least when internal timezones are taken from the zone database. Allen, were you perhaps testing on a system where internal TZIDs are not of the form /freeassociation.sourceforge.net/Tzfile/Europe/Berlin? How did you make strcmp("1997-08-01 12:00:00 Europe/Rome", ictt_as_string()) pass in test_convenience? In my understanding of the source it'll always insert the TZID, which is not Europe/Rome either way. I'm not sure what the right solution is: 1. Use three slashes consistently, including all tests? 2. Use four slashes consistently, including icaltz-util.c? 3. #ifdefs? Attached a patch which implements solution 1. I chose to go that route because the format /<source>/<version>/<location>/ seems to be the established one, with "Tzfile" taking the place of <version> when parsing zone info. Adding one more slash would introduce an unnecessary change compared to previous libical releases, too. -- Bye, Patrick Ohly -- Patrick.Ohly@gmx.de http://www.estamos.de/ --=-/1vtPu98jDPE+SOEP0I6 Content-Disposition: attachment; filename*0=0001-Revert-in-icaltimezone_get_builtin_timezone_from_tzi.pat; filename*1=ch Content-Type: text/x-patch; name="0001-Revert-in-icaltimezone_get_builtin_timezone_from_tzi.patch"; charset="UTF-8" Content-Transfer-Encoding: 7bit >From 2d77d4ce3e00cdbde85b0860e1796082809bc6bd Mon Sep 17 00:00:00 2001 From: Patrick Ohly <patrick.ohly@intel.com> Date: Tue, 16 Aug 2011 10:17:33 +0000 Subject: [PATCH 1/2] Revert "in icaltimezone_get_builtin_timezone_from_tzid(), skip past the 4th '/' character" This reverts commit a65a0894ce7cf9672f9837dd38a95bbcba84b49b. Builting TZIDs do not have four slashes. Therefore the change to icaltimezone_get_builtin_timezone_from_tzid() is incorrect. --- libical/src/libical/icaltimezone.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/libical/src/libical/icaltimezone.c b/libical/src/libical/icaltimezone.c index b4d998a..3a855c9 100644 --- a/libical/src/libical/icaltimezone.c +++ b/libical/src/libical/icaltimezone.c @@ -1477,18 +1477,17 @@ icaltimezone_get_builtin_timezone_from_tzid (const char *tzid) if (strncmp (tzid, ical_tzid_prefix, strlen(ical_tzid_prefix))) return NULL; - /* Get the location, which is after the 4th '/' character. */ - /* format is /unique_prefix/Olson_X/Tzfile/location */ + /* Get the location, which is after the 3rd '/' character. */ p = tzid; for (p = tzid; *p; p++) { if (*p == '/') { num_slashes++; - if (num_slashes == 4) + if (num_slashes == 3) break; } } - if (num_slashes != 4) + if (num_slashes != 3) return NULL; p++; -- 1.7.2.5 --=-/1vtPu98jDPE+SOEP0I6 Content-Disposition: attachment; filename="0002-regression-fixed-test_convenience.patch" Content-Type: text/x-patch; name="0002-regression-fixed-test_convenience.patch"; charset="UTF-8" Content-Transfer-Encoding: 7bit >From c8e31f486ca04be79953d92ad87cf69174d96556 Mon Sep 17 00:00:00 2001 From: Patrick Ohly <patrick.ohly@intel.com> Date: Tue, 16 Aug 2011 10:59:30 +0000 Subject: [PATCH 2/2] regression: fixed test_convenience The test failed for two reasons: - The prefix chosen by the test had one slash too many, causing icaltimezone_get_builtin_timezone_from_tzid() to fail because it expects internal TZIDs to have three slashes before the location part. - ictt_as_string() inserts the full TZID in the resulting string, not just the location. --- libical/src/test/regression.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libical/src/test/regression.c b/libical/src/test/regression.c index 1c7759a..d6abafd 100644 --- a/libical/src/test/regression.c +++ b/libical/src/test/regression.c @@ -2280,9 +2280,9 @@ void test_convenience(){ duration = icaldurationtype_as_int(icalcomponent_get_duration(c))/60; ok("Start is 1997-08-01 12:00:00 Europe/Rome", - (0 == strcmp("1997-08-01 12:00:00 Europe/Rome", ictt_as_string(icalcomponent_get_dtstart(c))))); + (0 == strcmp("1997-08-01 12:00:00 /softwarestudio.org/Tzfile/Europe/Rome", ictt_as_string(icalcomponent_get_dtstart(c))))); ok("End is 1997-08-01 13:30:00 Europe/Rome", - (0 == strcmp("1997-08-01 13:30:00 Europe/Rome", ictt_as_string(icalcomponent_get_dtend(c))))); + (0 == strcmp("1997-08-01 13:30:00 /softwarestudio.org/Tzfile/Europe/Rome", ictt_as_string(icalcomponent_get_dtend(c))))); ok("Duration is 90 m", (duration == 90)); icalcomponent_free(c); @@ -3733,7 +3733,7 @@ int main(int argc, char *argv[]) int do_header = 0; set_zone_directory("../../zoneinfo"); - icaltimezone_set_tzid_prefix("/softwarestudio.org/Olson_20010626_2/"); + icaltimezone_set_tzid_prefix("/softwarestudio.org/"); putenv("TZ="); test_start(0); -- 1.7.2.5 --=-/1vtPu98jDPE+SOEP0I6 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Disposition: inline Content-Transfer-Encoding: 7bit ------------------------------------------------------------------------------ uberSVN's rich system and user administration capabilities and model configuration take the hassle out of deploying and managing Subversion and the tools developers use with it. Learn more about uberSVN and get a free download at: http://p.sf.net/sfu/wandisco-dev2dev --=-/1vtPu98jDPE+SOEP0I6 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Disposition: inline Content-Transfer-Encoding: 7bit _______________________________________________ Freeassociation-devel mailing list Freeassociation-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freeassociation-devel --=-/1vtPu98jDPE+SOEP0I6-- ]