Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spdlog 1.14.0: undefined reference to `spdlog::get(char const*)' #3081

Closed
FlorianReimold opened this issue Apr 29, 2024 · 14 comments
Closed

Comments

@FlorianReimold
Copy link

FlorianReimold commented Apr 29, 2024

Hi,

We are using spdlog in Eclipse eCAL.

Is this API break intended / known?

Kind Regards
Florian

@gabime
Copy link
Owner

gabime commented Apr 29, 2024

It is not intended. Please provide steps to reproduce.

@FlorianReimold
Copy link
Author

FlorianReimold commented Apr 29, 2024

Hi @gabime,

This is the actual project (Eclipse eCAL) that fails building. The long CMake Options mostly turn everything off while still building enough to trigger the issue.
I know that this still isn't really a minimal setup. Creating one will cost me some time, so please tell me if you require one.

The commands target Ubuntu 22.04 but should work on other Ubuntu versions as well.

sudo apt-get -y update
sudo apt-get -y install git cmake  build-essential libprotobuf-dev libprotoc-dev protobuf-compiler libyaml-cpp-dev
 
git clone https://github.com/eclipse-ecal/ecal.git
 
cd ecal
 
git submodule init
git submodule update
 
git -C thirdparty/spdlog/spdlog/ checkout v1.14.0
 
mkdir _build
cd _build
 
cmake ..  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON  -DHAS_HDF5=OFF  -DHAS_QT=OFF  -DHAS_CURL=OFF  -DHAS_CAPNPROTO=OFF  -DHAS_FTXUI=OFF  -DBUILD_DOCS=OFF  -DBUILD_APPS=ON  -DBUILD_SAMPLES=OFF  -DBUILD_TIME=OFF  -DBUILD_PY_BINDING=OFF  -DBUILD_STANDALONE_PY_WHEEL=OFF  -DBUILD_CSHARP_BINDING=OFF  -DBUILD_ECAL_TESTS=OFF  -DECAL_INCLUDE_PY_SAMPLES=OFF  -DECAL_INSTALL_SAMPLE_SOURCES=OFF  -DECAL_JOIN_MULTICAST_TWICE=OFF  -DECAL_NPCAP_SUPPORT=OFF  -DECAL_THIRDPARTY_BUILD_CMAKE_FUNCTIONS=ON  -DECAL_THIRDPARTY_BUILD_PROTOBUF=OFF  -DECAL_THIRDPARTY_BUILD_SPDLOG=ON  -DECAL_THIRDPARTY_BUILD_TINYXML2=ON  -DECAL_THIRDPARTY_BUILD_FINEFTP=ON  -DECAL_THIRDPARTY_BUILD_CURL=OFF  -DECAL_THIRDPARTY_BUILD_GTEST=OFF  -DECAL_THIRDPARTY_BUILD_HDF5=OFF  -DECAL_THIRDPARTY_BUILD_RECYCLE=ON  -DECAL_THIRDPARTY_BUILD_TCP_PUBSUB=ON  -DECAL_THIRDPARTY_BUILD_QWT=OFF  -DECAL_THIRDPARTY_BUILD_YAML-CPP=OFF  -DCMAKE_BUILD_TYPE=Debug

make -j8

The error printed by this is:

[ 85%] Linking CXX executable ../../../bin/ecal_sys_client
/usr/bin/ld: CMakeFiles/sys_client.dir/src/ecal_sys_client_cli.cpp.o: in function `eCAL::sys_client::EcalSysClientLogger::EcalSysClientLogger()':
/home/florian/ecal/app/sys/sys_client_core/include/sys_client_core/ecal_sys_client_logger.h:80: undefined reference to `spdlog::get(char const*)'
/usr/bin/ld: /home/florian/ecal/app/sys/sys_client_core/include/sys_client_core/ecal_sys_client_logger.h:96: undefined reference to `spdlog::get(char const*)'
collect2: error: ld returned 1 exit status

Checking out spdlog 1.13.0 instead of 1.14.0 resolves the issue.

@tt4g
Copy link
Contributor

tt4g commented Apr 29, 2024

This commit (5532231) adds overloads to spdlog::get() if C++17 or later is specified.
ABI is broken after C++17.

@gabime
Copy link
Owner

gabime commented Apr 29, 2024

