Building on Windows from scratch - datediff problem found

[John Ralls]

On Jan 24, 2014, at 12:33 AM, Gary Bilkus <mail at gary.bilkus.com> wrote:

> On 23/01/2014 22:47, John Ralls wrote:
>> On Jan 23, 2014, at 7:23 AM, Gary Bilkus <mail at gary.bilkus.com> wrote:
>> 
>>> I've done some more testing and found a problem with libofx and possibly aqbanking ( which I don't use ). It's easy enough to work round, but I'm not sure what the correct fix should be on Windows
>>> 
>>>  The problem is that there's a known issue with datediff and mingw, due to differences in the versions of MSCVRT used by mingw as to whether they expect time_t to be 32 or 64 bits.
>>> 
>>>  If you know you are compiling for Windows 7, you can get it to work by explicitly setting the windows version appropriately, so that it will use the 64 bit versions of time_t ( even on 32 bit windows ). However, I suspect that compiling that way won't work on XP which doesn't have the 64 bit version.
>>> 
>>>  There is lots of noise about this issue in various forums, but no clear answer what the 'correct' solution is if you want to be able to compile on any supported Windows version, and ideally get a package which works everywhere as well.
>>> 
>>>  Anyone have any good ideas how to fix this cleanly. The least bad solution might be simply to replace datediff with a simple subtraction, which should work for the cases it's used in libofx. But other dependencies might be different.
>>> 
>> Are you perhaps thinking of “difftime”? DATEDIFF is a SQL function; difftime is a standard C Library function.
>> 
>> This bug seems to be what you’re talking about: http://sourceforge.net/p/mingw/bugs/2152/, linked from http://mingw.5.n7.nabble.com/difftime-broken-td33080.html.
>> 
>> So Microsoft has invented a new twist on the 32-bit vs. 64-bit time_t problem by having both and a rather graceless way of determining which to use. In GnuCash we finessed that by not using the standard C functions because we need 64-bit time_t regardless of whether the platform provides it. We’re currently using Glib functions that don’t use it; soon we’ll switch to Boost functions that don’t use it either.
>> 
>> It looks to from the bug that you have it backwards: You need to coerce 32-bit time_t because MinGW is screwed up and only calls difftime32() regardless of the size of time_t. That means that you can’t build GnuCash with MinGW wsl>=4.0 on a Win64 system until they fix the bug, or unless AQBanking and LibOFX work around the bug by replacing difftime with a subtraction-and-cast.
>> 
>> Regards,
>> John Ralls
>> 
> Sorry, yes, it is of course difftime I meant.
> I suppose I have it backwards if you mean that I want to use 64 bits but the latest mingw insists on 32 bits. In fact, it will call difftime64 if you define _WIN32_WINNT to _WIN32_WINNT_WIN7 inside _mingw.h . But while that proves the diagnosis, it clearly isn't the right patch!
> 
> This is not just a 'build on 64 bits' problem by the way. The problem exists even on 32 bit Windows XP using a VM I created to test on, although what's going wrong under the hood may be subtly different, and the WIN32_WINNT_WIN7 kludge obviously doesn't work! . So it's worth fixing if the aim is to have a 'just works' build of gnucash for Windows on mingw.
> 
> The safest fix is probably to define a 'sufficiently good' difftime and patch both libofx and aqbanking with that version as part of the install.sh process. But before I do that, I'd like to know that the gnucash maintainers would accept it, and if not, what alternative would be OK for eventual inclusion in geerts update.

You should update the MinGW bug report with the info about _WIN32_WINNT_WIN7; that's more breakage that might affect their solution.

What happens on XP? Since there's only 32-bit time_t in that ancient msvcrt, there should be only difftime and no difftime32 and difftime64 to choose from.  

Yes, of course it's worth fixing, ideally by the upstream developers, ideally by not using POSIX time functions. Stuffing a patch in GnuCash's 
windows build scripts is highly un-ideal, but might be necessary. Bear in mind that those build scripts primary purpose is to build GnuCash for distribution. Since I don't think that we want to create separate 32 and 64 bit packages, we need them to build for 32-bit by default since a 64-bit Windows will happily run a 32-bit app but not the other way around. If we're going to script for 64-bit builds as well then it should be enabled by an optional variable in custom.sh.

As for "sufficiently good" difftime, I imagine that the least-bad version would be what M$ should have done in the first place, which is

if (sizeof(time_t) == 8)
  return (double)((int64)time1 - (int64)time2);
else
  return (double)((int32)time1 - (int32)time2);

That could be done as a macro and inserted into one of the header files in each library.

Regards,
John Ralls