Improve cmake build to find openssl and fix windows build #2120
Conversation
4712a87 to
d9d82a8
Compare
|
this looks good, there will be a small related change today or tomorrow - i have moved prebuilt binaries dirs to |
|
@aleks-f Looks good ... unfortunately not really. Find openssl package modul from cmake expect following from openssl:
In following directories cmake searches for openssl:
I see that now the libraries are in openssl/build/win32/lib/debug/ with the names: |
|
Ok sorry, I found the answer ... |
|
@Bjoe our openssl directory has changed (I committed this morning, develop branch, master not yet) -everything is under As for external OpenSSL, we should not guess anything there - if user chooses external openssl, he is entirely on his own and should provide the proper location - it is a perilous process though because names on windows are all over the place - I suggest not to waste your time trying to guess what an external openssl binaries names may be :) |
CMakeLists.txt
Outdated
| @@ -92,11 +92,10 @@ option(DISABLE_INTERNAL_OPENSSL "Disable internal OpensSSL binaries use" OFF) | |||
| set(USING_INTERNAL_OPENSSL false) | |||
|
|
|||
| if(MSVC) | |||
| if((EXISTS "${PROJECT_SOURCE_DIR}\\openssl\\VS_120\\") AND NOT DISABLE_INTERNAL_OPENSSL) | |||
| if((EXISTS "${PROJECT_SOURCE_DIR}/openssl/VS_120/") AND NOT DISABLE_INTERNAL_OPENSSL) | |||
There was a problem hiding this comment.
Yes I know ... I merged now the latest develop into my branch and changed the path in CMakeLists.txt.
|
@Bjoe just FYI (in case you are wondering where are they), for our internal build openssl binary names are not needed anywhere in cmake because they are linked automatically. cmake only has to make sure everything from /openssl/build ends up in the right place in lib(64)/bin(64) directories used by cmake |
|
@aleks-f Hm I see. So we need only to set the path?
in Crypto/CMakeLists.txt. OpenSSL::SSL and OpenSSL::Crypto set all the required include-, libraries- paths and compile definitions for the local build system also for the external usage/linking of libPocoCrypto.lib. Btw. I see that this definitions POCO_EXTERNAL_OPENSSL should also be exported. I will fix this... |
no, it will statically link directly into any executable that includes Crypto.h; static libCrypto.lib binary will contain only barebone Crypto library code
correct |
|
Ok, libCrypto.lib means "static" on windows ... .lib on windows is also like .a on Unix/Linux? Hm Appveyor is not ready yet, but I have a test result from my test. Unfortunately its the same problem.... the openssl libraries are not found in openssl/build/lib/libcrypto32MTd.lib ... See also the test result |
it's different on windows - there is always a .lib file, for static and dynamic builds - for static build, the .lib file contains all the code, but for dynamic builds it is only a stub for linking from an executable (to resolve all linking at build time); at runtime, the stub code automatically loads DLL (equivalent of .so on linux), where the library actual code is. besides the name suffix MT, you can also distinguish between static and dynamic .lib files by size - the dynamic stub .lib is significantly smaller than the static one |
|
here's the full tree, |
|
Hi, cmake is now fixed and found the right openssl libraries:
I see also the openssl libraries as parameters for the linker:
But the linker is failing with following error:
Why they search for libcryptod.lib ? poco/Crypto/include/Poco/Crypto/Crypto.h Lines 106 to 107 in 4a7ab78 |
yes, and that should only be enabled when POCO_INTERNAL_OPENSSL_MSVC_VER is defined; that define in opensslv.h is inserted by our openssl build, so POCO_INTERNAL_OPENSSL_MSVC_VER is defined automatically if internal opensslv.h is included (which should not happen for external OpenSSL and there is a compile time warning if that header is included when POCO_EXTERNAL_OPENSSL is defined). in other words, when compiling POCO_EXTERNAL_OPENSSL, two conditions should be met
|
|
Also, when linking with external openssl, linking must be explicit because those pragmas are disabled - we do not know what the library names will be for an external lib |
|
Ok. I see following compiler output:
During the compile output I see:
See also appveyor build line 1522 ... This means for me POCO_INTERNAL_OPENSSL_MSVC_VER is defined because of \projects\poco\openssl\build\include as include path. POCO_EXTERNAL_OPENSSL is not set because of DISABLE_INTERNAL_OPENSSL is not set as cmake parameter and \projects\poco\openssl\build\ exists as directory. Also I didn't see any warnings like: poco/Crypto/include/Poco/Crypto/Crypto.h Line 89 in 8974a0e For me it looks like _DLL is defined because this if statement is not getting true: poco/Crypto/include/Poco/Crypto/Crypto.h Line 100 in 8974a0e and the linker is claiming about cannot open file 'libcryptod.lib' and not libcryptomtd.lib ... 'mt' is missing. Who defines _DLL ? |
|
See VisualStudio Predefined macros . _DLL is defined by the cl.exe compiler when the linked runtime is conditionned by the /MD or /MDd flags |
|
@Bjoe, given all you stated, it sounds like the path to openssl libraries (this, of course, is different from header path, which is obviously known to the compiler) is not specified, so linker can not find the libraries to link with |
3c4d849 to
795e496
Compare
|
@zosrothko Thanks, now I understand.
and this is also set as parameter for the linker:
With zosrothko hint, I understand now why _DLL is set. This means for me, we should use static openssl libraries when poco are build as static libraries. When poco is build as shared libraries then we should use the shared openssl libraries. This is what are defined in Crypto.h poco/Crypto/include/Poco/Crypto/Crypto.h Line 100 in 8974a0e May I ask, why we have these "static build dependencies" ? |
for convenience - otherwise you'd have to explicitly specify the library you're linking with. This way (if you're using poco prebuilt openssl), all you have to do is specify the openssl headers location and linking happens automatically through headers. as you can see, these dependencies are only in effect when our openssl header is included, otherwise, they are disabled |
8c3e680 to
83fdfe2
Compare
|
It compile and link now but unfortunately tests are failing with exception code 0xc0000135.
What is now the problem? |
|
most likely missing dll. the location where the DLL binary is must be in the PATH |
83fdfe2 to
5dfce53
Compare
5dfce53 to
a331a7d
Compare
|
Ok, test are fixed, I squashed and rebase to latest develop branch. So I'm done, please review. |
|
@zosrothko, where do you see this?
|
| #pragma comment(lib, "libcrypto.lib") | ||
| #pragma comment(lib, "libssl.lib") | ||
| #endif // POCO_EXTERNAL_OPENSSL | ||
| #if defined(POCO_INTERNAL_OPENSSL_MSVC_VER) |
There was a problem hiding this comment.
Is "this change" from line 64 to 94 correct? I do not have much windows knowledge. I copied from Crypto.h.
There was a problem hiding this comment.
yes, Crypto.h does all the work.
POCO_INTERNAL_OPENSSL_MSVC_VER is a new define that comes "automatically" from the define injected by poco internal openssl build.
we issue a compile-time warning if user has defined external ssl, but is using internal headers. if user has not specified anything, internal build is used by default.
There was a problem hiding this comment.
Good to know: we also base some hacking (in order to prevent static linking errors) on that define
|
@zosrothko sorry, I didn't get it too. What you mean with:
The only think what I see, I remove:
and I seen that this copied the openssl libs to the place where the poco libs are exist. |
|
@zosrothko Ok think I get it now, I see in Foundation/testsuite/CMakeLists.txt following lines: poco/Foundation/testsuite/CMakeLists.txt Line 48 in 19a1c52 poco/Foundation/testsuite/CMakeLists.txt Line 56 in 19a1c52 You mean this lines? |
2a08756 to
904e332
Compare
904e332 to
eb63890
Compare
Use features from cmake to find and set proper build settings for openssl.