estraier / tkrzw

a set of implementations of DBM
Apache License 2.0
164 stars 20 forks source link

Compile fails on FreeBSD12.2-rel-p4 / gcc10 #31

Closed puffetto closed 2 years ago

puffetto commented 2 years ago

Hello,

I am about to adopt tkrzw into a project of mine, currently developing on a FreeBSD machine.

<[TL;DR] Please find attached a PR with a couple of minor changes that make it work; sorry but I downloaded the code for the first time a couple of hours ago and cannot provide better solutions right now, apologizes also for the TL; message</[TL;DR]>

There are two issues preventing me from compiling the thing:

root@antani:~ uname -a
FreeBSD antani 12.2-RELEASE-p4 FreeBSD 12.2-RELEASE-p4 ANTANI  amd64
root@antani:~ g++10 --version
g++10 (FreeBSD Ports Collection) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
root@antani:~ gcc10 --version
gcc10 (FreeBSD Ports Collection) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
root@antani:~ cd tkrzw-1.0.23
root@antani:~/tkrzw-1.0.23 CC=gcc10 CXX=g++10 ./configure --enable-opt-native --enable-most-features
root@antani:~/tkrzw-1.0.23 gmake
g++10 -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_COMP_ZLIB -D_TKRZW_COMP_ZSTD -D_TKRZW_COMP_LZ4 -D_TKRZW_COMP_LZMA -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -llzma -llz4 -lzstd -lz -lstdc++ -lrt -latomic -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"1.0.23\"" -D_TKRZW_LIB_VERSION="\"1.66.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char -O3 -march=native tkrzw_lib_common.cc
g++10 -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_COMP_ZLIB -D_TKRZW_COMP_ZSTD -D_TKRZW_COMP_LZ4 -D_TKRZW_COMP_LZMA -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -llzma -llz4 -lzstd -lz -lstdc++ -lrt -latomic -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"1.0.23\"" -D_TKRZW_LIB_VERSION="\"1.66.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char -O3 -march=native tkrzw_str_util.cc
tkrzw_str_util.cc: In function 'void* tkrzw::tkrzw_memmem(const void*, size_t, const void*, size_t)':
tkrzw_str_util.cc:25:10: error: 'memmem' was not declared in this scope; did you mean 'memset'?
   25 |   return memmem(haystack, haystacklen, needle, needlelen);
      |          ^~~~~~
      |          memset
gmake: *** [Makefile:947: tkrzw_str_util.o] Error 1
root@antani:~/tkrzw-1.0.23 

and after fixing that (see below):

root@antani:~/tkrzw-1.0.23 g++10 -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -lstdc++ -lrt -latomic -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"1.0.23\"" -D_TKRZW_LIB_VERSION="\"1.66.0\"" -D__BSD_VISIBLE -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char tkrzw_str_util.cc
root@antani:~/tkrzw-1.0.23 gmake
g++10 -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -lstdc++ -lrt -latomic -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"1.0.23\"" -D_TKRZW_LIB_VERSION="\"1.66.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char tkrzw_hash_util.cc
g++10 -c -I. -I/usr/local/include -DNDEBUG -D_GNU_SOURCE=1 -D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L -D_FILE_OFFSET_BITS=64 -D_REENTRANT -D__EXTENSIONS__ -D_TKRZW_PREFIX="\"/usr/local\"" -D_TKRZW_INCLUDEDIR="\"/usr/local/include\"" -D_TKRZW_LIBDIR="\"/usr/local/lib\"" -D_TKRZW_BINDIR="\"/usr/local/bin\"" -D_TKRZW_LIBEXECDIR="\"/usr/local/libexec\"" -D_TKRZW_APPINC="\"-I/usr/local/include\"" -D_TKRZW_APPLIBS="\"-L/usr/local/lib -ltkrzw -lstdc++ -lrt -latomic -lpthread -lm -lc \"" -D_TKRZW_PKG_VERSION="\"1.0.23\"" -D_TKRZW_LIB_VERSION="\"1.66.0\"" -MMD -g -O2 -std=c++17 -pthread -fPIC -fsigned-char @.cc
tkrzw_time_util.cc: In function 'int32_t tkrzw::GetLocalTimeDifference()':
tkrzw_time_util.cc:88:29: error: expected primary-expression before ')' token
   88 |     tz_cache.store(-timezone);
      |                             ^
gmake: *** [Makefile:947: tkrzw_time_util.o] Error 1

