leosac / access-control

Leosac Access Control - Open Source Physical Access Control System
https://leosac.com
GNU Affero General Public License v3.0
240 stars 40 forks source link

CHAR_WIDTH macro name conflict #104

Closed knight-of-ni closed 6 years ago

knight-of-ni commented 6 years ago

This is not an urgent issue, but I thought I'd document it while it was still fresh in my mind.

libc6-dev 2.26-4 found in Debian testing (Buster) repo defines CHAR_WIDTH macro in /usr/include/limits.h. This conflicts with the macro of the same name found in deps/spdlog/fmt/bundled/format.h, which as you can see is part of the spdlog submodule.

Indeed, it appears this conflict has already been identified and resolved in the spdlog project: https://github.com/gabime/spdlog/issues/300

Naturally, my knee-jerk response was to update the spdlog submodule and try the build again. Unfortunately, the build fails with new errors. This time it complains about missing member functions and sinks.... so it would seem the spdlog api has changed. That is where I stopped troubleshooting for the time being.

Today, one can avoid this issue by just not doing a bulk apt-get update with the testing repo enabled. I should not have done that anyway. Eventually, we will need to address this.

xaqq commented 6 years ago

I have updated spdlog deps to use their latest release and fix the compile issue. Mostly some log level change, the path to the bundled format.h header and new SPDLOG_ENABLE_SYSLOG flag for the syslog_sink.

All should work now, i let you check as I don't have a Buster debian.

knight-of-ni commented 6 years ago

Wow, thanks for jumping on this so quickly. That would have taken me a week to figure out!

We are almost there, but develop branch now fails to build:

[ 81%] Linking CXX executable ../leosac
cd /tmp/tmp.aiPz3Rlt1I/leosac_0.7.0/obj-x86_64-linux-gnu/src && /usr/bin/cmake -E cmake_link_script CMakeFiles/leosac.dir/link.txt --verbose=1
/usr/bin/c++   -g -O2 -fdebug-prefix-map=/tmp/tmp.aiPz3Rlt1I/leosac_0.7.0=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -std=c++14 -Wno-undef -Wno-shadow -Wno-unknown-pragmas -O3 -DNDEBUG  -Wl,-z,relro CMakeFiles/leosac.dir/main.cpp.o  -o ../leosac -Wl,-rpath,/tmp/tmp.aiPz3Rlt1I/leosac_0.7.0/obj-x86_64-linux-gnu: -rdynamic ../libleosac_lib.so -lbacktrace -ldl -lpthread ../libzmqpp.so -lzmq -lboost_date_time -lboost_system -lboost_serialization -lboost_regex -lboost_filesystem -lodb -lodb-pgsql -lodb-sqlite -lodb-boost -lbacktrace -lscrypt ../libleosac_db.so 
../libleosac_lib.so: undefined reference to `int fmt::internal::CharTraits<char>::format_float<double>(char*, unsigned long, char const*, unsigned int, int, double)'
../libleosac_lib.so: undefined reference to `int fmt::internal::CharTraits<char>::format_float<long double>(char*, unsigned long, char const*, unsigned int, int, long double)'
collect2: error: ld returned 1 exit status
src/CMakeFiles/leosac.dir/build.make:110: recipe for target 'leosac' failed
make[3]: *** [leosac] Error 1
make[3]: Leaving directory '/tmp/tmp.aiPz3Rlt1I/leosac_0.7.0/obj-x86_64-linux-gnu'
CMakeFiles/Makefile2:194: recipe for target 'src/CMakeFiles/leosac.dir/all' failed
make[2]: *** [src/CMakeFiles/leosac.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....

I was surprised to see the leosac binary try to link against the bundled fmt library. Then I found this: https://github.com/fmtlib/fmt/issues/517

It looks like FMT_HEADER_ONLY is not defined. That macro gets defined in deps/spdlog/include/spdlog/fmt/fmt.h so I think we need to change these:

$ grep -r "format\.h" ./
./tools/ThreadUtils.cpp:#include <spdlog/fmt/bundled/format.h>
./tools/version.cpp:#include <spdlog/fmt/bundled/format.h>
./tools/db/Savepoint.cpp:#include <spdlog/fmt/bundled/format.h>
./exception/InvalidArgument.hpp:#include <spdlog/fmt/bundled/format.h>

to include spdlog/fmt/fmt.h, rather than include format.h directly.

UPDATE: I found time to knock this out this evening after all. Develop branch successfully builds with the proposed change. PR is incoming...

xaqq commented 6 years ago

It is a bit weird as I didn't experience this issue, at least on amd64, but it works with the PR too for me so...