m68k-atari-mint-gcc icon indicating copy to clipboard operation
m68k-atari-mint-gcc copied to clipboard

Embedding static zoneinfo in libstdc++

Open th-otto opened this issue 10 months ago • 8 comments

Something that i recently noticed while grepping through config.log:

configure:71667: zoneinfo data directory: none
configure:71682: static tzdata.zi file will be compiled into the library

I haven't checked yes, but i would bet that this has been the case already for quite some time. tzdata.zi is ~109k, and compiling it statically into the library seems a bad idea to me.

This could be fixed by using --with-libstdcxx-zoneinfo=no in configure scripts.

th-otto avatar Aug 26 '23 07:08 th-otto

Wouldn't this be fixed also by pointing the script to installed mintlib's timezone database? What files does it check for?

mikrosk avatar Aug 26 '23 10:08 mikrosk

The file is just a text file, and you could also use the hosts version theoretically. It is located in /usr/share/zoneinfo, both on linux and with mintlib. But i just want to avoid compiling in 109k of data (requiring to parse it at runtime), when that information can be taken from already precompiled, much smaller files, by using the already existing functions in mintlib.

Edit: have to check it, but it is possible that cross-compiled binaries are not affected by this (unless you point gcc to some existing file). But it would affect binaries generated by native compilers.

th-otto avatar Aug 26 '23 12:08 th-otto

Definitely, that's pointless. I'm just worried whether --with-libstdcxx-zoneinfo=no doesn't disable something in libstdc++ for good, i.e. whether all the time-related functions will still work as expected, just relying on underlying libc.

mikrosk avatar Aug 26 '23 17:08 mikrosk

Initial support for this was added in https://github.com/freemint/m68k-atari-mint-gcc/commit/559993b85744ae09d33eedb1cb062392ac482f94 so it is only present in the gcc-13 branch. There were also numerous patches for this afterwards.

Looks like when you use --with-libstdcxx-zoneinfo=no, instead of compiling the file in, it will be read at runtime, and still parsed. However it seems that only std::chrono::tzdb is affected by this, not the other time functions.

I still think that compiling in a static version is a bad idea, since that would make it impossible to replace it by a newer version.

Edit: --with-libstdcxx-zoneinfo=no is also wrong, that would disable the functionality completely. We have to use --with-libstdcxx-zoneinfo=/usr/share/zoneinfo. Problem with this is that this only works for linux, because then the paths for the cross-compiler and for mint happens to be the same, but not when building a cross-compiler on darwin or mingw, because /usr/share/zoneinfo either does not exist or does not contain a tzdata.zi file.

th-otto avatar Nov 18 '23 10:11 th-otto

Not to mention if somebody runs the binary on plain TOS. Conversion between time and zones isn't something which only unix-like apps are supposed to do properly. :)

mikrosk avatar Nov 18 '23 10:11 mikrosk

We could also use --with-libstdcxx-zoneinfo=/usr/share/zoneinfo,static. That should use the directory if it exists (and has a tzdata.zi file), but use the embedded version as a fallback.

OTOH, the usual time functions (that read the binary files from /usr/share/zoneinfo) are also affected by this. So i'm still not sure whats the better approach.

BTW, the static version is compiled in from a tzdata.zi file that is part of the c++ library (in libstdc++v3/src/c++20/tzdata.zi), and not taken from the host.

th-otto avatar Nov 18 '23 10:11 th-otto

Have you checked how much is executable's file size increased when linking the database statically?

mikrosk avatar Nov 30 '23 15:11 mikrosk

No, i didn't do any actual tests, because it is only used by the filesystem code (a rather rarely used C++20 feature). But it will increase at least by the size of the file (109k), plus some additional code.

I think it would be best to actually compile it in, for compatibility, and because it might be needed on TOS when the file is not available. But i still have to check how to configure it in MacOS/MinGW.

th-otto avatar Nov 30 '23 16:11 th-otto