The first problem looks quite trivial, tkrzw tries to use system's "memmem()", which under FreeBSD's glibc[++] is not exposed (as not being standard) if you compile with _POSIX_C_SOURCE defined; a quick (and dirty) workaround is to manually compile tkrzw_str_util.cc forcing "-D__BSD_VISIBLE" as I did: another workaround could be to make the code use its internal "memmem()" implementation by adding a trailing "&& !defined(_SYSFREEBSD)" to line 21 of tkrzw_str_util.cc; this should probably be fixed with proper configure macros.

Going on and forcing the compilation of tkrzw_str_util.cc with -D__BSD_VISIBLE the second issue arises, and here the problem is that I really do not understand what the code is supposed to do at line 88:

// File tkrzw_time_util.cc line 69+:

int32_t GetLocalTimeDifference() {
#if defined(_SYS_WINDOWS_)
  static std::atomic_int32_t tz_cache(INT32MIN);
  int32_t tz_value = tz_cache.load();
  if (tz_value == INT32MIN) {
    const time_t t = 86400;
    struct std::tm uts;
    GetUniversalCalendar(t, &uts);
    struct std::tm lts;
    GetLocalCalendar(t, &lts);
    tz_cache.store(std::mktime(&lts) - std::mktime(&uts));
    tz_value = tz_cache.load();
  }
  return tz_value;
#else
  static std::atomic_int32_t tz_cache(INT32MIN);
  int32_t tz_value = tz_cache.load();
  if (tz_value == INT32MIN) {
    tzset();
    tz_cache.store(-timezone);
    tz_value = tz_cache.load();
  }
  return tz_value;
#endif
}

Fact is that "timezone" as object or variable is not defined anywhere in the code, nor in the standard. I wonder how this can even compile whenever it is "!defined(_SYSWINDOWS)" (???)

If the intention is to store into tz_cache the difference in seconds between the local time and UTC time I attach a PR to fix it, however please before pulling it in take note that: a. It is conceptually broken (this value is NOT a "number", it will change even during the running time of the program, in example on leap seconds or Daylight Savings Time Changes). b. If it is used only for logging purposes or human interactions it might be ok (but I would always prefer logs in UTC...); if it is used for any internal purpose that actually impacts the functionality of the things (i.e. synchronization / precedence) ... it WILL cause problems. c. Both C++20 and several external libraries provide far cleaner and safer ways to do basically everything I see in tkrzw_time_util; I would suggest to take a look at that in the future.

Needless to say: with the two PRs merged the thing compiles happily under FreeBSD12/gcc10, passes "make check" and, so far, runs like charm, just:

CC=gcc10 CXX=g++10 ./configure --enable-opt-native --enable-most-features && gmake && gmake check && sudo gmake install

Thanks for your wonderful work; I love avoiding to depend on AGPL licensed Oracle's code ;)

A.

29 #30

estraier commented 2 years ago

Thanks for catching them. As for the time zone, the signature is like the following on Linux, MacOS, and other UNIX-like systems. It is just an integer variable.

extern long timezone;

According to the GNU man page, BSD 4.3 introduced different signature for it. It's a function.

char *timezone(zone, dst)

Anyway, as you suggested, the local time difference can change anytime. Thus caching it can cause loss/gain. I assume the purpose of those time utilities are for logging as you said. And, I designed the current behavior for performance reason. Logging functions can be called a thousand times in a second. For the time being, I'll add a caveat comment on it. I know that C++20 provides better time utility. However, Tkrzw's target platform is C++17 and later. So, I cannot use C++20 features, at least currently.

All in all, I'll take your PR and modify some code on top of it.

estraier commented 2 years ago

I pushed another change related to this.

puffetto commented 2 years ago

Thanks, I confirm that now it compiles and passes "make check", on FreeBSD12/gcc10. While there I also tested macOS Monterey 12.2.1 (aka Darwin 21.3.0) and everything looks ok here too. Only one caveat on FreeBSD: portinstall gcc10 and then force configure to use it: blackye@antani:~/test/tkrzw CC=gcc10 CXX=c++10 ./configure --enable-opt-native --enable-most-features Thank you for your work: I really appreciate it (as well as I did with Tokyo/Kyoto Cabinet)! The only serious issue is the name... I admit that I always google "Kyoto Cabinet" and then follow the link to tkrzw; remembering this name is too much for me :D

puffetto commented 2 years ago

Forgot to close the issue, doing it now. Should I see any problem in production/under-load I will report in a separate one (and try to fix it, as far as I can).

puffetto commented 2 years ago

