Bug #103275 for DateTime-TimeZone: Add DateTime::TimeZone::Local::Win32 to prereqs on windows builds?

Thu Apr 02 14:10:13 2015 WolfSage [...] cpan.org - Ticket created

Subject:

Add DateTime::TimeZone::Local::Win32 to prereqs on windows builds?

On windows, if you have DateTime::TimeZone installed, but not DateTime::TimeZone::Local::Win32, this apparently fails: DateTime->now(time_zone => 'local'); with: Cannot determine local time zone Perhaps this module should dynamically add DateTime::TimeZone::Local::Win32 to the dependency chain during Makefile.PL if it's being run on windows (if ::Win32 is truly needed to function there). -- Matthew Horsfall (alh)

Sun May 17 03:57:18 2015 dolmen [...] cpan.org - Correspondence added

Le 2015-04-02 20:10:13, WOLFSAGE a écrit :
Show quoted text

> On windows, if you have DateTime::TimeZone installed, but not
> DateTime::TimeZone::Local::Win32, this apparently fails:
>
> DateTime->now(time_zone => 'local');
>
> with:
>
> Cannot determine local time zone
>
> Perhaps this module should dynamically add
> DateTime::TimeZone::Local::Win32 to the dependency chain during
> Makefile.PL if it's being run on windows (if ::Win32 is truly needed
> to function there).

But there is a circular dependency issue:
https://rt.cpan.org/Ticket/Display.html?id=101589

--
Olivier Mengué - http://perlresume.org/DOLMEN - https://gratipay.com/dolmen/

Sun May 17 03:57:18 2015 The RT System itself - Status changed from 'new' to 'open'

Tue Jun 16 17:30:08 2015 ether [...] cpan.org - Correspondence added

This looks to be the root cause of: https://github.com/rjbs/Dist-Zilla/issues/450

Tue Jun 16 17:34:25 2015 DROLSKY [...] cpan.org - Correspondence added

On Tue Jun 16 17:30:08 2015, ETHER wrote: Show quoted text

> This looks to be the root cause of: https://github.com/rjbs/Dist- > Zilla/issues/450

Would reversing the dep direction improve things? We want to avoid a circular dep so it's one or the other.

Tue Jun 16 17:39:36 2015 ether [...] cpan.org - Correspondence added

Show quoted text

> > Perhaps this module should dynamically add > > DateTime::TimeZone::Local::Win32 to the dependency chain during > > Makefile.PL if it's being run on windows (if ::Win32 is truly needed > > to function there).

> > But there is a circular dependency issue: > https://rt.cpan.org/Ticket/Display.html?id=101589

This is awful. Is there another solution other than merging DateTime::TimeZone::Local::Win32 into the main DateTime-TimeZone installation? At the moment, win32 installations are not properly able to use timezones if they are missing DateTime::TimeZone::Local::Win32.

Tue Jun 16 17:52:53 2015 MITHALDU [...] cpan.org - Correspondence added

On Tue Jun 16 23:34:25 2015, DROLSKY wrote: Show quoted text

> Would reversing the dep direction improve things? We want to avoid a > circular dep so it's one or the other.

Look at this: http://grep.cpan.me/?q=DateTime%3A%3ATimeZone%3A%3ALocal%3A%3AWin32 No module that uses ::Local::Win32 doesn't also already dep on DateTime::TimeZone. This means it's safe to remove the dependency in ::Local::Win32 and have DT::TZ pull it in if needed.

Tue Jun 16 17:54:16 2015 ether [...] cpan.org - Requestor ETHER added

Tue Jun 16 18:14:38 2015 MITHALDU [...] cpan.org - Correspondence added

Retracting my previous comment, due to an issue haarg pointed out. Having TZ dep on Start with a system with neither of ::TZ or ::Win32 installed. Try to install ::TZ. ::TZ demands ::Win32 as dep. ::Win32 tries to install itself, but cannot run its tests since those require ::TZ. The situation is complexly broken. There are two possibilities in moving forward from here: 1. Rework ::Win32 so its tests do not require ::TZ. You would need to spool out the overwritten methods, test that, and then trust that the inclusion of them into ::Win32 works. 2. Merge it back into ::TZ and use the normal method of skipping things on Win32.

