zic.c - compilation broken on gcc 4.8.3 after commit e5b2ee634c7151484c06740ea2c8a4d4a75409b6

Good time of day. This commit breaks compilation at least on GCC 4.8.3: commit e5b2ee634c7151484c06740ea2c8a4d4a75409b6 Author: Paul Eggert <eggert@cs.ucla.edu> Date: Thu Oct 20 11:41:27 2022 -0700 Use C23 __has_include if available It causes this compilation error: zic.c: In function ‘main’: zic.c:813:16: error: ‘optarg’ undeclared (first use in this function) if (strcmp(optarg, "slim") == 0) { ^ zic.c:906:6: error: ‘optind’ undeclared (first use in this function) if (optind == argc - 1 && strcmp(argv[optind], "=") == 0) ^ Specifically hunks like this are problematic: @@ -55,8 +63,8 @@ #endif #ifndef HAVE_GETTEXT -# define HAVE_GETTEXT 0 -#endif /* !defined HAVE_GETTEXT */ +# define HAVE_GETTEXT HAS_INCLUDE(<libintl.h>, false) +#endif #ifndef HAVE_INCOMPATIBLE_CTIME_R # define HAVE_INCOMPATIBLE_CTIME_R 0 The culprit is the replacement 0 -> false and 1 -> true. Apparently it breaks #if preprocessor logic in older compilers somewhere downstream - what was #if 1 becomes #if true, which GCC treats as "false" and falls into #else branch. "Restoring" false and true back to 0 and 1 seems to fix the issue. Best regards, Igor Ivanov, Software engineer at DSSL

On 2022-10-28 07:43, Иванов Игорь via tz wrote:
Apparently it breaks #if preprocessor logic in older compilers somewhere downstream - what was #if 1 becomes #if true, which GCC treats as "false" and falls into #else branch.
Thanks for reporting the problem. I think the problem is in the HAS_INCLUDE macro, which now that I look at it more closely doesn't conform to C23 either. Although I doubt whether the problem is the true/false stuff I'll move those macros earlier just in case; that's more robust anyway. Please try the attached patch, which I installed into the development repository on GitHub.

Thank you, Paul. I've checked the main branch and the issue with undeclared variables is gone now. I will also dare to mention another issue in this same thread, as it is kinda part of the same story (us trying to build tz on older gcc / glibc / distro). In our case, linker fails with: zic.o: In function `get_rand_u64': zic.c:(.text+0x1978): undefined reference to `clock_gettime' Usage of clock_gettime was introduced in the commit commit 0733c65c14c424b5d9de5ee250a8bf2802a0b259 Author: Paul Eggert <eggert@cs.ucla.edu> Date: Thu Oct 20 12:35:54 2022 -0700 Improve randomness of zic temp file names The problem is that our distro uses glibc-2.15. In these ancient times, to use `clock_*` functions you had to link against `rt` library - but since version glibc-2.17 these functions are a part of standard C library. The problem can be easily fixed from our side, just by adding a link flag to build command: make LDFLAGS='-lrt' ... Since even Centos 7 nowadays uses glibc-2.17, I doubt that one should tinker with tz code to account for this case - but it might be worthy mentioning this somewhere in readme. Or not. ________________________________ From: Paul Eggert <eggert@cs.ucla.edu> Sent: Saturday, October 29, 2022 00:53 To: Иванов Игорь <i.ivanov@dssl.ru> Cc: Time zone mailing list <tz@iana.org> Subject: Re: [tz] zic.c - compilation broken on gcc 4.8.3 after commit e5b2ee634c7151484c06740ea2c8a4d4a75409b6 On 2022-10-28 07:43, Иванов Игорь via tz wrote:
Apparently it breaks #if preprocessor logic in older compilers somewhere downstream - what was #if 1 becomes #if true, which GCC treats as "false" and falls into #else branch.
Thanks for reporting the problem. I think the problem is in the HAS_INCLUDE macro, which now that I look at it more closely doesn't conform to C23 either. Although I doubt whether the problem is the true/false stuff I'll move those macros earlier just in case; that's more robust anyway. Please try the attached patch, which I installed into the development repository on GitHub.

On 2022-10-31 01:49, Иванов Игорь wrote:
Since even Centos 7 nowadays uses glibc-2.17, I doubt that one should tinker with tz code to account for this case - but it might be worthy mentioning this somewhere in readme. Or not.
Another possibility is to not worry about the CLOCK_REALTIME stuff since it merely improves a terrible fallback to be a merely-bad fallback. Doing so avoids the configuration hassle entirely, which is a win. I installed the attached patch to do that. This suggests that we should do something similar with the other recently-introduced configuration hassles, if there's a reasonable way to do so.
participants (2)
-
Paul Eggert
-
Иванов Игорь