Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time: use 64-bit time_t in supported mingw environments #6636

Closed
wants to merge 1 commit into from

Conversation

@vszakats
Copy link
Member

@vszakats vszakats commented Feb 19, 2021

Enable 64-bit time_t in supported mingw-w64 and mingw environments. (Unless 32-bit time_t is selected manually via the _USE_32BIT_TIME_T mingw macro.)

Previously, 64-bit time_t was enabled on VS2005 and newer only, and 32-bit time_t was used on all other Windows builds.

@vszakats vszakats added the Windows label Feb 19, 2021
@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from 8f202dc to 6eb71b0 Feb 19, 2021
@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from 6eb71b0 to 278a3eb Feb 19, 2021
@jay
Copy link
Member

@jay jay commented Feb 20, 2021

  • for all other builds, allow to override the default time_t size by passing -DSIZEOF_TIME_T=8 to the compiler. This for example allows to enable 64-bit time_t for 32-bit Windows builds when using mingw-w64.

That's too hacky, why can't we do it through preprocessor directives?

#ifdef __MINGW32__
/* mingw or mingw-w64 */
#include <_mingw.h>
#ifdef __MINGW64_VERSION_MAJOR
/* mingw-w64 */
/* check for _USE_32BIT_TIME_T */
#else
/* mingw */
#endif
#else
/* other */
/* check for _USE_32BIT_TIME_T */
#endif

edit:
mingw-w64 time.h

original mingw time.h some versions at least also use _USE_32BIT_TIME_T but slightly different...

#ifndef _TIME_T_DEFINED
/* FIXME __STRICT_ANSI__ ! */
#if __MSVCRT_VERSION__ >= 0x0800
#ifndef _USE_32BIT_TIME_T
typedef	__time64_t time_t;
#else
typedef	__time32_t time_t;
#endif /* !_USE_32BIT_TIME_T */
#else
typedef	__time32_t time_t;
#endif /* __MSVCRT_VERSION__ >= 0x0800 */
#define _TIME_T_DEFINED
#endif
@vszakats
Copy link
Member Author

@vszakats vszakats commented Feb 20, 2021

@jay: Agreed, it'd be nice to have this automated, but kept it minimal to stay on the safe side. (and even got that wrong, because _WIN64 may indeed have 32-bit time_t as an option, just overlooked it.)

So, the mingw-w64 part I do understand, but not so much the classic mingw. Do you have a complete patch-suggestion that includes both? Or, I may tackle mingw-w64 in this patch and we can add classic mingw in a different PR.

@vszakats
Copy link
Member Author

@vszakats vszakats commented Feb 20, 2021

I've added the mingw-w64 part.

@vszakats vszakats changed the title mingw: enable/allow 64-bit time_t support mingw: fix time_t size for mingw-w64 builds Feb 20, 2021
lib/config-win32.h Outdated Show resolved Hide resolved
@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from c28edbc to 032e317 Feb 20, 2021
@vszakats
Copy link
Member Author

@vszakats vszakats commented Feb 20, 2021

Failure is on Azure / mingw, test1525 (seems unrelatd) [and did disappear in the next run), plus a long list of tests:

TESTFAIL: These test cases failed: (165) (310) (571) (612) (1056) 1239 1240 1241 1244 1245 1246 1248 1249 1250 1251 1252 1253 1254 1255 1256 1257 1258 1259 1261 1266 1267 1270 1271 1280 1283 1286 1287 1288 1290 1292 1293 1294 1295 1296 1297 1298 (1299) 1311 1312 1313 1314 1315 1317 1318 1322 1325 1326 1327 1328 1331 1332 1333 1334 1335 1336 1337 1338 1339 1340 1341 1342 1343 1344 1345 1346 1347 1364 1365 1366 1367 1368 1369 1370 1371 1372 1373 1374 1375 1376 1377 1400 1401 1402 1403 1404 1411 1413 1415 1416 1417 1419 1421 1422 1423 1424 1425 1426 1428 1429 1430 1431 1432 1433 1434 1438 1439 1443 (1448) 1455 1457 1458 1465 1500 1502 1503 1504 1505 1506 1509 1511 1514 1515 1516 1517 1518 1519 1522 1523 (2034) (2037) (2041) (2046) (2047) (3000) (3001) 

[these disappeared too.]

and lots of lines like this (these remained):

Use of uninitialized value in subtraction (-) at ./runtests.pl line 5220.
Use of uninitialized value in subtraction (-) at ./runtests.pl line 5221.
Use of uninitialized value in subtraction (-) at ./runtests.pl line 5232.
Use of uninitialized value in subtraction (-) at ./runtests.pl line 5234.

Pushing a cleaned up / squashed version and see how it goes.

@vszakats vszakats force-pushed the vszakats:mingw_time_t branch 2 times, most recently from 721d7dd to 4aaefc9 Feb 20, 2021
@vszakats
Copy link
Member Author

@vszakats vszakats commented Feb 20, 2021

One CI session still has those strange Perl error messages. Otherwise I consider this patch ready, with the note that mingw-classic support can be added in a subsequent commit. I personally have long stopped following mingw-classic and no ways to make tests with it. (Official binaries rely on mingw-w64.)

@vszakats vszakats force-pushed the vszakats:mingw_time_t branch 2 times, most recently from f05d942 to 4e2e525 Feb 20, 2021
@jay
Copy link
Member

@jay jay commented Feb 20, 2021

I've pushed recommended changes to address original mingw

@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from 3649ae9 to d95f342 Feb 20, 2021
@vszakats
Copy link
Member Author

@vszakats vszakats commented Feb 20, 2021

@jay: Thank you very much, LGTM and just squashed.

@vszakats vszakats force-pushed the vszakats:mingw_time_t branch 2 times, most recently from c2f4889 to d7424f6 Feb 20, 2021
@vszakats vszakats changed the title mingw: fix time_t size for mingw-w64 builds mingw: use 64-bit time_t when supported Feb 20, 2021
@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from d7424f6 to 3e03f48 Feb 20, 2021
Enable 64-bit `time_t` in supported mingw-w64 and mingw environments.
(Unless 32-bit `time_t` is selected manually via the `_USE_32BIT_TIME_T`
mingw macro.)

Previously, 64-bit `time_t` was enabled on VS2005 and newer only, and
32-bit `time_t` was used on all other Windows builds.

Assisted-by: Jay Satiro

Closes #xxxx
@vszakats vszakats force-pushed the vszakats:mingw_time_t branch from 3e03f48 to bd30f33 Feb 20, 2021
@vszakats vszakats changed the title mingw: use 64-bit time_t when supported time: use 64-bit time_t in supported mingw environments Feb 20, 2021
@vszakats vszakats closed this in 6b0f217 Feb 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants