sony / nmos-cpp

An NMOS (Networked Media Open Specifications) Registry and Node in C++ (IS-04, IS-05)
Apache License 2.0
139 stars 80 forks source link

Issue building on Windows - seems to be missing bcrypt.lib #170

Closed jwpwh closed 3 years ago

jwpwh commented 3 years ago

When building the latest code with boost 1.75, the following link errors were received when build nmos-cpp-node:

9>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptCloseAlgorithmProvider referenced in function "private: void __cdecl boost::uuids::detail::random_provider_base::destroy(void)" (?destroy@random_provider_base@detail@uuids@boost@@AEAAXXZ)
9>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptGenRandom referenced in function "public: void __cdecl boost::uuids::detail::random_provider_base::get_random_bytes(void *,unsigned __int64)" (?get_random_bytes@random_provider_base@detail@uuids@boost@@QEAAXPEAX_K@Z)
9>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptOpenAlgorithmProvider referenced in function "public: __cdecl boost::uuids::detail::random_provider_base::random_provider_base(void)" (??0random_provider_base@detail@uuids@boost@@QEAA@XZ)

I found the prototype for these functions and it looked like a symbol BOOST_USE_WINDOWS_H could be defined to avoid these functions but that did not work out well. I looked some more and found that a bcrypt.lib needs to be included when linking.

I changed the CMakeLists.txt so that bcrypt.lib is a library for the node and registry:

target_link_libraries(
    nmos-cpp-node
    nmos-cpp_static
    ${CPPRESTSDK_TARGET}
    ${PLATFORM_LIBS}
    ${Boost_LIBRARIES}
    bcrypt.lib
    )

target_link_libraries(
    nmos-cpp-registry
    nmos-cpp_static
    ${CPPRESTSDK_TARGET}
    ${PLATFORM_LIBS}
    ${Boost_LIBRARIES}
    bcrypt.lib
    )

This seems to fix the link errors. (not sure about actual execution yet) Assuming the fix is changed to only affect the windows builds, is this the desired fix? Or is something supposed to include a #pragma to force the library to be included?

Thanks.

garethsb commented 3 years ago

