Hi Brad,

Thank you for review.

2015-01-08 19:44 GMT+03:00 Brad King <brad.king_at_kitware.com>:

> On 12/23/2014 4:28 PM, Sergei Nikulov wrote:
> > The issue related to winsock2 detection after openssl already found (it
> will include windows.h first).
> > In this case - winsock2 check will fail and marked as not found
> > I've solve this issue by explicit definition -D_WINSOCKAPI_ in
> > CMAKE_REQUIRED_DEFINITIONS for WIN32 build generation.
>
> A simpler solution is to use just check_include_file instead of
> check_include_file_concat for openssl headers. They do not
> need to be included in the accumulated, ordered system headers.
> Something like this should work:
>
> - check_include_file_concat("openssl/crypto.h" HAVE_OPENSSL_CRYPTO_H)
> - check_include_file_concat("openssl/engine.h" HAVE_OPENSSL_ENGINE_H)
> - check_include_file_concat("openssl/err.h" HAVE_OPENSSL_ERR_H)
> - check_include_file_concat("openssl/pem.h" HAVE_OPENSSL_PEM_H)
> - check_include_file_concat("openssl/pkcs12.h" HAVE_OPENSSL_PKCS12_H)
> - check_include_file_concat("openssl/rsa.h" HAVE_OPENSSL_RSA_H)
> - check_include_file_concat("openssl/ssl.h" HAVE_OPENSSL_SSL_H)
> - check_include_file_concat("openssl/x509.h" HAVE_OPENSSL_X509_H)
> - check_include_file_concat("openssl/rand.h" HAVE_OPENSSL_RAND_H)
> + check_include_file("openssl/crypto.h" HAVE_OPENSSL_CRYPTO_H)
> + check_include_file("openssl/engine.h" HAVE_OPENSSL_ENGINE_H)
> + check_include_file("openssl/err.h" HAVE_OPENSSL_ERR_H)
> + check_include_file("openssl/pem.h" HAVE_OPENSSL_PEM_H)
> + check_include_file("openssl/pkcs12.h" HAVE_OPENSSL_PKCS12_H)
> + check_include_file("openssl/rsa.h" HAVE_OPENSSL_RSA_H)
> + check_include_file("openssl/ssl.h" HAVE_OPENSSL_SSL_H)
> + check_include_file("openssl/x509.h" HAVE_OPENSSL_X509_H)
> + check_include_file("openssl/rand.h" HAVE_OPENSSL_RAND_H)
>

Without updating CMAKE_REQUIRED_DEFINITIONS your simple solution will not
work.
I've chacked on https://github.com/bagder/curl master without my changes
applied.

If it is just addition to my changes - all works fine.

> > Also I've fixed tools_hugehelp.c generation using "cmake -E echo"
> command.
>
> In this hunk:
>
> > + COMMAND ${CMAKE_COMMAND} -E echo "/* built-in manual is disabled,
> blank function */" > "tool_hugehelp.c"
> > + COMMAND ${CMAKE_COMMAND} -E echo "\#include \"tool_hugehelp.h\"" >>
> "tool_hugehelp.c"
> > + COMMAND ${CMAKE_COMMAND} -E echo "void hugehelp(void) {}" >>
> "tool_hugehelp.c"
>
> The backslash in "\#include..." should not be needed.
>

Got it. Will remove it.

>
> The addition of quotes to the redirected file names
> is not necessary but does not hurt either.
>
> -Brad
>
>
>

-- 
Best Regards,
Sergei Nikulov

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2015-01-09