search

ros / console_bridge

A ROS-independent package for logging that seamlessly pipes into rosconsole/rosout for ROS-dependent packages.
BSD 3-Clause "New" or "Revised" License
22 stars 62 forks source link

Fix static compilation on Windows #40

Closed ibadr closed 7 years ago

ibadr commented 8 years ago

Currently, compiling as a static library by setting BUILD_SHARED_LIBS=OFF fails on Windows, and the reason is that CONSOLE_BRIDGE_STATIC is not defined as it's supposed to in that case. A simple fix is included here.

The log for the build failure is below

d:\code\console_bridge\build>nmake

Microsoft (R) Program Maintenance Utility Version 12.00.21005.1
Copyright (C) Microsoft Corporation.  All rights reserved.

Scanning dependencies of target console_bridge
[ 12%] Building CXX object CMakeFiles/console_bridge.dir/src/console.cpp.obj
console.cpp
D:\code\console_bridge\src\console.cpp(82) : warning C4273: 'console_bridge::noOutputHandler' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(175) : see previous definition of 'noOutputHandler'
D:\code\console_bridge\src\console.cpp(89) : warning C4273: 'console_bridge::restorePreviousOutputHandler' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(178) : see previous definition of 'restorePreviousOutputHandler'
D:\code\console_bridge\src\console.cpp(95) : warning C4273: 'console_bridge::useOutputHandler' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(181) : see previous definition of 'useOutputHandler'
D:\code\console_bridge\src\console.cpp(102) : warning C4273: 'console_bridge::getOutputHandler' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(184) : see previous definition of 'getOutputHandler'
D:\code\console_bridge\src\console.cpp(108) : warning C4273: 'console_bridge::log_deprecated' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(202) : see previous definition of 'log_deprecated'
D:\code\console_bridge\src\console.cpp(133) : warning C4273: 'console_bridge::log' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(197) : see previous definition of 'log'
D:\code\console_bridge\src\console.cpp(153) : warning C4273: 'console_bridge::setLogLevel' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(188) : see previous definition of 'setLogLevel'
D:\code\console_bridge\src\console.cpp(159) : warning C4273: 'console_bridge::getLogLevel' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(192) : see previous definition of 'getLogLevel'
D:\code\console_bridge\src\console.cpp(167) : warning C4273: 'console_bridge::OutputHandlerSTD::log' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(151) : see previous definition of 'log'
D:\code\console_bridge\src\console.cpp(181) : warning C4273: 'console_bridge::OutputHandlerFile::OutputHandlerFile' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(161) : see previous definition of '{ctor}'
D:\code\console_bridge\src\console.cpp(194) : warning C4273: 'console_bridge::OutputHandlerFile::~OutputHandlerFile' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(163) : see previous definition of '{dtor}'
D:\code\console_bridge\src\console.cpp(201) : warning C4273: 'console_bridge::OutputHandlerFile::log' : inconsistent dll linkage
        D:\code\console_bridge\include\console_bridge/console.h(165) : see previous definition of 'log'
[ 25%] Linking CXX static library lib\console_bridge.lib
[ 25%] Built target console_bridge
Scanning dependencies of target gtest
[ 37%] Building CXX object test/CMakeFiles/gtest.dir/gtest/src/gtest-all.cc.obj
gtest-all.cc
[ 50%] Linking CXX static library ..\lib\gtest.lib
[ 50%] Built target gtest
Scanning dependencies of target gtest_main
[ 62%] Building CXX object test/CMakeFiles/gtest_main.dir/gtest/src/gtest_main.cc.obj
gtest_main.cc
[ 75%] Linking CXX static library ..\lib\gtest_main.lib
[ 75%] Built target gtest_main
Scanning dependencies of target console_TEST
[ 87%] Building CXX object test/CMakeFiles/console_TEST.dir/console_TEST.cc.obj
console_TEST.cc
[100%] Linking CXX executable ..\bin\console_TEST.exe
console_TEST.cc.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) void __cdecl console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)" (__imp_?log_deprecated@console_bridge@@YAXPBDHW4LogLevel@1@0ZZ) referenced in function "private: virtual void __thiscall ConsoleTest_MacroExpansionTest_ItShouldCompile_Test::TestBody(void)" (?TestBody@ConsoleTest_MacroExpansionTest_ItShouldCompile_Test@@EAEXXZ)
..\bin\console_TEST.exe : fatal error LNK1120: 1 unresolved externals
LINK failed. with 1120
NMAKE : fatal error U1077: '"C:\Program Files (x86)\CMake\bin\cmake.exe"' : return code '0xffffffff'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\BIN\nmake.exe"' : return code '0x2'
Stop.
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\BIN\nmake.exe"' : return code '0x2'
Stop.
scpeters commented 8 years ago

+1. urdfdom probably needs the same fix

ibadr commented 8 years ago

It looks like this fix could needed throughout all ROS projects that link against console_bridge. So far, I got the following errors while statically linking rosbag_storage, cpp_common, and tf2 in a MEX project:

Error using mex
   Creating library rosbag_wrapper.lib and object rosbag_wrapper.exp
tf2.lib(buffer_core.cpp.obj) : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl
console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)"
(__imp_?log_deprecated@console_bridge@@YAXPEBDHW4LogLevel@1@0ZZ)
rosbag_storage.lib(bag.cpp.obj) : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl
console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)"
(__imp_?log_deprecated@console_bridge@@YAXPEBDHW4LogLevel@1@0ZZ)
rosbag_storage.lib(bz2_stream.cpp.obj) : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl
console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)"
(__imp_?log_deprecated@console_bridge@@YAXPEBDHW4LogLevel@1@0ZZ)
rosbag_storage.lib(lz4_stream.cpp.obj) : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl
console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)"
(__imp_?log_deprecated@console_bridge@@YAXPEBDHW4LogLevel@1@0ZZ)
cpp_common.lib(header.cpp.obj) : error LNK2001: unresolved external symbol "__declspec(dllimport) void __cdecl
console_bridge::log_deprecated(char const *,int,enum console_bridge::LogLevel,char const *,...)"
(__imp_?log_deprecated@console_bridge@@YAXPEBDHW4LogLevel@1@0ZZ)
rosbag_wrapper.mexw64 : fatal error LNK1120: 1 unresolved externals

I think what is going on here is that, because CONSOLE_BRIDGE_STATIC is not defined in the CMakeLists.txt of these projects, the included console_bridge/exportdecl.h via console_bridge/console.h defines CONSOLE_BRIDGE_DLLAPI to be CONSOLE_BRIDGE_DLLIMPORT, where the latter is defined as __declspec(dllimport) on Windows.

The fix --at least conceptually-- is still the same. However, it needs to be applied to CMakeLists.txt in all ROS projects that depend on console_bridge. (Or maybe implement the fix in catkin?)

EDIT: I can confirm this. I implemented the same fix in CMakeLists.txt of rosbag_storage, cpp_common, and tf2, and all the linker errors disappeared, and the MATLAB MEX file is compiling fine.

Building with 'Microsoft Visual C++ 2013 Professional'.
MEX completed successfully.
scpeters commented 8 years ago

@jacquelinekay who from the ROS team could review this?

traversaro commented 7 years ago

I know that this is a old PR, but given that I am fighting quite a bit with this kind of issues while working on vcpkg ports, I think I can comment on this.

The fix --at least conceptually-- is still the same. However, it needs to be applied to CMakeLists.txt in all ROS projects that depend on console_bridge. (Or maybe implement the fix in catkin?)

This is not scalable, even because it is totally legitimate to link a shared library to a "static" console_bridge. What is necessary is that console_bridge in some way exports this option to the consumer projects. One possible way is to use some CMake machinary such as exporting a console_bridge_FLAGS or a defining a PUBLIC option in target_compile_options, but I found this quite error prone because it would not work for all users of console_bridge that do not use CMake.

In my little experience the most robust solution is to generate (at configure time) an header (tipically called <package>-config.h or a similar name) that defines or not CONSOLE_BRIDGE_STATIC depending on the BUILD_SHARED_LIBS option. This can be done by using the GenerateExportHeader CMake module.

An alternative solution for simple cases such as this one is to keep two headers in the source tree (one defining CONSOLE_BRIDGE_STATIC and the other not), and include and install one or another depending on BUILD_SHARED_LIBS. An example of this easier solution can be found in the freeimage port in vcpkg : https://github.com/Microsoft/vcpkg/blob/5bd036cf5a70e18c451cc5aa9d041c530bf7adf1/ports/freeimage/CMakeLists.txt#L97 .

j-rivero commented 7 years ago

In my little experience the most robust solution is to generate (at configure time) an header (tipically called -config.h or a similar name) that defines or not CONSOLE_BRIDGE_STATIC depending on the BUILD_SHARED_LIBS option. This can be done by using the GenerateExportHeader CMake module.

In #43

scpeters commented 7 years ago

Is this needed anymore since #43 was merged?

scpeters commented 7 years ago

I'm going to close this since I think it is not needed since #43 was merged. Please re-open if that is not the case.