Dual-Life / Time-Piece

Object Oriented time objects
Other
15 stars 33 forks source link

Time::Piece 1.32 fails to build on Windows with Visual Studio 2013 due to reliance on variable length arrays #33

Closed nanis closed 7 years ago

nanis commented 7 years ago

For background, see Shall I prefer constants over defines?.

In _get_localization, you have:

    const size_t bufsize = 150;
    char buf[bufsize];

This compiles fine with gcc because, by default, it accepts char buf[bufsize] even when not in in C99 mode:

Variable-length automatic arrays are allowed in ISO C99, and as an extension GCC accepts them in C90 mode and in C++. These arrays are declared like any other automatic arrays, but with a length that is not a constant expression.

On Windows, perl doesn't build with Visual Studio 2015 and later. Visual Studio 2013 and earlier do not implement C99 fully, so the above declaration causes compilation to fail.

AFAIK, one is supposed to be able to build Perl source with c89 and pedantic, so this may be a concern there as well, although gcc only warns in that case:

$ cat t.c
#include <stdlib.h>

int main(void)
{
    const size_t bufsize = 128;
    char buffer[bufsize];
    return EXIT_SUCCESS;
}
$ gcc -std=c89 -ansi -pedantic -c t.c
t.c: In function ‘main’:
t.c:6:5: warning: ISO C90 forbids variable length array ‘buffer’ [-Wvla]
     char buffer[bufsize];
     ^~~~

The simplest fix seems to be to use a local #define for those few lines:

        size_t i;
        size_t len;
    #define TIME_PIECE_BUFSIZE 150
        char buf[TIME_PIECE_BUFSIZE];
        const size_t bufsize = TIME_PIECE_BUFSIZE;
    #undef TIME_PIECE_BUFSIZE
        time_t t = 1325386800; /*1325386800 = Sun, 01 Jan 2012 03:00:00 GMT*/
        struct tm mytm = *gmtime(&t);

or similar.

smith153 commented 7 years ago

Ah that stinks. I see within _strfime there is char tmpbuf[128] which has been there for years without issue. So I sorta copied that. I've pushed out b9fdd22 but have not uploaded to cpan yet. If you feel up to it, give it a test. Strangely, I've not had any issues on any of the other windows smokers.

nanis commented 7 years ago

I don't know if there is anyone else who's testing using a perl built with Visual Studio 2013. Most other test reports I see come from people using Strawberry (that is, gcc).

I'll pull and test later tonight.

Thank you.

nanis commented 7 years ago

Hmmmm ... I get new failures:

C:\Users\sinan\src\Time-Piece (master) > nmake test                                                                                                            

Microsoft (R) Program Maintenance Utility Version 12.00.21005.1                                                                                                
Copyright (C) Microsoft Corporation.  All rights reserved.                                                                                                     

        "C:\opt\perl\5.24.1\bin\perl.exe" -MExtUtils::Command::MM -e cp_nonempty -- Piece.bs blib\arch\auto\Time\Piece\Piece.bs 644                            
        "C:\opt\perl\5.24.1\bin\perl.exe" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib\lib', 'blib\a
rch')" t\*.t                                                                                                                                                   
t\01base.t ...... ok                                                                                                                                           
t\02core.t ...... ok                                                                                                                                           
t\02core_dst.t .. ok                                                                                                                                           
t\03compare.t ... ok                                                                                                                                           
t\04mjd.t ....... ok                                                                                                                                           
t\05overload.t .. ok                                                                                                                                           
t\06subclass.t .. ok                                                                                                                                           
t\07arith.t ..... ok                                                                                                                                           
t\08truncate.t .. ok                                                                                                                                           
t\09locales.t ... ok                                                                                                                                           
t\99legacy.t .... 1/5                                                                                                                                          
#   Failed test 'LEGACY: parse array'                                                                                                                          
#   at t\99legacy.t line 17.                                                                                                                                   
#          got: '1969-12-31T19:00:00'                                                                                                                          
#     expected: '2000-01-01T06:00:00'                                                                                                                          