This could be something in the project setup(e.g. linking to wrong spdlog version). Pleas provide minimal reproducible example. The steps you provided are time consuming and project specific.

@FlorianReimold
Copy link
Author

FlorianReimold commented Apr 29, 2024

@tt4g: Thanks for pointing that out. As I see it, that header takes the C++ version of the library consumer into account and not the C++ version used for compiling spdlog. So consuming a C++14 compatible spdlog binary with a C++ 17 project will break.

The proper solution I guess would for example be to configure a header file while building spdlog and saving whether the lib was built with / without C++17 API. This header file can then be included later by the public API headers, so the existence of the overloads depend on the actual library and not on the C++ standard of the consumer.
Another solution would be to set public preprocessor defines via cmake (so no configured header file is needed) that then activate those overloads for the consumer.

I checked the build logs and found the following:

spdlog is built with C++11

[  0%] Building CXX object thirdparty/spdlog/spdlog/CMakeFiles/spdlog.dir/src/spdlog.cpp.o
cd /home/florian/ecal/_build/thirdparty/spdlog/spdlog && /usr/bin/c++ -DSPDLOG_COMPILED_LIB -I/home/florian/ecal/thirdparty/spdlog/spdlog/include -g   -Wall -Wextra -std=c++11 -MD -MT thirdparty/spdlog/spdlog/CMakeFiles/spdlog.dir/src/spdlog.cpp.o -MF CMakeFiles/spdlog.dir/src/spdlog.cpp.o.d -o CMakeFiles/spdlog.dir/src/spdlog.cpp.o -c /home/florian/ecal/thirdparty/spdlog/spdlog/src/spdlog.cpp
[  0%] Building CXX object thirdparty/spdlog/spdlog/CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o
cd /home/florian/ecal/_build/thirdparty/spdlog/spdlog && /usr/bin/c++ -DSPDLOG_COMPILED_LIB -I/home/florian/ecal/thirdparty/spdlog/spdlog/include -g   -Wall -Wextra -std=c++11 -MD -MT thirdparty/spdlog/spdlog/CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o -MF CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o.d -o CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o -c /home/florian/ecal/thirdparty/spdlog/spdlog/src/stdout_sinks.cpp

ecal_sys_client is built with C++17 (gcc 11 default)

cd /home/florian/ecal/_build/app/sys/sys_client_cli && /usr/bin/cmake -E cmake_link_script CMakeFiles/sys_client.dir/link.txt --verbose=1
/usr/bin/c++ -g CMakeFiles/sys_client.dir/src/ecal_sys_client_cli.cpp.o CMakeFiles/sys_client.dir/src/ecal_sys_client_service.cpp.o -o ../../../bin/ecal_sys_client-5.13.0  -Wl,-rpath,/home/florian/ecal/_build/lib: ../../../thirdparty/spdlog/spdlog/libspdlogd.a /usr/lib/x86_64-linux-gnu/libprotobuf.so ../sys_client_core/libsys_client_core.a ../../../lib/CustomTclap/libecal_CustomTclap.a ../../../lib/ecal_utils/libecal_ecal-utils.a ../../../thirdparty/spdlog/spdlog/libspdlogd.a ../../../lib/libecal_core.so.5.13.0 ../../../contrib/ecalproto/libecal_proto.a /usr/lib/x86_64-linux-gnu/libprotobuf.so ../../../lib/libecal_core_pb.so.5.13.0 /usr/lib/x86_64-linux-gnu/libprotobuf.so ../../../lib/libecal_app_pb.so.5.13.0 /usr/lib/x86_64-linux-gnu/libprotobuf.so /usr/lib/x86_64-linux-gnu/libprotobuf.so ../../../lib/EcalParser/libEcalParser.a ../../../lib/ecal_utils/libecal_ecal-utils.a 

Later it breaks when linking the binaries:

/usr/bin/ld: CMakeFiles/sys_client.dir/src/ecal_sys_client_cli.cpp.o: in function `eCAL::sys_client::EcalSysClientLogger::EcalSysClientLogger()':
/home/florian/ecal/app/sys/sys_client_core/include/sys_client_core/ecal_sys_client_logger.h:80: undefined reference to `spdlog::get(char const*)'
/usr/bin/ld: /home/florian/ecal/app/sys/sys_client_core/include/sys_client_core/ecal_sys_client_logger.h:96: undefined reference to `spdlog::get(char const*)'

@gabime
Copy link
Owner

gabime commented Apr 29, 2024

If I understand the issue correctly, any C++17 (or later) project that uses spdlog, which was compiled with C++11, will fail. This is indeed problematic and should be addressed in the next release of spdlog.

@tt4g
Copy link
Contributor

tt4g commented Apr 29, 2024

C++ version is a tricky issue.
The C++ standard specification does not guarantee behavior when linking with libraries of different C++ versions.
So, even if spdlog thinks it has a workaround for different C++ versions, the workaround may not work as expected due to compiler incompatibility.

The best workaround I can think of is to use the same C++ version in the project with build tools such as autotools and CMake.

@gabime
Copy link
Owner

gabime commented Apr 29, 2024

The C++ standard specification does not guarantee behavior when linking with libraries of different C++ versions.

Thanks for pointing this out @tt4g , I wasn’t aware of this. But even so, I am thunking about reverting this commit if it causes a problem that didn’t exist before.

@FlorianReimold
Copy link
Author

@gabime Yes, that is the case.

spdlog downgrades itself to C++11 by default, which also isn't the nicest behavior:

spdlog/CMakeLists.txt

Lines 31 to 34 in 238c9ff

elseif(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
endif()

We don't use CMAKE_CXX_STANDARD in our projects but rather set a minimum Cxx standard via target_compile_features. Thus, spdlog downgraded itself to C++11, even though gcc11 defaults to C++17.

@tt4g: at least on gcc and msvc it is quite common to use different cxx standards. As long as the API doesn't use unsupported features, there is no issue with linking a library that was built with a different cxx standard.

@tt4g
Copy link
Contributor

tt4g commented Apr 29, 2024

@FlorianReimold In fact, I have never encountered the problem of C++ standard features being broken by different C++ versions.
However, there have been cases of ABI breakage due to the migration from libstdc++ to libstdc++11 to comply with the C++11 standard.
Developers must understand that STL can also be added and deprecated by C++ versions, so the memory layout of STL objects can be changed and broken.

@yurivict
Copy link

Please make a release that corrects this problem.
Many depending ports might be affected.

@tt4g
Copy link
Contributor

tt4g commented Apr 30, 2024

@yurivict Reverted 37b8476

@gabime
Copy link
Owner

gabime commented Apr 30, 2024

Fixed in version 1.14.1

@gabime gabime closed this as completed Apr 30, 2024
@KerstinKeller
Copy link

In general, you need to make sure that at both build and consumption time, the user sees the same features.
If you still want to integrate this, I would recommend to introduce a define SPDLOG_USE_STRING_VIEW and guard the availability of the functions with this define:

#if SPDLOG_USE_STRING_VIEW 

#endif

In CMake you can then do something like

if () # tbd, check the active cxx standard
set(SPDLOG_USE_STRING_VIEW 1)
else()
set(SPDLOG_USE_STRING_VIEW 0)
endif()

target_compile_definition(spdlog PUBLIC SPDLOG_USE_STRING_VIEW=${SPDLOG_USE_STRING_VIEW})

kraj pushed a commit to YoeDistro/meta-openembedded that referenced this issue May 4, 2024
Fixed compatibility issue between c++17 and c++11 (#3081).
Fixed creating folders under windows when full path with drive letter is given (#3079).
Added mdc sample. Note: mdc is not supported in async loggers.

* v1.14.0 introduced in:
  https://git.openembedded.org/meta-openembedded/commit/?id=a40b1fa392d20ebb29e38374d0f7ce1494bb797f
  causes:
  undefined reference to `spdlog::get(char const*)'
  in various consumers, more details in:
  gabime/spdlog#3081

Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
daregit pushed a commit to daregit/yocto-combined that referenced this issue May 22, 2024
Fixed compatibility issue between c++17 and c++11 (#3081).
Fixed creating folders under windows when full path with drive letter is given (#3079).
Added mdc sample. Note: mdc is not supported in async loggers.

* v1.14.0 introduced in:
  https://git.openembedded.org/meta-openembedded/commit/?id=a40b1fa392d20ebb29e38374d0f7ce1494bb797f
  causes:
  undefined reference to `spdlog::get(char const*)'
  in various consumers, more details in:
  gabime/spdlog#3081

Signed-off-by: Martin Jansa <martin.jansa@gmail.com>
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants