You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Cliff Jansen <cl...@gmail.com> on 2012/12/17 22:37:43 UTC
Re: Review Request: patches for mingw
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/
-----------------------------------------------------------
(Updated Dec. 17, 2012, 9:37 p.m.)
Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and Rafael Schloming.
Changes
-------
Updated for trunk and reworked to use the platform.h/platform.c structure.
Headers now generate automatically thanks to a prior fix from Ken.
If approved, I will follow this with an additional change to move the existing src/driver.c to src/posix/driver.c to match the tree structure of the new src/windows/driver.c.
This plus the C++ related patches should get us 98% of the way to a Visual Studio build.
Description
-------
This patch set works with a recent mingw32, cmake 2.8.1, python 2.5, swig 2.0.7.
A push-button build is still a ways off. The custom_commands in the cmake script to generate the protocol headers don't work yet on Windows.
The most intrusive change was the introduction of a pn_socket_t type to hold a socket on both Windows and Posix platforms. An attempt was made to minimize the use of #ifdefs and split platform code into separate posix and windows directories, as is done for the C++ code. There is so little needed at the moment, this may be overkill. The qpid-proton-posix library was ditched and combined with the main qpid-proton library. Instead, the work is done in CMake to assemble the correct shared and platform specific sources as is done in the C++ tree.
The driver.c implementation is proof of concept using Winsock select(). Future work would most likely replace this with an I/O completion port implementation.
1. mkdir ...\trunk\proton-c\build
2. set env vars as per trunk\config.sh
set PATH=C:\Program Files (x86)\CMake 2.8\bin;C:\python25;C:\mingw_ptn\bin;C:\Windows\System32;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build
set PYTHONPATH=c:\cj\work\amqp\proton\mingw4\trunk\tests;c:\cj\work\amqp\proton\mingw4\trunk\proton-c;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
set PYTHON_BINDINGS=c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
set PROTON_HOME=C:\cj\work\amqp\proton\mingw4\trunk
3. generate the headers:
cd trunk\proton-c\build
python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\codec\encodings.h.py >encodings.h
python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\protocol.h.py >protocol.h
4. build
cmake -G "MinGW Makefiles" -DSWIG_EXECUTABLE=C:\swigwin-2.0.7\swig.exe c:\cj\work\amqp\proton\mingw4\trunk\proton-c
mingw32-make
python ..\..\tests\proton-test
This addresses bug QPID-4181.
https://issues.apache.org/jira/browse/QPID-4181
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/engine/engine.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/ssl/ssl_stub.c 1423094
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/windows/driver.c PRE-CREATION
Diff: https://reviews.apache.org/r/6302/diff/
Testing
-------
Fedora, Windows
Thanks,
Cliff Jansen
Re: Review Request: patches for mingw
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review14617
-----------------------------------------------------------
I thought we agreed to move the driver.h APIs that used pn_socket_t into their own header file driver_extras.h?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/6302/#comment31087>
You shouldn't need "-D" in add_definitions, so this should just be:
add_definitions(WIN32_LEAN_AND_MEAN NOMINMAX ...)
But actually I'd strongly suggest you should only add the windows specific flags on the windows only files, which would only be platform.c, driver.c, and anything else?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c
<https://reviews.apache.org/r/6302/#comment31093>
Pretty sure this isn't necessary.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/engine/engine.c
<https://reviews.apache.org/r/6302/#comment31094>
Again not necessary
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c
<https://reviews.apache.org/r/6302/#comment31089>
I think this line is gone in the current code (or it should be at any rate)
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c
<https://reviews.apache.org/r/6302/#comment31097>
This won't be necessary if pn_i_error_from_errno() is moved.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c
<https://reviews.apache.org/r/6302/#comment31088>
I think the entirety of pn_error_from_errno() (renamed as pn_i_error_from_errno() ) should be in platform.c. errno is an inherently platform specific concept and shouldn't be in the non platform code at all.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c
<https://reviews.apache.org/r/6302/#comment31095>
not necessary
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h
<https://reviews.apache.org/r/6302/#comment31090>
I think this entire block is obsolete now the code doesn't use ntohl() etc.
Surely you don't need windows.h just to get strerror_s (or whatever)
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h
<https://reviews.apache.org/r/6302/#comment31091>
Replace this with pn_i_error_from_errno() as above.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.c
<https://reviews.apache.org/r/6302/#comment31098>
I didn't realise earlier versions of mingw didn't have RPC_CSTR - I'll fix this.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c
<https://reviews.apache.org/r/6302/#comment31092>
Is this necessary?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/ssl/ssl_stub.c
<https://reviews.apache.org/r/6302/#comment31096>
shouldn't be necessary
- Andrew Stitcher
On Dec. 17, 2012, 9:37 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2012, 9:37 p.m.)
>
>
> Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and Rafael Schloming.
>
>
> Description
> -------
>
> This patch set works with a recent mingw32, cmake 2.8.1, python 2.5, swig 2.0.7.
>
> A push-button build is still a ways off. The custom_commands in the cmake script to generate the protocol headers don't work yet on Windows.
>
> The most intrusive change was the introduction of a pn_socket_t type to hold a socket on both Windows and Posix platforms. An attempt was made to minimize the use of #ifdefs and split platform code into separate posix and windows directories, as is done for the C++ code. There is so little needed at the moment, this may be overkill. The qpid-proton-posix library was ditched and combined with the main qpid-proton library. Instead, the work is done in CMake to assemble the correct shared and platform specific sources as is done in the C++ tree.
>
> The driver.c implementation is proof of concept using Winsock select(). Future work would most likely replace this with an I/O completion port implementation.
>
>
> 1. mkdir ...\trunk\proton-c\build
>
> 2. set env vars as per trunk\config.sh
>
>
> set PATH=C:\Program Files (x86)\CMake 2.8\bin;C:\python25;C:\mingw_ptn\bin;C:\Windows\System32;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build
> set PYTHONPATH=c:\cj\work\amqp\proton\mingw4\trunk\tests;c:\cj\work\amqp\proton\mingw4\trunk\proton-c;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
> set PYTHON_BINDINGS=c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
> set PROTON_HOME=C:\cj\work\amqp\proton\mingw4\trunk
>
> 3. generate the headers:
>
> cd trunk\proton-c\build
> python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\codec\encodings.h.py >encodings.h
> python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\protocol.h.py >protocol.h
>
> 4. build
>
> cmake -G "MinGW Makefiles" -DSWIG_EXECUTABLE=C:\swigwin-2.0.7\swig.exe c:\cj\work\amqp\proton\mingw4\trunk\proton-c
> mingw32-make
> python ..\..\tests\proton-test
>
>
> This addresses bug QPID-4181.
> https://issues.apache.org/jira/browse/QPID-4181
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/engine/engine.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/ssl/ssl_stub.c 1423094
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/windows/driver.c PRE-CREATION
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review14883
-----------------------------------------------------------
Ship it!
I'm happy with this.
I think one question in my mind still is whether driver_extras.h should include winsock2.h or not, but the issue is not big enough to stop this going in now.
- Andrew Stitcher
On Dec. 24, 2012, 8:55 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2012, 8:55 p.m.)
>
>
> Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and Rafael Schloming.
>
>
> Description
> -------
>
> This patch set works with a recent mingw32, cmake 2.8.1, python 2.5, swig 2.0.7.
>
> A push-button build is still a ways off. The custom_commands in the cmake script to generate the protocol headers don't work yet on Windows.
>
> The most intrusive change was the introduction of a pn_socket_t type to hold a socket on both Windows and Posix platforms. An attempt was made to minimize the use of #ifdefs and split platform code into separate posix and windows directories, as is done for the C++ code. There is so little needed at the moment, this may be overkill. The qpid-proton-posix library was ditched and combined with the main qpid-proton library. Instead, the work is done in CMake to assemble the correct shared and platform specific sources as is done in the C++ tree.
>
> The driver.c implementation is proof of concept using Winsock select(). Future work would most likely replace this with an I/O completion port implementation.
>
>
> 1. mkdir ...\trunk\proton-c\build
>
> 2. set env vars as per trunk\config.sh
>
>
> set PATH=C:\Program Files (x86)\CMake 2.8\bin;C:\python25;C:\mingw_ptn\bin;C:\Windows\System32;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build
> set PYTHONPATH=c:\cj\work\amqp\proton\mingw4\trunk\tests;c:\cj\work\amqp\proton\mingw4\trunk\proton-c;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
> set PYTHON_BINDINGS=c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
> set PROTON_HOME=C:\cj\work\amqp\proton\mingw4\trunk
>
> 3. generate the headers:
>
> cd trunk\proton-c\build
> python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\codec\encodings.h.py >encodings.h
> python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\protocol.h.py >protocol.h
>
> 4. build
>
> cmake -G "MinGW Makefiles" -DSWIG_EXECUTABLE=C:\swigwin-2.0.7\swig.exe c:\cj\work\amqp\proton\mingw4\trunk\proton-c
> mingw32-make
> python ..\..\tests\proton-test
>
>
> This addresses bug QPID-4181.
> https://issues.apache.org/jira/browse/QPID-4181
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/cproton.i 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver_extras.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/error.h 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.c 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1425672
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/windows/driver.c PRE-CREATION
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Cliff Jansen <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/
-----------------------------------------------------------
(Updated Dec. 24, 2012, 8:55 p.m.)
Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and Rafael Schloming.
Changes
-------
Hopefully, this follows all of Andrew's suggestions...
Description
-------
This patch set works with a recent mingw32, cmake 2.8.1, python 2.5, swig 2.0.7.
A push-button build is still a ways off. The custom_commands in the cmake script to generate the protocol headers don't work yet on Windows.
The most intrusive change was the introduction of a pn_socket_t type to hold a socket on both Windows and Posix platforms. An attempt was made to minimize the use of #ifdefs and split platform code into separate posix and windows directories, as is done for the C++ code. There is so little needed at the moment, this may be overkill. The qpid-proton-posix library was ditched and combined with the main qpid-proton library. Instead, the work is done in CMake to assemble the correct shared and platform specific sources as is done in the C++ tree.
The driver.c implementation is proof of concept using Winsock select(). Future work would most likely replace this with an I/O completion port implementation.
1. mkdir ...\trunk\proton-c\build
2. set env vars as per trunk\config.sh
set PATH=C:\Program Files (x86)\CMake 2.8\bin;C:\python25;C:\mingw_ptn\bin;C:\Windows\System32;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build
set PYTHONPATH=c:\cj\work\amqp\proton\mingw4\trunk\tests;c:\cj\work\amqp\proton\mingw4\trunk\proton-c;c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
set PYTHON_BINDINGS=c:\cj\work\amqp\proton\mingw4\trunk\proton-c\build\bindings\python
set PROTON_HOME=C:\cj\work\amqp\proton\mingw4\trunk
3. generate the headers:
cd trunk\proton-c\build
python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\codec\encodings.h.py >encodings.h
python c:\cj\work\amqp\proton\mingw4\trunk\proton-c\src\protocol.h.py >protocol.h
4. build
cmake -G "MinGW Makefiles" -DSWIG_EXECUTABLE=C:\swigwin-2.0.7\swig.exe c:\cj\work\amqp\proton\mingw4\trunk\proton-c
mingw32-make
python ..\..\tests\proton-test
This addresses bug QPID-4181.
https://issues.apache.org/jira/browse/QPID-4181
Diffs (updated)
-----
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/cproton.i 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver_extras.h PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/error.h 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/error.c 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.h 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/platform.c 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1425672
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/windows/driver.c PRE-CREATION
Diff: https://reviews.apache.org/r/6302/diff/
Testing
-------
Fedora, Windows
Thanks,
Cliff Jansen