#   Failed test 'LEGACY: parse with no args dies'                                                                                                              
#   at t\99legacy.t line 19.                                                                                                                                   
#          got: '1969-12-31T19:00:00'                                                                                                                          
#     expected: '2000-01-01T06:00:00'                                                                                                                          

#   Failed test 'LEGACY: parse as non-method dies'                                                                                                             
#   at t\99legacy.t line 21.                                                                                                                                   
#          got: '1969-12-31T19:00:00'                                                                                                                          
#     expected: '2000-01-01T06:00:00'                                                                                                                          
# Looks like you failed 3 tests of 5.                                                                                                                          
t\99legacy.t .... Dubious, test returned 3 (wstat 768, 0x300)                                                                                                  
Failed 3/5 subtests                                                                                                                                            

Test Summary Report                                                                                                                                            
-------------------                                                                                                                                            
t\99legacy.t  (Wstat: 768 Tests: 5 Failed: 3)                                                                                                                  
  Failed tests:  3-5                                                                                                                                           
  Non-zero exit status: 3                                                                                                                                      
Files=11, Tests=448,  3 wallclock secs ( 0.11 usr +  0.06 sys =  0.17 CPU)                                                                                     
Result: FAIL                                                                                                                                                   
Failed 1/11 test programs. 3/448 subtests failed.                                                                                                              
NMAKE : fatal error U1077: 'C:\opt\perl\5.24.1\bin\perl.exe' : return code '0x3'                                                                               
Stop. 

That's an 11 hour difference. My PC is set to use America/New_York time zone (currently UTC-04:00).

Update: Well, it's a wee bit more than an 11 hour difference. I missed the 1969!

nanis commented 7 years ago

Incidentally, I get:

C:\... > perl -MTime::Local -E "say scalar localtime timelocal(0, 0, 6, 1, 0, 100)"
Sat Jan  1 06:00:00 2000

on this system, so the failures in 99legacy.t seem to be related to Time::Piece's _strftime ...

smith153 commented 7 years ago

I want to say that dates that come from strftime in Windows with a year of 1969 is just the default if something failed. And also, I don't think %s works on Windows either... hence getting the default 1969. But even worse, the parse() isn't even documented or used anywhere in the code. I've only left it because its been there for 10+ years... The tests for it were added during the PR challenge just get to 100% test code coverage. I will add a skip for all OS's other than linux.

smith153 commented 7 years ago

Ok, if you can, do a pull and try again. Should work this time, sorry.

nanis commented 7 years ago

Thank you. I don't think %s is even in POSIX, so in theory, that wouldn't just be a Windows fail ... in theory. I don't have my Windows laptop handy right now, but I'll check tomorrow morning.

nanis commented 7 years ago
C:\Users\sinan\src\Time-Piece (master) > git pull
remote: Counting objects: 13, done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 13 (delta 8), reused 13 (delta 8), pack-reused 0
Unpacking objects: 100% (13/13), done.
From https://github.com/Dual-Life/Time-Piece
   c7b26a7..1da8c79  master     -> origin/master
Updating c7b26a7..1da8c79
Fast-forward
 Changes      |  1 +
 Piece.pm     |  2 +-
 Piece.xs     |  4 ++++
 t/99legacy.t | 23 ++++++++++++++---------
 4 files changed, 20 insertions(+), 10 deletions(-)
...
t\01base.t ...... ok
t\02core.t ...... ok
t\02core_dst.t .. ok
t\03compare.t ... ok
t\04mjd.t ....... ok
t\05overload.t .. ok
t\06subclass.t .. ok
t\07arith.t ..... ok
t\08truncate.t .. ok
t\09locales.t ... ok
t\99legacy.t .... ok
All tests successful.
Files=11, Tests=448,  3 wallclock secs ( 0.09 usr +  0.01 sys =  0.11 CPU)
Result: PASS