curl-library
Re: CMake build patch - updated version.
Date: Thu, 01 Apr 2010 12:01:35 -0400
tetetest tetetest wrote:
> * Bill Hoffman <bill.hoffman_at_kitware.com
> <mailto: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>.
>
OK, why did you add CURL_PULL_WS32TCPIP_H ?
> 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.
>
Not now, but if windows or some compiler ever changes socklen_t it could
break. Seems much safer to use the system define if there is one.
> 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.
It would be helpful for me to see the problem before fixing it.
>
> 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.
CMake should overwrite the file because it will see there are changes
right?
> 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 */
>
>
I am not seeing this, I am pretty sure my build is using the correct
file....
> > - 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.
This seems OK.
-Bill
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2010-04-01