DigitalInBlue / Celero

C++ Benchmark Authoring Library/Framework
Other
824 stars 95 forks source link

Problems with building / packaging celero #163

Closed KerstinKeller closed 1 year ago

KerstinKeller commented 2 years ago

Hi thanks for this project!

I am trying to do a CMake build for an integration of Celero. I came across a few issues:

MT / MD Visual Studio

First question: why are you forcing a static linkage VS runtime linkage: https://github.com/DigitalInBlue/Celero/blob/9d0b1e8d77dd44b385591612a486e668a404d7b2/CMakeLists.txt#L46 ?

The CMake default is always MD and that has nothing to do with the question if I am building a shared or static library. So I would avoid coupling that to the CELERO_COMPILE_DYNAMIC_LIBRARIES option altogether. It could be a completely separate option for the project in my point of view, and please don't make MT the default.

(actually you can pass it also from outside via CMAKE_MSVC_RUNTIME_LIBRARY and CMP0091, see here which I would recommend a hundred times over forcing it on the user)

Generated Config files

The following line causes the generated celero-config.cmake file not to be relocatable: https://github.com/DigitalInBlue/Celero/blob/9d0b1e8d77dd44b385591612a486e668a404d7b2/CMakeLists.txt#L209-L213

The following code is generated by CMake: image

Instead, you should omit the `CMAKE_INSTALL_PREFIX``

target_include_directories(${PROJECT_NAME} PUBLIC
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
  $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
)

Exporting targets to a namespace

It's CMake best practice to export targets to a namespace. (e.g. to reference the celero library from CMake as celero::celero instead of only celero. It does have some CMake advantages (e.g. it's recognized as a target, and not acidentially mistaken as a linker command). However it might break users who are already consuming the celero library.

Warning / build errors when linking static library:

Lastly, I am also getting these warnings (and respective error messages) when compiling a sample against the build static libraries. (I am building one of the samples

D:\Conan\Fresh\celero\package\96f5122383318fb0b29bd75a76b20a51004d12d4\include\celero/UserDefinedMeasurementCollector.h(43,84): warning C4251: 'celero::UserDefinedMeas
urementCollector::collected': class 'std::unordered_map<std::string,std::shared_ptr<celero::UserDefinedMeasurement>,std::hash<std::string>,std::equal_to<std::string>,std::allocator<std::pai
r<const std::string,std::shared_ptr<celero::UserDefinedMeasurement>>>>' needs to have dll-interface to be used by clients of class 'celero::UserDefinedMeasurementCollector' [D:\test\packages\celero\all\test_package\build\66562db33a92334c1d04620ac86f04a958457d64\Debug\celero_test.vcxproj]

and then a bunch of unresolved external errors

main.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) int __cdecl celero::Random(void)" (__imp_?Random@celero@@YAHXZ) referenced in function "protected: virtual void __cdecl CeleroUserBaseline_DemoToString_Baseline::UserBenchmark(void)" (?UserBenchmark@CeleroUserBaseline_DemoToString_Baseline@@MEAAXXZ) [D:\test\packages\celero\all\test_package\build\66562db33a92334c1d04620ac86f04a958457d64\Debug\celero_test.vcxproj]
main.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: __cdecl celero::Factory::Factory(void)" (__imp_??0Factory@celero@@QEAA@XZ) referenced in function "public : __cdecl celero::GenericFactory<class CeleroUserBaseline_DemoToString_Baseline>::GenericFactory<class CeleroUserBaseline_DemoToString_Baseline>(void)" (??0?$GenericFactory@VCeleroUserBaseline_DemoToString_Baseline@@@celero@@QEAA@XZ) [D:\test\packages\celero\all\test_package\build\66562db33a92334c1d04620ac86f04a958457d64\Debug\celero_test.vcxproj]

Do I need to take explicit care when linking celero as a static library?

I'd be happy to create a PR for the first issues. Please let me know if you are interested.

DigitalInBlue commented 2 years ago

I honestly don't recall why there Celero forces static linkage. I would be happy to take a Pull Request if you have one.

DigitalInBlue commented 1 year ago

Some improvements integrated into v2.8.4. Please provide a pull request if there are other suggestions here! The CMake methodology is a bit old at this point, so updates are welcome!

ferkulat commented 1 year ago

I also had linker errors when compiling and using celeoro v2.8.5 as static library. in my case ( using mingw) I needed to do

if(MINGW)
    target_compile_definitions(${PROJECT_NAME} PRIVATE CELERO_STATIC)
endif(MINGW)

for the executable that links against static celero.

DigitalInBlue commented 1 year ago

Apologies for that. I don't have a MinGW configuration to test with right now.

DigitalInBlue commented 1 year ago

I am closing because the original issue has not been updated in over a year.