After feedback, and talking with a few of you, I've reduced the scope of the proposed patch a bit. See:
https://reviews.apache.org/r/7826/ The main purpose of this patch is to fix the python script bits in cmake so it will run in the windows environment. Additionally, I did add a bit of a tweak to the configuration of the extra compiler commands - the original method (unconditionally set via the COMPILE_FLAGS property) is causing a problem on windows: "command line error D8021: invalid numerical argument: /Werror" Or something like that - probably due to cmake not coping with the -Werror gnu compiler flag. That's about the extent of my Windows chops - hopefully these changes will ease the effort for the folks doing the windows port. They should be able to use cmake directly without having to monkey with the python scripts and such. They'll still have to come up with any additional compiler flags, as well as proper library and include path configuration - this patch doesn't address that stuff. -K ----- Original Message ----- > I'm looking forward to the python cmake support for Visual Studio, > especially for creating the _cproton library for python. > > I compared my qpid-proton dll project file for the one generated by > cmake > using the "Visual Studio 10" makefile selection. > > Ken's fix for the LIB64 on line 45 of CmakeLists.txt is definitely > needed to > create the Visual Studio project files with the current > CmakeLists.txt file. > > The main differences between the cmake automatically created project > file > for the qpid-proton.dll and my project file are listed below:: > > 1) Remove this line for Visual Studio: > <AdditionalOptions> /Zm1000 -Werror -pedantic-errors -std=c99 -g > -fPIC > %(AdditionalOptions)</AdditionalOptions> > Visual Studio does not use the c99 standard > > 2) change: (this may need to be added to the CMakeLists.txt for > Visual > Studio) > <CompileAs>CompileAsC</CompileAs> > to > <CompileAs>CompileAsCpp</CompileAs> > > The Visual Studio C compiler does not support C99. The C++ compiler > is > closer to the C99 standard and less changes to the code had to be > made to > support the Windows port using the Visual Studio C++ compiler. The > "extern > C" already in the code should take care of the C++ name mangling. > > 3) Turned off C++ exception handling so it would be more like C. > <ExceptionHandling>false</ExceptionHandling> > > 4) besides the preprocessor Definitions, cmake automatically gives > you, I > added a few more: > <PreprocessorDefinitions> > qpid_proton_python_EXPORTS;WIN32_LEAN_AND_MEAN;_WIN32_WINNT=0x0600; > (other > cmake preprocessor definitions added automatically) > The first one may change when the proton group discusses the dll > imports and > exports defines for Visual Studio. > _WIN32_WINNT=0x0600 -- Since the Windows socket doesn't have poll(), > I'm > using WSApoll() in place of it and it requires Vista or greater, > which is > represented by the 0x0600. > WIN32_LEAN_AND_MEAN is used with the socket library. > > 5) added some Visual Studio libraries that are not normally > included: > <AdditionalDependencies>ws2_32.lib;rpcrt4.lib; uuid.lib; (other > window > libraries automatically included etc...) > The first is the winsock2 library. Rpcrt4.lib and uuid.lib are used > for > creating the uuid. > > Mary > > -----Original Message----- > From: Rafael Schloming [mailto:r...@alum.mit.edu] > Sent: Thursday, November 01, 2012 12:17 PM > To: proton@qpid.apache.org > Subject: Re: review request: patch to cmake for windows builds > > FWIW, a lot of the patches that have been made to the build system so > far > were taken from the cpp broker's build and to be honest when you push > for > the reason they were done that way there is sometimes a good reason > and > sometimes no good reason, and sometimes a reason that doesn't apply > to > proton. Growing the build system by patching in a bunch of stuff > copied over > from another build system without sufficient thought is IMHO a bad > idea. > Even with only a handful of these patches the build system was > actually in a > pretty sorry state prior to the first 0.1 RC. I spent the better part > of a > day just cleaning up and fixing various issues that crept in with > different > kinds of install scenarios. I think in general given the portability > goals > of the project the build system needs to stay small, simple, easily > understood, and fairly self contained. In other words it should > accurately > reflect the minimal settings and configuration necessary to build > *proton* > source code, *not* the cpp broker's source code. > > --Rafael > > On Thu, Nov 1, 2012 at 10:49 AM, Ken Giusti <kgiu...@redhat.com> > wrote: > > > Hi Cliff, > > > > I agree, but where are those flags in the qpid source tree? > > > > Aside from the warning exclusion flags, that is (/wdXXXX). > > > > I've grepped about until blue in the fingers - about all I have > > found was: > > > > ./src/CMakeLists.txt:721: set (CMAKE_CXX_FLAGS_RELWITHDEBINFO > > "/MD /O2 > > /Ob2 /D NDEBUG") > > > > Nothing specific (that I can easily find) for Release or Debug > > targets. > > > > All - > > > > There is a ton of MSVC stuff spread throughout the myriad of CMake > > configuration files in the QPID source base. I was assuming we > > could > > start with a bare bones cmake just for the proton libraries, and > > add > > to it as needed. Would it be a better approach to try to port the > > existing qpid cmake files over to proton? That would probably > > involve > > more time than I can devote right now. > > > > Opinions? > > > > -K > > > > ----- Original Message ----- > > > I would personally just use the settings Steve came up with for > > > the > > > main qpid cpp build. Those should serve us well. The only > > > obvious > > > addition is the flag that forces C++ compilation from files named > > > foo.c. > > > > > > Cliff > > > > > > On Wed, Oct 31, 2012 at 12:05 PM, Ken Giusti <kgiu...@redhat.com> > > > wrote: > > > > Ah, almost forgot: > > > > > > > > The patch modifies the CMakeLists.txt file by adding compiler > > > > flags > > > > for the VC++ compiler. The flags differ based on each type of > > > > target build (Debug/Release/etc). > > > > > > > > Now, IANAVCP (I Am Not a VC Programmer), so I have no idea what > > > > the appropriate flags should be for the windows build. I've > > > > taken > > > > a guess at them, based on my limited understanding of the > > > > compiler. > > > > > > > > If anyone can provide better settings - please let me know. > > > > Here's the current settings: > > > > > > > > > > > > > > > > CMAKE_C_FLAGS_DEBUG "/Wall /Iinclude /MDd /Od /Zi") > > > > CMAKE_C_FLAGS_RELEASE "/Wall /Iinclude /MD /Ox /D > > > > NDEBUG") > > > > CMAKE_C_FLAGS_RELWITHDEBINFO "/Wall /Iinclude /MD /Ox /Zi") > > > > > > > > > > > > -K > > > > > > > > ----- Original Message ----- > > > >> Ah, then this patch is for *you*! :) > > > >> > > > >> It -should- fix it so you don't have to modify the project > > > >> files > > > >> at all, and it should fix those header files too. You should > > > >> just be able to run cmake, then pull the result into VS and > > > >> build. > > > >> > > > >> It doesn't go so far as to run the tests though - we'll need > > > >> to > > > >> get the code ported first. > > > >> > > > >> thanks, > > > >> > > > >> -K > > > >> > > > >> ----- Original Message ----- > > > >> > I used cmake to create Visual Studio 10 projects files and > > > >> > solution workspace. Then I modified the project files to add > > > >> > what I needed. > > > >> > I hadn't used cmake before this project. So I'm not an > > > >> > expert. > > > >> > > > > >> > I'll be happy to have your help with creating good support > > > >> > for > > > >> > building proton from cmake. > > > >> > > > > >> > The python support definitely needs to be fixed. And I would > > > >> > be > > > >> > happy to have that part working properly. I've been > > > >> > creating > > > >> > the header files (encodings.h and protocol.h) and setting > > > >> > up > > > >> > the python to run the tests outside of the project files. > > > >> > I'll > > > >> > look at your changes as soon as I can. > > > >> > > > > >> > Mary > > > >> > > > > >> > -----Original Message----- > > > >> > From: Ken Giusti [mailto:kgiu...@redhat.com] > > > >> > Sent: Wednesday, October 31, 2012 1:47 PM > > > >> > To: cliffjan...@gmail.com; Mary Hinton > > > >> > Cc: proton@qpid.apache.org > > > >> > Subject: review request: patch to cmake for windows builds > > > >> > > > > >> > Hi Cliff/Mary, > > > >> > > > > >> > I'm trying to add support for building proton using > > > >> > Microsoft > > > >> > Visual > > > >> > C++ express. This patch updates cmake to enable generation > > > >> > of > > > >> > the > > > >> > VC++ project files for proton. Can you review and try it > > > >> > out - > > > >> > let > > > >> > me know what you think? > > > >> > > > > >> > Note that the patch only adds build support - the problems > > > >> > with > > > >> > building proton on windows are not addressed. > > > >> > > > > >> > The diff can be viewed here: > > > >> > > > > >> > https://github.com/kgiusti/qpid-proton/compare/win7 > > > >> > > > > >> > And the changes are on the win7 branch of my github proton > > > >> > repo: > > > >> > > > > >> > git://github.com/kgiusti/qpid-proton.git > > > >> > > > > >> > -K > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > > > > >
