(0039506)
Brad King (manager)
2015-10-05 11:06
edited on: 2015-10-05 11:06

Thanks for working on this.

The dll-first problem is more general than just FindOpenSSL so let's see if we can solve that directly. Does this patch work for your case?

diff --git a/Modules/Platform/Windows-GNU.cmake b/Modules/Platform/Windows-GNU.cmake
index d8a423e..b4fc4aa 100644
--- a/Modules/Platform/Windows-GNU.cmake
+++ b/Modules/Platform/Windows-GNU.cmake
@@ -35,7 +35,7 @@ endif()

if(MINGW)
   set(CMAKE_FIND_LIBRARY_PREFIXES "lib" "")
- set(CMAKE_FIND_LIBRARY_SUFFIXES ".dll" ".dll.a" ".a" ".lib")
+ set(CMAKE_FIND_LIBRARY_SUFFIXES ".dll.a" ".a" ".lib" ".dll")
   set(CMAKE_C_STANDARD_LIBRARIES_INIT "-lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32")
   set(CMAKE_CXX_STANDARD_LIBRARIES_INIT "${CMAKE_C_STANDARD_LIBRARIES_INIT}")
endif()

(0039507)
Wayne Stambaugh (reporter)
2015-10-05 11:59

Your patch does not fix the issue. There may be several issues at play here. I'm not sure about the path search order CMake uses in find_library() but I can tell you that building and installing OpenSSL from source on mingw results in libraries named libcrypto.dll and libssl.dll not libeay32.dll and libssleay32.dll. This may be some legacy output naming to prevent conflicts on windows but it not longer appears to be the case. If you look at my patch, you will notice that I moved the search for the crypto and ssl libraries out of the cross build test. If memory serves, I also had to remove the system paths for find_library() to prevent the libraries in the system path being found. This is what confused me. I would expect that when I defined OPENSSL_ROOT_DIR that it would take precedence over all other search paths. Perhaps this is an invalid assumption on my part.

(0039508)
Brad King (manager)
2015-10-05 12:41

Sorry, I meant the patch in 0015765:0039506 as a replacement for just the hunks that look like this:

+ # Do not search system path. Otherwise the DLL will be found rather than the link library.
+ NO_SYSTEM_ENVIRONMENT_PATH
+ NO_CMAKE_SYSTEM_PATH

Does removing those hunks and adding my patch still work?

(0039509)
Wayne Stambaugh (reporter)
2015-10-05 12:49
edited on: 2015-10-05 12:52

I commented out NO_SYSTEM_ENVIRONMENT_PATH and NO_CMAKE_SYSTEM_PATH in my custom FindOpenSSL.cmake and used your patch but no luck. It still found the libraries in the windows system path.

(0039516)
Brad King (manager)
2015-10-06 11:42

Re 0015765:0039509: Thanks for trying that. I realized that the suffix ordering only matters within a single directory, not over the whole search path, so the patch in 0015765:0039506 has no chance of working.

(0039517)
Brad King (manager)
2015-10-06 11:59

I split out the version parsing fix into its own commit:

FindOpenSSL: Tolerate tabs in header while parsing version
https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=6b575dec [^]

Then I made another change based on your main MinGW-specific hunk:

FindOpenSSL: Search for unix-named libraries first on MinGW
https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1bf66fed [^]

By searching for "crypto" and "ssl" first this may avoid finding the DLLs. Please try it.

(0039520)
Wayne Stambaugh (reporter)
2015-10-06 14:05

I applied both patches to the CMake version 3.3.2 of FindOpenSSL.cmake and it solved the problem. The $10K question is what happens if at some point in the future the openssl project changes the name for the native windows builds to libsll and libcrypto instead of libealy32 and ssleay32. We would be right back to where we are now. While this fixes this single problem, I have a much larger concern that find_library() does not honor the user's request for a specific library search path when it is defined. The path search order should take precedence over the library name search order. It appears that the library path search loop is inside the library name search loop instead of the other way around. I may have to rethink some of my custom find cmake modules.

(0039521)
Brad King (manager)
2015-10-06 14:31

Re 0015765:0039520: Try the NAMES_PER_DIR option to find_library to flip the loop order.

The full find_library search order is documented here:

https://cmake.org/cmake/help/v3.4/command/find_library.html [^]

(0039522)
Wayne Stambaugh (reporter)
2015-10-06 14:51

I can't believe as many times as I've looked at the find_library() documentation I never noticed that option. I need to slow down and read the documentation more carefully. Thanks for the tip.

(0039551)
Brad King (manager)
2015-10-08 14:09

Re 0015765:0039520: Even if the native Windows OpenSSL changes to the ssl/crypto names this should still work because the HINTS will precede the PATH anyway. It was failing before this fix only due to preferring the eay32 names first regardless of directory. Now we try the eay32 names last.

As a separate change we could consider using NAMES_PER_DIR in FindOpenSSL. Please try it and post another patch if it works.

FYI, the reason for the default dirs-per-name loop ordering is that it was originally done to support version-specific names from newest to oldest. This was done VERY early in CMake development and modern find conventions had not yet been worked out. Later NAMES_PER_DIR was added for the use case we have in OpenSSL: multiple alternative names for the same library where we want the first of any name to be found.

(0039552)
Brad King (manager)
2015-10-08 14:10

Marking as 'resolved' because the main issue has been fixed. We can still discuss the NAMES_PER_DIR follow-up change though.

(0040610)
Robert Maynard (manager)
2016-03-07 09:12

Closing resolved issues that have not been updated in more than 4 months.

Notes
(0039506) Brad King (manager) 2015-10-05 11:06 edited on: 2015-10-05 11:06	Thanks for working on this. The dll-first problem is more general than just FindOpenSSL so let's see if we can solve that directly. Does this patch work for your case? diff --git a/Modules/Platform/Windows-GNU.cmake b/Modules/Platform/Windows-GNU.cmake index d8a423e..b4fc4aa 100644 --- a/Modules/Platform/Windows-GNU.cmake +++ b/Modules/Platform/Windows-GNU.cmake @@ -35,7 +35,7 @@ endif() if(MINGW) set(CMAKE_FIND_LIBRARY_PREFIXES "lib" "") - set(CMAKE_FIND_LIBRARY_SUFFIXES ".dll" ".dll.a" ".a" ".lib") + set(CMAKE_FIND_LIBRARY_SUFFIXES ".dll.a" ".a" ".lib" ".dll") set(CMAKE_C_STANDARD_LIBRARIES_INIT "-lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32") set(CMAKE_CXX_STANDARD_LIBRARIES_INIT "${CMAKE_C_STANDARD_LIBRARIES_INIT}") endif()

(0039507) Wayne Stambaugh (reporter) 2015-10-05 11:59	Your patch does not fix the issue. There may be several issues at play here. I'm not sure about the path search order CMake uses in find_library() but I can tell you that building and installing OpenSSL from source on mingw results in libraries named libcrypto.dll and libssl.dll not libeay32.dll and libssleay32.dll. This may be some legacy output naming to prevent conflicts on windows but it not longer appears to be the case. If you look at my patch, you will notice that I moved the search for the crypto and ssl libraries out of the cross build test. If memory serves, I also had to remove the system paths for find_library() to prevent the libraries in the system path being found. This is what confused me. I would expect that when I defined OPENSSL_ROOT_DIR that it would take precedence over all other search paths. Perhaps this is an invalid assumption on my part.

(0039508) Brad King (manager) 2015-10-05 12:41	Sorry, I meant the patch in 0015765:0039506 as a replacement for just the hunks that look like this: + # Do not search system path. Otherwise the DLL will be found rather than the link library. + NO_SYSTEM_ENVIRONMENT_PATH + NO_CMAKE_SYSTEM_PATH Does removing those hunks and adding my patch still work?

(0039509) Wayne Stambaugh (reporter) 2015-10-05 12:49 edited on: 2015-10-05 12:52	I commented out NO_SYSTEM_ENVIRONMENT_PATH and NO_CMAKE_SYSTEM_PATH in my custom FindOpenSSL.cmake and used your patch but no luck. It still found the libraries in the windows system path.

(0039516) Brad King (manager) 2015-10-06 11:42	Re 0015765:0039509: Thanks for trying that. I realized that the suffix ordering only matters within a single directory, not over the whole search path, so the patch in 0015765:0039506 has no chance of working.

(0039517) Brad King (manager) 2015-10-06 11:59	I split out the version parsing fix into its own commit: FindOpenSSL: Tolerate tabs in header while parsing version https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=6b575dec [^] Then I made another change based on your main MinGW-specific hunk: FindOpenSSL: Search for unix-named libraries first on MinGW https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=1bf66fed [^] By searching for "crypto" and "ssl" first this may avoid finding the DLLs. Please try it.

(0039520) Wayne Stambaugh (reporter) 2015-10-06 14:05	I applied both patches to the CMake version 3.3.2 of FindOpenSSL.cmake and it solved the problem. The $10K question is what happens if at some point in the future the openssl project changes the name for the native windows builds to libsll and libcrypto instead of libealy32 and ssleay32. We would be right back to where we are now. While this fixes this single problem, I have a much larger concern that find_library() does not honor the user's request for a specific library search path when it is defined. The path search order should take precedence over the library name search order. It appears that the library path search loop is inside the library name search loop instead of the other way around. I may have to rethink some of my custom find cmake modules.

(0039521) Brad King (manager) 2015-10-06 14:31	Re 0015765:0039520: Try the NAMES_PER_DIR option to find_library to flip the loop order. The full find_library search order is documented here: https://cmake.org/cmake/help/v3.4/command/find_library.html [^]

(0039522) Wayne Stambaugh (reporter) 2015-10-06 14:51	I can't believe as many times as I've looked at the find_library() documentation I never noticed that option. I need to slow down and read the documentation more carefully. Thanks for the tip.

(0039551) Brad King (manager) 2015-10-08 14:09	Re 0015765:0039520: Even if the native Windows OpenSSL changes to the ssl/crypto names this should still work because the HINTS will precede the PATH anyway. It was failing before this fix only due to preferring the eay32 names first regardless of directory. Now we try the eay32 names last. As a separate change we could consider using NAMES_PER_DIR in FindOpenSSL. Please try it and post another patch if it works. FYI, the reason for the default dirs-per-name loop ordering is that it was originally done to support version-specific names from newest to oldest. This was done VERY early in CMake development and modern find conventions had not yet been worked out. Later NAMES_PER_DIR was added for the use case we have in OpenSSL: multiple alternative names for the same library where we want the first of any name to be found.

(0039552) Brad King (manager) 2015-10-08 14:10	Marking as 'resolved' because the main issue has been fixed. We can still discuss the NAMES_PER_DIR follow-up change though.

(0040610) Robert Maynard (manager) 2016-03-07 09:12	Closing resolved issues that have not been updated in more than 4 months.

Issue History
Date Modified	Username	Field	Change
2015-10-03 11:40	Wayne Stambaugh	New Issue
2015-10-03 11:40	Wayne Stambaugh	File Added: find-openssl-cmake-3.3-mingw-fix.patch
2015-10-05 11:06	Brad King	Note Added: 0039506
2015-10-05 11:06	Brad King	Note Edited: 0039506
2015-10-05 11:59	Wayne Stambaugh	Note Added: 0039507
2015-10-05 12:41	Brad King	Note Added: 0039508
2015-10-05 12:49	Wayne Stambaugh	Note Added: 0039509
2015-10-05 12:52	Wayne Stambaugh	Note Edited: 0039509
2015-10-06 11:42	Brad King	Note Added: 0039516
2015-10-06 11:51	Brad King	Relationship added	related to 0013431
2015-10-06 11:59	Brad King	Note Added: 0039517
2015-10-06 14:05	Wayne Stambaugh	Note Added: 0039520
2015-10-06 14:31	Brad King	Note Added: 0039521
2015-10-06 14:51	Wayne Stambaugh	Note Added: 0039522
2015-10-08 14:09	Brad King	Note Added: 0039551
2015-10-08 14:10	Brad King	Note Added: 0039552
2015-10-08 14:10	Brad King	Assigned To	=> Brad King
2015-10-08 14:10	Brad King	Status	new => resolved
2015-10-08 14:10	Brad King	Resolution	open => fixed
2015-10-08 14:10	Brad King	Fixed in Version	=> CMake 3.5
2015-10-08 14:10	Brad King	Target Version	=> CMake 3.5
2016-03-07 09:12	Robert Maynard	Note Added: 0040610
2016-03-07 09:12	Robert Maynard	Status	resolved => closed