Tue Jun 16 20:26:22 2015 DROLSKY [...] cpan.org - Cc DAPINK added

Tue Jun 16 20:26:35 2015 DROLSKY [...] cpan.org - Correspondence added

On Tue Jun 16 18:14:38 2015, MITHALDU wrote: Show quoted text

> Retracting my previous comment, due to an issue haarg pointed out. > Having TZ dep on > > Start with a system with neither of ::TZ or ::Win32 installed. > > Try to install ::TZ. > > ::TZ demands ::Win32 as dep. > > ::Win32 tries to install itself, but cannot run its tests since those > require ::TZ. > > The situation is complexly broken. There are two possibilities in > moving forward from here: > > 1. Rework ::Win32 so its tests do not require ::TZ. You would need to > spool out the overwritten methods, test that, and then trust that the > inclusion of them into ::Win32 works. > > 2. Merge it back into ::TZ and use the normal method of skipping > things on Win32.

#2 isn't a great solution. I'm not in a position to maintain Win32 code since I don't develop on it. #1 is a lot of work for David Pinkowitz. #3 is to simply make most or all of the tests for Local::Win32 author tests that implicitly rely on DT::TZ being installed. That's probably the easiest change. Then I can make DT::TZ conditionally require Local::Win32 on Win32 boxes and Local::Win32 can stop depending on DT::TZ. David, what do you think?

Tue Jun 16 21:44:54 2015 ether [...] cpan.org - Correspondence added

Show quoted text

> #3 is to simply make most or all of the tests for Local::Win32 author > tests that implicitly rely on DT::TZ being installed. That's probably > the easiest change. Then I can make DT::TZ conditionally require > Local::Win32 on Win32 boxes and Local::Win32 can stop depending on > DT::TZ.

Instead of making them author tests, they should skip if DT::TZ isn't yet installed. That way not all test coverage is lost - the tests will still be run if upgrading an existing installation. Coverage from CPAN Testers smokers is invaluable for finding and fixing issues. However, I don't understand the objection to absorbing the win32 code -- I don't think this should be used as a factor in the decision. The community is available to help, as they are when win32-specific issues arise in our other code.

Tue Jun 16 22:12:51 2015 DROLSKY [...] cpan.org - Correspondence added

On Tue Jun 16 21:44:54 2015, ETHER wrote: Show quoted text

>

> > #3 is to simply make most or all of the tests for Local::Win32 author > > tests that implicitly rely on DT::TZ being installed. That's probably > > the easiest change. Then I can make DT::TZ conditionally require > > Local::Win32 on Win32 boxes and Local::Win32 can stop depending on > > DT::TZ.

> > Instead of making them author tests, they should skip if DT::TZ isn't > yet installed. That way not all test coverage is lost - the tests will > still be run if upgrading an existing installation. Coverage from CPAN > Testers smokers is invaluable for finding and fixing issues. > > However, I don't understand the objection to absorbing the win32 code > -- I don't think this should be used as a factor in the decision. The > community is available to help, as they are when win32-specific issues > arise in our other code.

The problem is that I can't _test_ the Win32 code easily, so I'm not in a good position to accept patches, test patches, etc. If the code has a separate maintainer who can do the work on a Win32 system, it seems to me that's the ideal situation. This code _used_ to be part of the core DT::TZ distro and we had exactly this problem. If fixing the prereq situation is enough to help other distros, I think that's the best solution.

Wed Jun 17 11:50:57 2015 rt-cpan [...] trout.me.uk - Correspondence added

On Tue Jun 16 20:26:35 2015, DROLSKY wrote: Show quoted text

> #3 is to simply make most or all of the tests for Local::Win32 author > tests that implicitly rely on DT::TZ being installed. That's probably > the easiest change. Then I can make DT::TZ conditionally require > Local::Win32 on Win32 boxes and Local::Win32 can stop depending on > DT::TZ.

That's basically how I'd've expected it to be done when you originally split it out, but y'know, hindsight, 20/20, etc. I'd vote for "tests must run in author mode, tests try and run under AUTOMATED_TESTING to provide cpan tester coverage, tests skip for 'normal' users to minimise the likelihood of confusion for them". Seems like a decent compromise between the available possibilities.

Wed Jun 17 12:24:11 2015 DAPINK [...] cpan.org - Correspondence added

On Tue Jun 16 21:44:54 2015, ETHER wrote: Show quoted text

>

> > #3 is to simply make most or all of the tests for Local::Win32 author > > tests that implicitly rely on DT::TZ being installed. That's probably > > the easiest change. Then I can make DT::TZ conditionally require > > Local::Win32 on Win32 boxes and Local::Win32 can stop depending on > > DT::TZ.

> > Instead of making them author tests, they should skip if DT::TZ isn't > yet installed. That way not all test coverage is lost - the tests will > still be run if upgrading an existing installation. Coverage from CPAN > Testers smokers is invaluable for finding and fixing issues. > > However, I don't understand the objection to absorbing the win32 code > -- I don't think this should be used as a factor in the decision. The > community is available to help, as they are when win32-specific issues > arise in our other code.

I will update the Win32 code to run the tests condtionally on the existence of the DT::TZ module. I agree we need the test coverage as it is usually through user testing where I can identify that Microsoft updated its time zone implementation to add or modify zones. As far as keeping the distros separate, there are practical reasons beyond that fact that Dave is not set up to test on Windows. The other is that DT::TZ mostly updates when there are changes to the TZ database. The Win32 code does not update as often, but is expected to update when Microsoft updates Windows time zones, which usually happens no more than a few times a year at most. Sure, it is possible to do them together, but separately, it seems more straightforward.

Mon Jun 22 03:55:38 2015 DAPINK [...] cpan.org - Correspondence added

On Wed Jun 17 12:24:11 2015, DAPINK wrote: Show quoted text

> On Tue Jun 16 21:44:54 2015, ETHER wrote:

> >

> > > #3 is to simply make most or all of the tests for Local::Win32 > > > author > > > tests that implicitly rely on DT::TZ being installed. That's > > > probably > > > the easiest change. Then I can make DT::TZ conditionally require > > > Local::Win32 on Win32 boxes and Local::Win32 can stop depending on > > > DT::TZ.

> > > > Instead of making them author tests, they should skip if DT::TZ isn't > > yet installed. That way not all test coverage is lost - the tests > > will > > still be run if upgrading an existing installation. Coverage from > > CPAN > > Testers smokers is invaluable for finding and fixing issues. > > > > However, I don't understand the objection to absorbing the win32 code > > -- I don't think this should be used as a factor in the decision. The > > community is available to help, as they are when win32-specific > > issues > > arise in our other code.

> > > I will update the Win32 code to run the tests condtionally on the > existence of the DT::TZ module. I agree we need the test coverage as > it is usually through user testing where I can identify that Microsoft > updated its time zone implementation to add or modify zones. > > As far as keeping the distros separate, there are practical reasons > beyond that fact that Dave is not set up to test on Windows. The other > is that DT::TZ mostly updates when there are changes to the TZ > database. The Win32 code does not update as often, but is expected to > update when Microsoft updates Windows time zones, which usually > happens no more than a few times a year at most. Sure, it is possible > to do them together, but separately, it seems more straightforward.

I've made a new release of DateTime::TimeZone::Local::Win32, removing the prerequisites to DateTime::TimeZone and DateTime.

Fri Jun 26 21:12:05 2015 stro [...] cpan.org - Correspondence added

Show quoted text

> I've made a new release of DateTime::TimeZone::Local::Win32, removing > the prerequisites to DateTime::TimeZone and DateTime.

See RT#105519 -- Serguei Trouchelle

Sat Jul 18 11:09:24 2015 DROLSKY [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #103275 for DateTime-TimeZone: Add DateTime::TimeZone::Local::Win32 to prereqs on windows builds?

Preferred bug tracker