Re: issues with Data::ICal::DateTime and possible DateTime::Set

[email protected] ("Flavio S. Glock") Fri, 18 Aug 2017 21:10:01 +0200
Newsgroups perl.datetime
Message-ID <CAHMRfDw0JF_cTGC-qfh_XUZbGtTwmQ+mzL9-ivWjXj3BnM=S9g@mail.gmail.com>
--001a113d31be443abf05570be029
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

This patch fixes the problem - but as I explained in the previous mail, I'm
not sure if it is the right thing to do (the alternative is that
DateTime::Event::ICal didn't ignore the time_zone in dtstart).

diff --git a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm
index ecf9e2f..ab49e78 100644
--- a/lib/Data/ICal/DateTime.pm
+++ b/lib/Data/ICal/DateTime.pm
@@ -499,6 +499,7 @@ sub _rule_set {
     for (@{ $self->property($name) }) {
         my $recur   =3D DateTime::Format::ICal->parse_recurrence(recurrenc=
e
=3D> $_->value, dtstart =3D> $start);
         # $recur->set_time_zone($_->parameters->{TZID}) if
$_->parameters->{TZID};
+        $recur->set_time_zone($start->time_zone) if
!$start->time_zone->is_floating;
         $set =3D $set->union($recur);
     }
     # $set->set_time_zone($tz);


2017-08-18 19:27 GMT+02:00 Flavio S. Glock <[email protected]>:

> (forwarding to DateTime list because this looks interesting)
>
> I've created this test - https://gist.github.com/fglock
> /cf1117ad000b41d9e5dbee6fa8b78993
>
> this fails (set time zone implicitly through dtstart):
>
>     $a =3D DateTime::Event::ICal->recur(
>             dtstart =3D> $dt19970902T090000_tz ,
>             freq =3D> 'daily',
>             count =3D> 10 )
>             ->intersection( $period_1995_1999 );
>
> this other test works (time zone is set explicitly for the whole
> recurrence):
>
>     $a =3D DateTime::Event::ICal->recur(
>             dtstart =3D> $dt19970902T090000_tz ,
>             freq =3D> 'daily',
>             count =3D> 10 )
>             ->set_time_zone( "America/New_York" )
>             ->intersection( $period_1995_1999 );
>
> I believe this is the correct behaviour (though it should warn!), because
> http://www.ietf.org/rfc/rfc2445.txt
> says:
>
> <quote>
>
> Dawson & Stenerson          Standards Track                   [Page 117]
> =0C
> RFC 2445                       iCalendar                   November 1998
>
>
>    The "DTSTART" and "DTEND" property pair or "DTSTART" and "DURATION"
>    property pair, specified within the iCalendar object defines the
>    first instance of the recurrence. When used with a recurrence rule,
>    the "DTSTART" and "DTEND" properties MUST be specified in local time
>    and the appropriate set of "VTIMEZONE" calendar components MUST be
>    included. For detail on the usage of the "VTIMEZONE" calendar
>    component, see the "VTIMEZONE" calendar component definition.</quote>
>
> I *think* DateTime::Event::ICal is doing the right thing, but the module =
documentation is not clear:
>
> <quote>
>
> This method takes parameters which correspond to the rule parts
> specified in section 4.3.10 of RFC 2445.  Rather than rewrite that RFC
> here, you are encouraged to read that first if you want to understand
> what all these parameters represent.
>
> </quote>
>
> It *could* as well also use the time_zone from "dtstart".
>
> Simon: do you think this explains the problem, and can this can be fixed =
in your module maybe?
>
> Fl=C3=A1vio S. Glock
>
>
>
> 2017-08-18 18:28 GMT+02:00 Flavio S. Glock <[email protected]>:
>
>> just to confirm, I did a fresh install and I get:
>>
>> t/01.parse_recurring.t ...... 1/18
>> #   Failed test at t/01.parse_recurring.t line 25.
>> #          got: 'floating'
>> #     expected: 'Europe/London'
>>
>> I'm investigating a bit
>>
>> Fl=C3=A1vio
>>
>>
>> 2017-08-18 17:33 GMT+02:00 Th.J. van Hoesel <[email protected]>:
>> > just fiddling along, it works if I first install
>> FGLOCK/DateTime-Event-Recurrence-0.16
>> >
>> >> On 18 Aug 2017, at 15:34, Th.J. van Hoesel <[email protected]>
>> wrote:
>> >>
>> >> Hi Fl=C3=A1vio, Hello Simon,
>> >>
>> >> i've some issues with installing Data::Ical::DateTime and I can not
>> exactly say who is in the right or who is in the wrong, that would requi=
re
>> more investigation.
>> >>
>> >> Data::Ical::DateTime is depending on DateTime::Event::Recurrence, wit=
h
>> is dependent on DateTime::Set. When that was changed back in november 20=
15,
>> that is when Data::ICal::DateTime started failing on CPAN testers.
>> >>
>> >> Maybe one does a wrong call and should actually had expected to get
>> "floating" timezones, maybe one does expected that his timezone from the
>> .ics test file would be honoured.
>> >>
>> >> Could you please be so kind and have a look, you probably do
>> understand the problem-space much better than I do. Would I have
>> understood, I probably would had sent a bug fix
>> >>
>> >> Theo
>> >>
>> >> PS. It is part of the "Act-out-of-the-Box" install script, and it jus=
t
>> looks ugly to do a force install, but it will do the job for now
>> >
>>
>
>
>

--001a113d31be443abf05570be029
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>This patch fixes the problem - but as I explained in =
the previous mail, I&#39;m not sure if it is the right thing to do (the alt=
ernative is that DateTime::Event::ICal didn&#39;t ignore the time_zone in d=
tstart).</div><div><br></div><font face=3D"monospace, monospace">diff --git=
 a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm<br>index ecf9e2f..=
ab49e78 100644<br>--- a/lib/Data/ICal/DateTime.pm<br>+++ b/lib/Data/ICal/Da=
teTime.pm<br>@@ -499,6 +499,7 @@ sub _rule_set {<br>=C2=A0 =C2=A0 =C2=A0for=
 (@{ $self-&gt;property($name) }) {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0my=
 $recur =C2=A0 =3D DateTime::Format::ICal-&gt;parse_recurrence(recurrence =
=3D&gt; $_-&gt;value, dtstart =3D&gt; $start);<br>=C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0# $recur-&gt;set_time_zone($_-&gt;parameters-&gt;{TZID}) if $_-&g=
t;parameters-&gt;{TZID};<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0$recur-&gt;set_tim=
e_zone($start-&gt;time_zone) if !$start-&gt;time_zone-&gt;is_floating;<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0$set =3D $set-&gt;union($recur);<br>=C2=
=A0 =C2=A0 =C2=A0}<br>=C2=A0 =C2=A0 =C2=A0# $set-&gt;set_time_zone($tz);</f=
ont><div><font face=3D"monospace, monospace"><br></font><div><div class=3D"=
gmail_extra"><br><div class=3D"gmail_quote">2017-08-18 19:27 GMT+02:00 Flav=
io S. Glock <span dir=3D"ltr">&lt;<a href=3D"mailto:[email protected]" targe=
t=3D"_blank">[email protected]</a>&gt;</span>:<br><blockquote class=3D"gmail=
_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:=
1ex"><div class=3D"HOEnZb"><div class=3D"h5"><div dir=3D"ltr"><div class=3D=
"gmail_quote"><span style=3D"font-family:arial,helvetica,sans-serif;color:r=
gb(0,0,0);white-space:pre-wrap">(forwarding to DateTime list because this l=
ooks interesting)</span></div><div class=3D"gmail_quote"><font color=3D"#00=
0000" face=3D"arial, helvetica, sans-serif"><span style=3D"white-space:pre-=
wrap"><br></span></font><div dir=3D"ltr">I&#39;ve created this test - <a hr=
ef=3D"https://gist.github.com/fglock/cf1117ad000b41d9e5dbee6fa8b78993" targ=
et=3D"_blank">https://gist.github.com/fglock<wbr>/cf1117ad000b41d9e5dbee6fa=
8b78<wbr>993</a><br><br>this fails (set time zone implicitly through dtstar=
t):<br><br><font face=3D"monospace, monospace">=C2=A0 =C2=A0 $a =3D DateTim=
e::Event::ICal-&gt;recur(<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dtst=
art =3D&gt; $dt19970902T090000_tz ,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 freq =3D&gt; &#39;daily&#39;,<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 count =3D&gt; 10 )<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -&gt=
;intersection( $period_1995_1999 );</font><div><br></div><div>this other te=
st works (time zone is set explicitly for the whole recurrence):</div><div>=
<br></div><div><div><font face=3D"monospace, monospace">=C2=A0 =C2=A0 $a =
=3D DateTime::Event::ICal-&gt;recur(</font></div><div><font face=3D"monospa=
ce, monospace">=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dtstart =3D&gt; $d=
t19970902T090000_tz ,</font></div><div><font face=3D"monospace, monospace">=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 freq =3D&gt; &#39;daily&#39;,</fo=
nt></div><div><font face=3D"monospace, monospace">=C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 count =3D&gt; 10 )</font></div><div><font face=3D"monospa=
ce, monospace">=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -&gt;set_time_zone=
( &quot;America/New_York&quot; )</font></div><div><font face=3D"monospace, =
monospace">=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -&gt;intersection( $pe=
riod_1995_1999 );</font></div></div><div><br></div><div>I believe this is t=
he correct behaviour (though it should warn!), because=C2=A0</div><div><a h=
ref=3D"http://www.ietf.org/rfc/rfc2445.txt" target=3D"_blank">http://www.ie=
tf.org/rfc/rfc244<wbr>5.txt</a><br></div><div>says:</div><div><br></div><di=
v>&lt;quote&gt;</div><div><pre style=3D"word-wrap:break-word">Dawson &amp; =
Stenerson =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Standards Track =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 [Page 117]<br>=0C<br>RFC 2=
445 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 iCalendar =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 November 1998<br><br><br>=C2=A0 =C2=A0The &quot;DTSTART&quot; and &q=
uot;DTEND&quot; property pair or &quot;DTSTART&quot; and &quot;DURATION&quo=
t;<br>=C2=A0 =C2=A0property pair, specified within the iCalendar object def=
ines the<br>=C2=A0 =C2=A0first instance of the recurrence. When used with a=
 recurrence rule,<br>=C2=A0 =C2=A0the &quot;DTSTART&quot; and &quot;DTEND&q=
uot; properties MUST be specified in local time<br>=C2=A0 =C2=A0and the app=
ropriate set of &quot;VTIMEZONE&quot; calendar components MUST be<br>=C2=A0=
 =C2=A0included. For detail on the usage of the &quot;VTIMEZONE&quot; calen=
dar<br>=C2=A0 =C2=A0component, see the &quot;VTIMEZONE&quot; calendar compo=
nent definition.<font color=3D"#000000"><span style=3D"white-space:pre-wrap=
">&lt;/quote&gt;</span></font></pre><pre style=3D"color:rgb(0,0,0);word-wra=
p:break-word;white-space:pre-wrap"><font face=3D"arial, helvetica, sans-ser=
if">I *think* DateTime::Event::ICal is doing the right thing, but the modul=
e documentation is not clear:</font></pre><pre style=3D"color:rgb(0,0,0);wo=
rd-wrap:break-word;white-space:pre-wrap"><font face=3D"arial, helvetica, sa=
ns-serif">&lt;quote&gt;</font></pre><pre style=3D"word-wrap:break-word"><fo=
nt color=3D"#000000"><span style=3D"white-space:pre-wrap"><font face=3D"mon=
ospace, monospace">This method takes parameters which correspond to the rul=
e parts
specified in section 4.3.10 of RFC 2445.  Rather than rewrite that RFC
here, you are encouraged to read that first if you want to understand
what all these parameters represent.</font><font face=3D"arial, helvetica, =
sans-serif">
</font></span></font></pre><div><font face=3D"arial, helvetica, sans-serif"=
>&lt;/quote&gt;</font></div><pre style=3D"color:rgb(0,0,0);word-wrap:break-=
word;white-space:pre-wrap"><font face=3D"arial, helvetica, sans-serif">It *=
could* as well also use the time_zone from &quot;dtstart&quot;.</font></pre=
><pre style=3D"color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">=
<font face=3D"arial, helvetica, sans-serif">Simon: do you think this explai=
ns the problem, and can this can be fixed in your module maybe?</font></pre=
><pre style=3D"color:rgb(0,0,0);word-wrap:break-word;white-space:pre-wrap">=
<font face=3D"arial, helvetica, sans-serif">Fl=C3=A1vio S. Glock</font><br>=
</pre><pre style=3D"color:rgb(0,0,0);word-wrap:break-word;white-space:pre-w=
rap"><font face=3D"arial, helvetica, sans-serif"><br></font></pre></div></d=
iv><div class=3D"m_1848727811611487409HOEnZb"><div class=3D"m_1848727811611=
487409h5"><div class=3D"gmail_extra"><br><div class=3D"gmail_quote">2017-08=
-18 18:28 GMT+02:00 Flavio S. Glock <span dir=3D"ltr">&lt;<a href=3D"mailto=
:[email protected]" target=3D"_blank">[email protected]</a>&gt;</span>:<br><b=
lockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1px =
#ccc solid;padding-left:1ex">just to confirm, I did a fresh install and I g=
et:<br>
<br>
t/01.parse_recurring.t ...... 1/18<br>
#=C2=A0 =C2=A0Failed test at t/01.parse_recurring.t line 25.<br>
#=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 got: &#39;floating&#39;<br>
#=C2=A0 =C2=A0 =C2=A0expected: &#39;Europe/London&#39;<br>
<br>
I&#39;m investigating a bit<br>
<br>
Fl=C3=A1vio<br>
<div class=3D"m_1848727811611487409m_-2462350057489538370HOEnZb"><div class=
=3D"m_1848727811611487409m_-2462350057489538370h5"><br>
<br>
2017-08-18 17:33 GMT+02:00 Th.J. van Hoesel &lt;<a href=3D"mailto:slrn40626=
[email protected]" target=3D"_blank">[email protected]</a>&gt;:<br>
&gt; just fiddling along, it works if I first install FGLOCK/DateTime-Event=
-Recurren<wbr>ce-0.16<br>
&gt;<br>
&gt;&gt; On 18 Aug 2017, at 15:34, Th.J. van Hoesel &lt;<a href=3D"mailto:t=
[email protected]" target=3D"_blank">[email protected]</a>&gt; wrote:<=
br>
&gt;&gt;<br>
&gt;&gt; Hi Fl=C3=A1vio, Hello Simon,<br>
&gt;&gt;<br>
&gt;&gt; i&#39;ve some issues with installing Data::Ical::DateTime and I ca=
n not exactly say who is in the right or who is in the wrong, that would re=
quire more investigation.<br>
&gt;&gt;<br>
&gt;&gt; Data::Ical::DateTime is depending on DateTime::Event::Recurrence, =
with is dependent on DateTime::Set. When that was changed back in november =
2015, that is when Data::ICal::DateTime started failing on CPAN testers.<br=
>
&gt;&gt;<br>
&gt;&gt; Maybe one does a wrong call and should actually had expected to ge=
t &quot;floating&quot; timezones, maybe one does expected that his timezone=
 from the .ics test file would be honoured.<br>
&gt;&gt;<br>
&gt;&gt; Could you please be so kind and have a look, you probably do under=
stand the problem-space much better than I do. Would I have understood, I p=
robably would had sent a bug fix<br>
&gt;&gt;<br>
&gt;&gt; Theo<br>
&gt;&gt;<br>
&gt;&gt; PS. It is part of the &quot;Act-out-of-the-Box&quot; install scrip=
t, and it just looks ugly to do a force install, but it will do the job for=
 now<br>
&gt;<br>
</div></div></blockquote></div><br></div>
</div></div></div><br></div>
</div></div></blockquote></div><br></div></div></div></div>

--001a113d31be443abf05570be029--