* Bill Hoffman <bill.hoffman_at_kitware.com> [Thu, 01 Apr 2010 08:29:26
-0400]:

> - socklen_t
> I still can not reproduce the issue you are having with socklen_t. I
> have built with VS 2008 and it works fine. There must be a way to get
> socklen_t on windows it is a system type. I guess this happens if an
> application includes curl.h before windows.h? Can you provide a small
> example program that shows this failure?

I met the issue when I tried to add CURL_PULL_WS32TCPIP_H into
curlbuild.h.
The problem is that it includes <winsock2.h>.

The following sequence causes lots of compile errors in my VS2008
environment:

---- cut ---
#undef WIN32_LEAN_AND_MEAN
#include <winsock.h>
#include <winsock2.h>

... whatever code ...
--- cut ---

If you include the files the other way round, it works. The problem is,
you cannot guarantee the order of #includes.

Anyway, there is no problem with socklen_t in my patch.

For windows I have done it exactly like it was in "non-configure"
curlbuild.h.dist:
CURL_TYPEOF_CURL_SOCKLEN_T is 'int' for all windows versions.

> - CURL_PULL_SYS_TYPES_H 1
>
> I guess this sort of goes with the socklen_t. If these are issues, I
> think a new test file that shows the issue should be created and made
> part of curl.

Yes, without including <sys/types.h> you cannot use socklen_t on
OpenSolaris.
On Linuxes or OpenBSD, you need not include it (at least on those where
I can
check the build).

I admit that my way of setting CURL_PULL_SYS_TYPES_H is a bit
aggressive,
but it doesn't cause problems. I hope it fixes more than it breaks.

Of course, adding a new test for it would be the best solution. But we
can do
it later.

> - remove curlbuild.h
> Also, what is this for:
> +# Remove include/curl/curlbuild.h file if exists.
> +# Otherwise the build may be broken.
> +execute_process(COMMAND "${CMAKE_COMMAND}" -E remove
> + ${CMAKE_SOURCE_DIR}/include/curl/curlbuild.h ERROR_QUIET)
>
> I don't think you want to have that stuck in the cmake files forever..

Well, this is exactly how curl autoconf works:

dnl Remove non-configure distributed curlbuild.h
if test -f ${srcdir}/include/curl/curlbuild.h; then
  rm -f ${srcdir}/include/curl/curlbuild.h
fi

I admit, it looks awful and cannot be considered a good practice.But we
need it.

Here is a more in-depth explanation why it is required:

1. If CMake is used to build curl in-source, curlbuild.h is not renewed
because it
    already comes with the distibution. This causes problems on OSes
that are
    not handled with the distributed curlbuild.h.
2. If CMake is used to build curl out-of-source, then curl/curl.h
includes the wrong
    header (again, the one that comes with the distribution). Look:

curl/curl.h:
#include "curlver.h" /* libcurl version defines */
#include "curlbuild.h" /* libcurl build definitions !!! Note the
quotes around the filename */
#include "curlrules.h" /* libcurl rules enforcement */

> - version of CMake required
> Are you sure that 2.6.0 and 2.6.1 CMake will work? (did you test?)
>
> -cmake_minimum_required(VERSION 2.6.2 FATAL_ERROR)
> +cmake_minimum_required(VERSION 2.6 FATAL_ERROR)

Yes, I did.
I have changed the version check because I still have CMake 2.6.0 and
2.6.1 on
some of my test machines.

--
tetetest tetetest.

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