Adding a comment on this closed issue just to keep you posted on further updates. Using it in my code is becoming a nightmare, mostly because I rely on two things: being compiler-agnostic and standard-compliant. On my production environment (FreeBSD) "the" compiler, nowadays, is LLVM. Fact is that there are issues linking a library compiled with GCC with code written on LLVM, not only: even compiling tkrzw-rpc turns out to be a problem (if compiled with GCC it won't link with grpc installed as a port [and thus compiled with LLVM]; if compiled with LLVM won't link with "tkrzw" compiled as above with CC=gcc10 CXX=c++10. So far I did a quick&dirty test: I know that libraries, headers and LLVM itself is a lot more picky when you ask for "compliance"; as a matter of fact I just compiled the thing on LLVM simply removing "-D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L" from MYCPPFLAGS in configure.in, running autoconf and compiling "as per README" and... it compiled without a warning and passed "make check". I Will not, of course, suggest this line of action before digging in the code, which I can't do right now. I might have some spare hours next weekend to dedicate to a "make it compatible with LLVM (or better: with any standard-compliant compiler)". If you have any hint about:

yonas commented 2 years ago

@puffetto With the changes you mentioned, I can get further down the compilation, but it breaks at tkrzw_build_util:

$ make
...
c++ -g -O2 -std=c++17 -pthread -fPIC -fsigned-char -O3 -march=native -o tkrzw_build_util  -L. -L/usr/local/lib -L. -L/usr/local/lib -static -ltkrzw -llzma -llz4 -lzstd -lz -lstdc++ -lrt -Wl,--whole-archive -lpthread -Wl,--no-whole-archive -lm -lc
ld: error: undefined symbol: main
>>> referenced by crt1_c.c:75 (/usr/src/lib/csu/amd64/crt1_c.c:75)
>>>               /usr/lib/crt1.o:(_start)
c++: error: linker command failed with exit code 1 (use -v to see invocation)
*** Error code 1

Stop.
make: stopped in /usr/home/yonas/git/make-cd/files/tkrzw/cpp

I'm on FreeBSD 13.1-RELEASE, Clang 13.0.0 (git@github.com:llvm/llvm-project.git llvmorg-13.0.0-0-gd7b669b3a303)

estraier commented 1 year ago

Hi, The reason why the target platform is only GCC is not so strong. My main platform (Ubuntu Linux 21.10) uses GCC 11.20. Though I also run tests on MacOS and Windows, doing so on every possible platform is not practical. Of course, I want to support as many platforms as possible, by modifying code and configuration if necessary and someone gives me how-tos. As for regression tests, basic tests are done by "make check" and more complicated ones are done by "make test && make testrun", which requires Google Test. I'd add more test cases occasionally when I think it is needed.

You suggested that some compatibility macros have side effects on LLVM. What are error messages? I believe those compatibility macros are basically to show new symbols, which can cause conflicts if the application code uses the same names. As GCC on MacOS (XCode) is actually CLang's wrapper, Tkrzw and Tkrzw-RPC should not be hostile to LLVM, at least on MacOS. Anyway, I'm curious why you have to mix multiple compilers on one platform? I'd rather avoid it for fear of nightmare of tampering with complicated linker options :(

On Mon, Mar 28, 2022 at 4:38 AM Andrea Cocito @.***> wrote:

Adding a comment on this closed issue just to keep you posted on further updates. Using it in my code is becoming a nightmare, mostly because I rely on two things: being compiler-agnostic and standard-compliant. On my production environment (FreeBSD) "the" compiler, nowadays, is LLVM. Fact is that there are issues linking a library compiled with GCC with code written on LLVM, not only: even compiling tkrzw-rpc turns out to be a problem (if compiled with GCC it won't link with grpc installed as a port [and thus compiled with LLVM]; if compiled with LLVM won't link with "tkrzw" compiled as above with CC=gcc10 CXX=c++10. So far I did a quick&dirty test: I know that libraries, headers and LLVM itself is a lot more picky when you ask for "compliance"; as a matter of fact I just compiled the thing on LLVM simply removing "-D_ISOC99_SOURCE=1 -D_POSIX_C_SOURCE=999999L" from MYCPPFLAGS in configure.in, running autoconf and compiling "as per README" and... it compiled without a warning and passed "make check". I Will not, of course, suggest this line of action before digging in the code, which I can't do right now. I might have some spare hours next weekend to dedicate to a "make it compatible with LLVM (or better: with any standard-compliant compiler)". If you have any hint about:

  • Critical points about why it is "Only GCC supported"
  • Reggression tests ... I would really appreciate them. Thanks, A.

— Reply to this email directly, view it on GitHub https://github.com/estraier/tkrzw/issues/31#issuecomment-1080005091, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQGJVRC4FOFQU5FGUAYQ2ADVCDBNHANCNFSM5RUYWNWQ . You are receiving this because you commented.Message ID: @.***>