After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 747890 - Revert some removed checks to make it work on FreeBSD
Revert some removed checks to make it work on FreeBSD
Status: RESOLVED FIXED
Product: libgweather
Classification: Core
Component: general
unspecified
Other FreeBSD
: Normal normal
: future
Assigned To: libgweather-maint
libgweather-maint
Depends on:
Blocks:
 
 
Reported: 2015-04-15 06:49 UTC by Ting-Wei Lan
Modified: 2015-04-27 01:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to bring back needed checks (4.36 KB, patch)
2015-04-15 06:51 UTC, Ting-Wei Lan
none Details | Review

Description Ting-Wei Lan 2015-04-15 06:49:52 UTC
nl_langinfo is supported by POSIX, but _NL_MEASUREMENT_MEASUREMENT isn't. We cannot assume all systems with nl_langinfo supports _NL_MEASUREMENT_MEASUREMENT.

timezone is a XSI extension, but it is not available on FreeBSD, NetBSD, and OpenBSD. A function with the same name already exists on FreeBSD and NetBSD, so it is not easy to add the timezone variable.

https://www.freebsd.org/cgi/man.cgi?query=timezone&apropos=0&sektion=0&manpath=FreeBSD+10.1-RELEASE&arch=default&format=html
http://netbsd.gw.com/cgi-bin/man-cgi?timezone++NetBSD-current
Comment 1 Ting-Wei Lan 2015-04-15 06:51:25 UTC
Created attachment 301593 [details] [review]
Patch to bring back needed checks
Comment 2 Giovanni Campagna 2015-04-17 00:55:32 UTC
Review of attachment 301593 [details] [review]:

Argh, I should have known better than relying on POSIX, I'll look into this but I don't want to reintroduce the old code.

The nl_langinfo() part is especially annoying because it introduces two completely different code paths, one of which is never tested, and it uses LC_MESSAGES instead of LC_MEASUREMENT, which is wrong.
Are you sure there is nothing we can use on BSDs?

::: libgweather/weather-metar.c
@@ +65,3 @@
+    /* mktime() assumes value is local, not UTC.  Use tm_gmtoff to compensate */
+#ifdef HAVE_TM_TM_GMOFF
+    return tm.tm_gmtoff + mktime (&tm);

This breaks glibc, because it needs _BSD_SOURCE instead of _XOPEN_SOURCE.
I really do not want to readd that if there is a portable way to do it.
Comment 3 Ting-Wei Lan 2015-04-17 18:30:58 UTC
(In reply to Giovanni Campagna from comment #2)
> Review of attachment 301593 [details] [review] [review]:
> 
> Argh, I should have known better than relying on POSIX, I'll look into this
> but I don't want to reintroduce the old code.
> 
> The nl_langinfo() part is especially annoying because it introduces two
> completely different code paths, one of which is never tested, and it uses
> LC_MESSAGES instead of LC_MEASUREMENT, which is wrong.

LC_MEASUREMENT isn't supported by POSIX, and it doesn't exist in *BSD.

> Are you sure there is nothing we can use on BSDs?

I did't find things that we can use, but it seems there are only 5 locales set to non-metric in glibc 2.21. Can we fallback to check whether the name of the locale ends with _US or _PR on non-glibc systems?


> 
> ::: libgweather/weather-metar.c
> @@ +65,3 @@
> +    /* mktime() assumes value is local, not UTC.  Use tm_gmtoff to
> compensate */
> +#ifdef HAVE_TM_TM_GMOFF
> +    return tm.tm_gmtoff + mktime (&tm);
> 
> This breaks glibc, because it needs _BSD_SOURCE instead of _XOPEN_SOURCE.
> I really do not want to readd that if there is a portable way to do it.

We can check the timezone variable before checking tm.tm_gmtoff, so glibc always use the timezone variable. Unfortunately, there is no autoconf macro can be used to check whether a global variable exists. I try to use the following code:

AC_MSG_CHECKING([whether you have timezone global variable])
AC_COMPILE_IFELSE(
    [AC_LANG_PROGRAM([[#include <time.h>]],
        [[long offset = _Generic (timezone, long: timezone);]])],
    [have_timezone=yes], [have_timezone=no])
AC_MSG_RESULT([$have_timezone])

if test "$have_timezone" = "yes"; then
    AC_DEFINE([HAVE_TIMEZONE], [1], [Define if we have timezone global variable])
fi

It works on both GNU/Linux (with GCC 4.9.2) and FreeBSD (with Clang 3.4.1), but it cannot be the real solution because there are many *BSD systems still using old compilers that don't support C11. 

I found timezone and tm.tm_gmtoff return values in different sign. We may need to fix it if we decide to keep both timezone and tm.tm_gmtoff.
Comment 4 Giovanni Campagna 2015-04-27 01:18:50 UTC
In the end I reverted everything for 3.17.1, because it was not
worth to replace some configure checks with other configure checks.
Thanks for testing this early and making sure it never got in any
release.