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

[email protected] ("Flavio S. Glock") Mon, 21 Aug 2017 20:00:25 +0200
Newsgroups perl.datetime
Message-ID <CAHMRfDxxN_e9=2HEF626SQYdPxnQYJpMrTGPfbc1fqc2ivaseg@mail.gmail.com>
--001a1135c8005beb33055747438a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Data::ICal::DateTime 0.82 implements the fix discussed below.

0.82 - Mon 21 Aug 2017
    * Set recurrence timezone to the same as the 'start' attribute (fix
github issue #2)


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

> 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(recurre=
nce
> =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!), becaus=
e
>> 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 requ=
ire
>>> more investigation.
>>> >>
>>> >> Data::Ical::DateTime is depending on DateTime::Event::Recurrence,
>>> with is dependent on DateTime::Set. When that was changed back in novem=
ber
>>> 2015, 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 th=
e
>>> .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
>>> just looks ugly to do a force install, but it will do the job for now
>>> >
>>>
>>
>>
>>
>

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

<div dir=3D"ltr">Data::ICal::DateTime 0.82 implements the fix discussed bel=
ow.<div><br></div><div><div><font face=3D"monospace, monospace">0.82 - Mon =
21 Aug 2017</font></div><div><font face=3D"monospace, monospace">=C2=A0 =C2=
=A0 * Set recurrence timezone to the same as the &#39;start&#39; attribute =
(fix github issue #2)</font></div><div><br></div><div class=3D"gmail_extra"=
><br><div class=3D"gmail_quote">2017-08-18 21:10 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><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex"><div dir=3D"ltr"><div>This patch fixes the problem - but as I e=
xplained in the previous mail, I&#39;m not sure if it is the right thing to=
 do (the alternative is that DateTime::Event::ICal didn&#39;t ignore the ti=
me_zone in dtstart).</div><div><br></div><font face=3D"monospace, monospace=
">diff --git a/lib/Data/ICal/DateTime.pm b/lib/Data/ICal/DateTime.pm<br>ind=
ex ecf9e2f..ab49e78 100644<br>--- a/lib/Data/ICal/DateTime.pm<br>+++ b/lib/=
Data/ICal/DateTime.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_<wbr>rec=
urrence(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;<wbr>paramete=
rs-&gt;{TZID}) if $_-&gt;parameters-&gt;{TZID};<br>+ =C2=A0 =C2=A0 =C2=A0 =
=C2=A0$recur-&gt;set_time_zone($start-<wbr>&gt;time_zone) if !$start-&gt;ti=
me_zone-&gt;is_<wbr>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);</font><div><div class=3D"gmail-h5"><div><font =
face=3D"monospace, monospace"><br></font><div><div class=3D"gmail_extra"><b=
r><div class=3D"gmail_quote">2017-08-18 19:27 GMT+02:00 Flavio S. Glock <sp=
an dir=3D"ltr">&lt;<a href=3D"mailto:[email protected]" target=3D"_blank">fg=
[email protected]</a>&gt;</span>:<br><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex"><div class=3D"gmail-m_-3591553219584233613HOEnZb"><div class=3D"=
gmail-m_-3591553219584233613h5"><div dir=3D"ltr"><div class=3D"gmail_quote"=
><span style=3D"font-family:arial,helvetica,sans-serif;color:rgb(0,0,0);whi=
te-space:pre-wrap">(forwarding to DateTime list because this looks interest=
ing)</span></div><div class=3D"gmail_quote"><font color=3D"#000000" face=3D=
"arial, helvetica, sans-serif"><span style=3D"white-space:pre-wrap"><br></s=
pan></font><div dir=3D"ltr">I&#39;ve created this test - <a href=3D"https:/=
/gist.github.com/fglock/cf1117ad000b41d9e5dbee6fa8b78993" target=3D"_blank"=
>https://gist.github.com/fglock<wbr>/cf1117ad000b41d9e5dbee6fa8b78<wbr>993<=
/a><br><br>this fails (set time zone implicitly through dtstart):<br><br><f=
ont face=3D"monospace, monospace">=C2=A0 =C2=A0 $a =3D DateTime::Event::ICa=
l-&gt;recur(<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dtstart =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;intersectio=
n( $period_1995_1999 );</font><div><br></div><div>this other test works (ti=
me zone is set explicitly for the whole recurrence):</div><div><br></div><d=
iv><div><font face=3D"monospace, monospace">=C2=A0 =C2=A0 $a =3D DateTime::=
Event::ICal-&gt;recur(</font></div><div><font face=3D"monospace, monospace"=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dtstart =3D&gt; $dt19970902T0900=
00_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;,</font></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"monospace, monospace"=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -&gt;set_time_zone( &quot;Americ=
a/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( $period_1995_1999=
 );</font></div></div><div><br></div><div>I believe this is the correct beh=
aviour (though it should warn!), because=C2=A0</div><div><a href=3D"http://=
www.ietf.org/rfc/rfc2445.txt" target=3D"_blank">http://www.ietf.org/rfc/rfc=
244<wbr>5.txt</a><br></div><div>says:</div><div><br></div><div>&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 2445 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 iCalenda=
r =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 November 1=
998<br><br><br>=C2=A0 =C2=A0The &quot;DTSTART&quot; and &quot;DTEND&quot; p=
roperty pair or &quot;DTSTART&quot; and &quot;DURATION&quot;<br>=C2=A0 =C2=
=A0property pair, specified within the iCalendar object defines the<br>=C2=
=A0 =C2=A0first instance of the recurrence. When used with a recurrence rul=
e,<br>=C2=A0 =C2=A0the &quot;DTSTART&quot; and &quot;DTEND&quot; properties=
 MUST be specified in local time<br>=C2=A0 =C2=A0and the appropriate set of=
 &quot;VTIMEZONE&quot; calendar components MUST be<br>=C2=A0 =C2=A0included=
. For detail on the usage of the &quot;VTIMEZONE&quot; calendar<br>=C2=A0 =
=C2=A0component, see the &quot;VTIMEZONE&quot; calendar component definitio=
n.<font color=3D"#000000"><span style=3D"white-space:pre-wrap">&lt;/quote&g=
t;</span></font></pre><pre style=3D"color:rgb(0,0,0);word-wrap:break-word;w=
hite-space:pre-wrap"><font face=3D"arial, helvetica, sans-serif">I *think* =
DateTime::Event::ICal is doing the right thing, but the module documentatio=
n is not clear:</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">&lt;=
quote&gt;</font></pre><pre style=3D"word-wrap:break-word"><font color=3D"#0=
00000"><span style=3D"white-space:pre-wrap"><font face=3D"monospace, monosp=
ace">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.</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"gmail-m_-3591553219584233613m_1848727811611487409HOEnZb"><=
div class=3D"gmail-m_-3591553219584233613m_1848727811611487409h5"><div clas=
s=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><blockquote class=3D"=
gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(20=
4,204,204);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"gmail-m_-3591553219584233613m_1848727811611487409m_-246235005=
7489538370HOEnZb"><div class=3D"gmail-m_-3591553219584233613m_1848727811611=
487409m_-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></div></div>
</blockquote></div><br></div></div></div>

--001a1135c8005beb33055747438a--