Interesting. The Windows build seems to be successful in CI (but I don't see bcrypt.lib mentioned in the raw logs). On my local environment, I can see bcrypt.lib is being added automatically. Are you using Conan?

jwpwh commented 3 years ago

I have not tried Conan. I assume that will work on Windows and will try it. Will that work or help when building for Raspberry Pi in a cross environment?

garethsb commented 3 years ago

No, I suspect not at the moment... Conan does support cross-compilation, but we haven't attempted to get that working for e.g. Raspberry Pi yet. The Raspberry Pi build hasn't had any love for a while... It'd be great if this could be made to "just work" too, as build instructions tend to rot even faster than code.

But, I will take from your original report to also investigate the non-Conan build on Windows and whether we're missing a dependency on bcrypt.lib in that case. Thanks!

garethsb commented 3 years ago

@jwpwh, I have just done a fresh build on Windows with Visual Studio 2019 (Version 16.8.3), using Boost 1.75.0, cpprestsdk v2.10.18, and then nmos-cpp master with USE_CONAN off. I tried building cpprestsdk both with and without specifying 'asio' for CPPREST_HTTP_(CLIENT|LISTENER)_IMPL. Both builds succeeded and nmos-cpp-node runs without having to add bcrypt.lib. Could you share a bit more about your environment?

jwpwh commented 3 years ago

Here is how I ran CMake for nmos-cpp:

...

cmake.exe .. ^ -G "Visual Studio 16 2019" -A x64^ -DCMAKE_INSTALL_PREFIX:PATH="%INSTALL_DIR_UNDOS%" ^ -DCMAKE_CONFIGURATION_TYPES:STRING="Debug;Release" ^ -DBoost_USE_STATIC_LIBS:BOOL="1" ^ -DBOOST_INCLUDEDIR:PATH="%BOOST_INC%" ^ -DBOOST_LIBRARYDIR:PATH="%BOOST_LIB%" ^ -Dcpprestsdk_DIR:PATH="%CPPRESTSDK_CMAKE%" ^ -DUSE_CONAN:BOOL="0" ^ -DWEBSOCKETPP_INCLUDE_DIR:PATH="%CPPRESTSDK_HOME%/cpprestsdk/Release/libs/websocketpp" ...

Where:

When I make my application, I was using a different approach where the CMakeLists.txt file for the application was providing the needed paths and arguments. The above command is what I use to test a new version of nmos-cpp. I hadn't done this in a while.

Other dependencies:

... cd build.win64 "e:\Program Files\Cmake\bin\cmake" .. ^ -G "Visual Studio 16 2019" -A x64^ -DCMAKE_INSTALL_PREFIX=%NMOS_LIBS% ^ -DCPPREST_PPLX_IMPL:STRING="winpplx" ^ -DCPPREST_EXCLUDE_COMPRESSION=1 ^ -DCMAKE_CONFIGURATION_TYPES:STRING="Debug;Release" ^ -DCPPREST_HTTP_LISTENER_IMPL:STRING="asio" ^ -DCPPREST_HTTP_CLIENT_IMPL:STRING="asio" ^ -DBoost_USE_STATIC_LIBS:BOOL="1" ^ -DBOOST_INCLUDEDIR:PATH="%BOOST_INC%" ^ -DBOOST_LIBRARYDIR:PATH="%BOOST_LIB%" ^ -DOPENSSL_ROOT_DIR:PATH="%OPENSSL_INSTALL%" ^ -DWERROR=0 ^ -DBUILD_SAMPLES:BOOL="0" ^ -DBUILD_TESTS:BOOL="0" ...

Where:

garethsb commented 3 years ago

I am using the v2.10.16 version. I chose this version because the dependencies.md file indicated this was the version. When I used the conan method the other day, I noticed that it was using 17. Now you are using 18. Hard to keep up.

I agree, it's really hard to keep up with OSS dependencies! I only spotted cpprestsdk v2.10.18 had been tagged earlier today, so I thought I'd try it out. I attempt to keep most of the GitHub CI builds of nmos-cpp close to the bleeding edge, so that we all get notification of issues with current versions sooner rather than later. Sticking with earlier versions in production is completely reasonable, we attempt to maintain compatibility as long as possible. To that end, the CI also includes a non-Conan build on ancient Ubuntu 14.04/gcc 4.8 with cpprestsdk v2.10.16 (back as far as v2.10.11 ought to be OK) using the system Boost 1.54.0.

This shouldn't make a difference, they've made minimal changes

garethsb commented 3 years ago

I've checked my CMake options and they're almost identical to yours (just different install paths). Perplexing.

jwpwh commented 3 years ago

In the conan files for boost and cpprestsdk there are references to bcyrpt as shown below:

.../.conan/data/boost/1.72.0/_/_/export/conanfile.py:                self.cpp_info.components["_libboost"].system_libs.append("bcrypt")
.../.conan/data/boost/1.75.0/_/_/export/conanfile.py:                self.cpp_info.components["_libboost"].system_libs.append("bcrypt")
.../.conan/data/cpprestsdk/2.10.16/_/_/export/conanfile.py:            self.cpp_info.components["cpprest"].system_libs.append("bcrypt")
.../.conan/data/cpprestsdk/2.10.17/_/_/export/conanfile.py:            self.cpp_info.components["cpprest"].system_libs.append("bcrypt")

I am not sure how these export files are used but it seems like they are adding bcrypt to the system libs which is exactly what seems to be needed for the non-conan builds. I think I tried this on not-windows and it seemed to work ok there also.

I have tried a modified CMakeLists.txt with changes to add bcrypt on windows. The key lines are the definition and use of NMOS_CPP_SYSTEM_LIBS

if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
    set (NMOS_CPP_SYSTEM_LIBS bcrypt.lib)
else()
    set (NMOS_CPP_SYSTEM_LIBS )
endif()

source_group("Source Files" FILES ${NMOS_CPP_NODE_SOURCES})
source_group("Header Files" FILES ${NMOS_CPP_NODE_HEADERS})

target_link_libraries(
    nmos-cpp-node
    nmos-cpp_static
    ${CPPRESTSDK_TARGET}
    ${PLATFORM_LIBS}
    ${Boost_LIBRARIES}
    ${NMOS_CPP_SYSTEM_LIBS}
    )
garethsb commented 3 years ago

Thanks, I'll investigate.

garethsb commented 3 years ago

I think cpprestsdk has always had a private dependency on bcrypt which shouldn't affect us. Boost.Filesystem and Boost.UUID are the two Boost libs which currently utilise bcrypt. We don't use Boost.Filesystem on Windows, only on older Linux / gcc platforms, so I suspected our use of Boost.UUID is the culprit. Its dependency on bcrypt arrived in Boost 1.67.0 though, which we certainly built with before we introduced Conan. I'll try again to repro the issue locally.

jwpwh commented 3 years ago

The change suggested to the target_link_libraries for nmos-cpp-node would also need to be made for nmos-cpp-registry.

I suspect you are right about the Boost.UUID. I seem to recall that the error messages when bcrypt.lib was not included were about symbols in modules related to this. Seems odd that boost does not include the dependency.

garethsb commented 3 years ago

The thing is that Boost.UUID does add the auto-linking#pragma comment(lib, bcrypt.lib)... see https://github.com/boostorg/uuid/blob/master/include/boost/uuid/detail/random_provider_bcrypt.ipp#L20-L29.

When I added /VERBOSE to the Linker settings for nmos-cpp-node, I see:

2>Searching libraries
2>    Searching Debug\nmos-cpp_static.lib:
2>      Found "public: __cdecl nmos::id_generator::id_generator(void)" (??0id_generator@nmos@@QEAA@XZ)
2>        Referenced in main.obj
2>        Loaded nmos-cpp_static.lib(id.obj)
2>Processed /DEFAULTLIB:bcrypt.lib

... and later...

2>    Searching C:\Program Files (x86)\Windows Kits\10\lib\10.0.18362.0\um\x64\bcrypt.lib:
2>      Found BCryptCloseAlgorithmProvider
2>        Referenced in nmos-cpp_static.lib(id.obj)
2>        Loaded bcrypt.lib(bcrypt.dll)
2>      Found BCryptGenRandom
2>        Referenced in nmos-cpp_static.lib(id.obj)
2>        Loaded bcrypt.lib(bcrypt.dll)
2>      Found BCryptOpenAlgorithmProvider
2>        Referenced in nmos-cpp_static.lib(id.obj)
2>        Loaded bcrypt.lib(bcrypt.dll)

...

jwpwh commented 3 years ago

I think I have discovered my problem. The path I was using to the BOOST include directory in CMAKE was not quite right. The path used a "_" where a "-" should have been used. With the wrong path, cmake (which was coming from what is provided with VS 2019) was using a different version of the boost cmake files and ended up with BOOST_ALL_NO_LIB defined which made it not do the request for the bcrypt.lib.

When the correct path is used, then BOOST_ALL_NO_LIB is not defined and then everything is ok.

Sorry for the inconvenience and thanks for your patience.

garethsb commented 3 years ago

Phew, that explains it! Thanks for sharing, I've encountered similar before, so I'm sure this will help someone else in the future.

jwpwh commented 3 years ago

This is more complicated than I thought.

I was looking into this further since I was seeing varied results depending on where I called cmake. (script vs VS2019 command shell...)

The warning error looked like this:

CMake Warning at E:/Program Files (x86)/Microsoft Visual Studio/2019/Professional/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.17/Modules/FindBoost.cmake:1164 (message):
  New Boost version may have incorrect or missing dependencies and imported
  targets

So the auto-link seems to be turned off on purpose and Cmake is eager to find anything on your system which might be helpful. I have found a couple of things which might help.

# since auto-link for uuid in boost gets turned off add_definitions(/DBOOST_UUID_FORCE_AUTO_LINK) ``` * It also seems possible that adding Boost::boost or maybe even Boost::headers as a TARGET might avoid the addition of the BOOST_ALL_NO_LIB symbol. See https://cmake.org/cmake/help/v3.20/module/FindBoost.html. I did not try this and don't know if it works. The cmake file which adds BOOST_ALL_NO_LIB does have a line at the top which seems like it would return if Boost::headers was a target.
garethsb commented 3 years ago

I'm still confused at the moment.

When using Conan, we're explicitly specifying "Config" mode for find_package, but when not using Conan it should use the find_package "Module" mode and only fall back to "Config" mode if that fails. Since we specify BOOST_INCLUDEDIR and BOOST_LIBRARYDIR I expect module mode to be used, and none of the Boost config files to apply?

Perhaps more importantly we add ${Boost_LIBRARIES} to target_link_libraries not the Boost::headers target or Boost::<component> targets, so I thought those target properties wouldn't be applied anyway?

jwpwh commented 3 years ago

I am also confused. I don't understand all the ways Cmake uses for finding packages and dependencies.

It seems like nmos-cpp is looking for boost last and I was wondering if it should ask for boost first. By looking for cpprestsdk before boost, it seems like cpprestsdk looks for boost and once Cmake finds a boost, then that is the version to be used. I am not sure the BOOST_INCLUDEDIR and such are used on all methods of looking for boost so maybe cpprestsdk finds boost in a different way. When installing the dependencies, boost is first, so why not look for it first?

My goal in this is to build the nmos-cpp and install the libraries and headers so I can use those in my application. I also want to build the registry executable so it can be used. When building my application, I am not using cmake.

I have run Cmake with debug output and tracing on and am surprised how much things change apparently because of minor differences in the environment. I am also surprised when paths on my system show up in the debug output that I haven't pointed it to. Cmake seems very eager to find packages and it seems mostly out of my control. I find this frustrating and a bit scary.

When you ran the builds without conan, were you on a clean machine or is there a chance that Cmake found and used something already on your machine which may not have been exactly what you thought it was going to use?

If Cmake actually finds the boost cmake snippets generated by boost and pointed to by the BOOST_LIBRARYDIR it seems to include at least the boost_headers-config.cmake snippet. All the snippets define BOOST_ALL_NO_LIB so including any of them could cause problems. I am guessing that the strategy for the auto-link references is that when building a boost library, auto-linking would be done. When building the applications, it would not. This would work great except for using a headers-only module. (which is what nmos-cpp is doing with uuid). By adding the target Boost::headers then the BOOST_ALL_NO_LIB is avoided and the auto-linking is done. Without the target, then the cmake snippet turns it off. I can't seem to control how the boost cmake snippets are brought in so I can't control when BOOST_ALL_NO_LIB is defined and when it isn't. But, defining BOOST_UUID_FORCE_AUTO_LINK works regardless of which boost cmake snippets Cmake happens to find and use. So that is the route I am taking.

garethsb commented 3 years ago

OK, so I just reproduced this, I think... Specifically I'm seeing:

12>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptCloseAlgorithmProvider referenced in function "private: void __cdecl boost::uuids::detail::random_provider_base::destroy(void)" (?destroy@random_provider_base@detail@uuids@boost@@AEAAXXZ)
12>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptGenRandom referenced in function "public: void __cdecl boost::uuids::detail::random_provider_base::get_random_bytes(void *,unsigned __int64)" (?get_random_bytes@random_provider_base@detail@uuids@boost@@QEAAXPEAX_K@Z)
12>nmos-cpp_static.lib(id.obj) : error LNK2019: unresolved external symbol BCryptOpenAlgorithmProvider referenced in function "public: __cdecl boost::uuids::detail::random_provider_base::random_provider_base(void)" (??0random_provider_base@detail@uuids@boost@@QEAA@XZ)

These seem to match your recollection.

I have USE_CONAN turned off. The difference from what you've reported is that I was explicitly trying out using Boost_DIR to specify the location of the BoostConfig.cmake file created by b2 (which for me it has installed in ...\boost_1_76_0\x64\lib\cmake\Boost-1.76.0). When I remove that CMake variable and go back to specifying BOOST_INCLUDEDIR (as ...\boost_1_76_0) and BOOST_LIBRARYDIR (as ...\boost_1_76_0\x64\lib) instead, the build works fine again.

I can swap between these repeatably. The difference is as you reported that BOOST_ ALL_NO_LIB is included in the preprocessor settings for nmos-cpp_static. On adding /VERBOSE to the linker settings for the apps, in the working case the log has Processed /DEFAULTLIB:bcrypt.lib immediately after loading nmos-cpp_static.lib(id.obj) which it doesn't in the other.

This is indeed down to the different way that the Boost libraries are "linked" (== "declared as dependencies" in modern CMake). In the non-working case, Boost_LIBRARIES contains targets (i.e. Boost::system;Boost::date_time;Boost::regex;Boost::thread) rather than paths. This means that BOOST_ALL_NO_LIB is picked up from any of their -config.cmake files which all have it in their targets' INTERFACE_COMPILE_DEFINITIONS as a result of the boost-install rule.

This seems to be different from CMake's own FindBoost.cmake (see here) which requires the user to "link" to Boost::disable_autolink to get this behaviour.

I can't find an explanation of why boost-install forces BOOST_ALL_NO_LIB. There's a great comment in Boost.UUID however (here):

      # boost.jam defines BOOST_ALL_NO_LIB, this makes library management
      # near-impossible with the platform selection logic in the random_provider
      <target-os>windows:<define>BOOST_UUID_FORCE_AUTO_LINK
      <target-os>windows,<toolset>gcc:<library>bcrypt

So in conclusion, it looks like specifying BOOST_UUID_FORCE_AUTO_LINK would be a reasonable solution...

FWIW, I got here because I'm attempting to rewrite the CMake build to make it much easier to use nmos-cpp as a library...