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/08/01 23:23:42 UTC
Review Request: patches for mingw
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/
-----------------------------------------------------------
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 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
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/#review9775
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h
<https://reviews.apache.org/r/6302/#comment20764>
In the most recent API (maybe not in the version you started with) there is also pn_connector_fd() that also needs modifying in this way.
- Andrew Stitcher
On Aug. 1, 2012, 9:23 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 9:23 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 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Rafael Schloming <rh...@apache.org>.
> On Aug. 7, 2012, 3:58 p.m., Andrew Stitcher wrote:
> > I think this is pretty much what I was thinking. I don't really like the actual client file doing the #include <winsock2> but in the circumstances it's better than the previous alternative.
> >
> > Another possibility is to move the pn_connector_fd()/pn_listener_fd() APIs to their own file say "driver_extras.h" as currently these APIs aren't actually used by anything as far as anyone can tell me.
I like the idea of splitting them out into a separate .h file, although we could do that in a separate patch if it's more convenient.
- Rafael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9964
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> 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>.
> On Aug. 7, 2012, 3:58 p.m., Andrew Stitcher wrote:
> > I think this is pretty much what I was thinking. I don't really like the actual client file doing the #include <winsock2> but in the circumstances it's better than the previous alternative.
> >
> > Another possibility is to move the pn_connector_fd()/pn_listener_fd() APIs to their own file say "driver_extras.h" as currently these APIs aren't actually used by anything as far as anyone can tell me.
>
> Rafael Schloming wrote:
> I like the idea of splitting them out into a separate .h file, although we could do that in a separate patch if it's more convenient.
I am happy to rework the patch this way.
- Cliff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9964
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> 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/#review9964
-----------------------------------------------------------
I think this is pretty much what I was thinking. I don't really like the actual client file doing the #include <winsock2> but in the circumstances it's better than the previous alternative.
Another possibility is to move the pn_connector_fd()/pn_listener_fd() APIs to their own file say "driver_extras.h" as currently these APIs aren't actually used by anything as far as anyone can tell me.
- Andrew Stitcher
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> 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
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. 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 Cliff Jansen <cl...@gmail.com>.
> On Aug. 7, 2012, 1:37 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h, line 192
> > <https://reviews.apache.org/r/6302/diff/1-2/?file=132726#file132726line192>
> >
> > Minor "artistic flourish" - how about we change this api to substitute "socket" for "fd"? I think that makes it's intent clearer. E.g:
> >
> > "pn_connector_socket( pn_driver_t *driver, pn_socket_t sock, void *context)"
> >
> > Just wonderin'....
>
> Rafael Schloming wrote:
> Sorry if I'm missing some context here, still catching up on the details, but wouldn't that simply shift the inaccuracy from windows to unix, i.e. it would be called _socket_, but fd's would also work? Or are you suggesting a separate API for windows and it would remain the same on unix?
>
> Kenneth Giusti wrote:
> I'm not recommending separate windows/unix api, but the type "pn_socket_t" seems likely to imply the intent of that parameter, no? And to a windows programmer, a file descriptor (fd) wouldn't be the correct argument.
>
> Would it be more appropriate to typedef the parameter as pn_network_handle_t, or a more generic pn_io_handle_t?
>
> Rafael Schloming wrote:
> I think my question was more whether we're trying to shoehorn two concepts into one by having one type that is both the generic notion of fd on unix, but is restricted to a socket on windows. It seems like we have the following options:
>
> (1) restrict the unix usage to socket only (in which case the naming you suggest is appropriate)
> (2) allow generic fds on unix and sockets only on windows and be vague/fuzzy about the naming
> (3) split this portion of API according to platform, e.g. have a windows include and a unix include
> (4) introduce a more sophisticated facade layer akin to OpenSSL's BIOs that could represent both socket and non socket sources of I/O on any platform
>
> We also have the option to omit this portion of the API for now as I don't think it's currently used, and defer the decision till later.
Is the API intended to work on non-sockets, such as pipes? In that case a Windows HANDLE would be the common denominator. Windows SOCKETs are essentially OS HANDLEs from our perspective. The overlapped I/O implementation in the C++ code should be compatible with this usage. I would second pn_io_handle_t.
- Cliff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9957
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Kenneth Giusti <kg...@apache.org>.
> On Aug. 7, 2012, 1:37 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h, line 192
> > <https://reviews.apache.org/r/6302/diff/1-2/?file=132726#file132726line192>
> >
> > Minor "artistic flourish" - how about we change this api to substitute "socket" for "fd"? I think that makes it's intent clearer. E.g:
> >
> > "pn_connector_socket( pn_driver_t *driver, pn_socket_t sock, void *context)"
> >
> > Just wonderin'....
>
> Rafael Schloming wrote:
> Sorry if I'm missing some context here, still catching up on the details, but wouldn't that simply shift the inaccuracy from windows to unix, i.e. it would be called _socket_, but fd's would also work? Or are you suggesting a separate API for windows and it would remain the same on unix?
I'm not recommending separate windows/unix api, but the type "pn_socket_t" seems likely to imply the intent of that parameter, no? And to a windows programmer, a file descriptor (fd) wouldn't be the correct argument.
Would it be more appropriate to typedef the parameter as pn_network_handle_t, or a more generic pn_io_handle_t?
- Kenneth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9957
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Rafael Schloming <rh...@apache.org>.
> On Aug. 7, 2012, 1:37 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h, line 192
> > <https://reviews.apache.org/r/6302/diff/1-2/?file=132726#file132726line192>
> >
> > Minor "artistic flourish" - how about we change this api to substitute "socket" for "fd"? I think that makes it's intent clearer. E.g:
> >
> > "pn_connector_socket( pn_driver_t *driver, pn_socket_t sock, void *context)"
> >
> > Just wonderin'....
Sorry if I'm missing some context here, still catching up on the details, but wouldn't that simply shift the inaccuracy from windows to unix, i.e. it would be called _socket_, but fd's would also work? Or are you suggesting a separate API for windows and it would remain the same on unix?
- Rafael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9957
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Rafael Schloming <rh...@apache.org>.
> On Aug. 7, 2012, 1:37 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h, line 192
> > <https://reviews.apache.org/r/6302/diff/1-2/?file=132726#file132726line192>
> >
> > Minor "artistic flourish" - how about we change this api to substitute "socket" for "fd"? I think that makes it's intent clearer. E.g:
> >
> > "pn_connector_socket( pn_driver_t *driver, pn_socket_t sock, void *context)"
> >
> > Just wonderin'....
>
> Rafael Schloming wrote:
> Sorry if I'm missing some context here, still catching up on the details, but wouldn't that simply shift the inaccuracy from windows to unix, i.e. it would be called _socket_, but fd's would also work? Or are you suggesting a separate API for windows and it would remain the same on unix?
>
> Kenneth Giusti wrote:
> I'm not recommending separate windows/unix api, but the type "pn_socket_t" seems likely to imply the intent of that parameter, no? And to a windows programmer, a file descriptor (fd) wouldn't be the correct argument.
>
> Would it be more appropriate to typedef the parameter as pn_network_handle_t, or a more generic pn_io_handle_t?
I think my question was more whether we're trying to shoehorn two concepts into one by having one type that is both the generic notion of fd on unix, but is restricted to a socket on windows. It seems like we have the following options:
(1) restrict the unix usage to socket only (in which case the naming you suggest is appropriate)
(2) allow generic fds on unix and sockets only on windows and be vague/fuzzy about the naming
(3) split this portion of API according to platform, e.g. have a windows include and a unix include
(4) introduce a more sophisticated facade layer akin to OpenSSL's BIOs that could represent both socket and non socket sources of I/O on any platform
We also have the option to omit this portion of the API for now as I don't think it's currently used, and defer the decision till later.
- Rafael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9957
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9957
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h
<https://reviews.apache.org/r/6302/#comment21156>
Personally, I'd put the #include <winsock2.h> in here, right before the typedef SOCKET pn_socket_t line.
Not sure if that conflicts with what Andrew recommended, but I think it would be OK to pull in the windows stuff in this file, and remove it from all the other places that include it in this patch (e.g. python.i, types.h, messenger.c, etc).
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h
<https://reviews.apache.org/r/6302/#comment21157>
Minor "artistic flourish" - how about we change this api to substitute "socket" for "fd"? I think that makes it's intent clearer. E.g:
"pn_connector_socket( pn_driver_t *driver, pn_socket_t sock, void *context)"
Just wonderin'....
- Kenneth Giusti
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> 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>.
> On Aug. 10, 2012, 3:10 p.m., Rafael Schloming wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c, line 327
> > <https://reviews.apache.org/r/6302/diff/2/?file=134512#file134512line327>
> >
> > Is there a way to do the equivalent of "0.0.0.0" on windows? This is intended to allow remote incoming connections, so the 127.0.0.1 default isn't an ideal choice here. We should at least make the default platform dependent if we have no better option at the moment.
Windows is OK with "0.0.0.0" for listening, but returns an error when connecting (complaining that it is a meaningless destination).
I believe that Linux converts it to the loopback, or it defaults to the loopback for the majority of routing tables.
I think I left all listening addresses as they were and only changed the outbound connections.
> On Aug. 10, 2012, 3:10 p.m., Rafael Schloming wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c, line 24
> > <https://reviews.apache.org/r/6302/diff/2/?file=134514#file134514line24>
> >
> > What's the #include <winsock2.h> for? Offhand I can't spot what part of the code would need that.
It is needed for gethostname(), perhaps others.
> On Aug. 10, 2012, 3:10 p.m., Rafael Schloming wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c, line 1
> > <https://reviews.apache.org/r/6302/diff/2/?file=134518#file134518line1>
> >
> > Is this just a wholesale copy + mod of src/driver.c?
Essentially yes. It came from Ken's branch which had select() substituted for poll. I first tried WSAPoll() but mingw kicked up a fuss.
- Cliff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review10122
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Rafael Schloming <rh...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review10122
-----------------------------------------------------------
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c
<https://reviews.apache.org/r/6302/#comment21430>
Is there a way to do the equivalent of "0.0.0.0" on windows? This is intended to allow remote incoming connections, so the 127.0.0.1 default isn't an ideal choice here. We should at least make the default platform dependent if we have no better option at the moment.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c
<https://reviews.apache.org/r/6302/#comment21424>
What's the #include <winsock2.h> for? Offhand I can't spot what part of the code would need that.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c
<https://reviews.apache.org/r/6302/#comment21433>
Is this just a wholesale copy + mod of src/driver.c?
- Rafael Schloming
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> 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 Aug. 6, 2012, 8:16 p.m.)
Review request for qpid, Andrew Stitcher, Kenneth Giusti, Steve Huston, and Rafael Schloming.
Changes
-------
pn_socket_t moved to driver.h; pn_connector_fd references fixed.
uuid.h: windows.h removed.
util.c: unneeded strings.h and _stricmp removed.
proton-dump.c: perror replaced with custom fatal_error() function.
windows.h or winsock2.h were pulled in to just the source files needing them.
The posix driver.c move to the sys/posix directory will be done separately.
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 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
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>.
> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt, line 39
> > <https://reviews.apache.org/r/6302/diff/1/?file=132723#file132723line39>
> >
> > Actually, I think driver.c should be split in two: there's quite a bit of platform generic code in this file that should be shared.
> >
> > Take a look at the "driver_abstraction" branch upstream. I've split driver.c into two files - isolating (some) of the platform code into a separate "impl" file.
> >
> > To support windows, we'd have to cleave off more code than I have in the branch, but it's a start.
I agree. The current windows/driver.c is a straw man, just to get us rolling. When the time comes to do it properly using I/O completion ports (and presumably stick handling the ssl layer too), I (or any interested party) can continue working with what you have.
> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c, line 324
> > <https://reviews.apache.org/r/6302/diff/1/?file=132732#file132732line324>
> >
> > The sematics are somewhat different here: 0.0.0.0 == all addresses. Is this change ok?
It is necessary to work on Windows, where the OS treats a connection to "all addresses" as meaningless, and returns an error.
Listening on 0.0.0.0 is fine, and those uses are unchanged.
> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c, line 35
> > <https://reviews.apache.org/r/6302/diff/1/?file=132733#file132733line35>
> >
> > Problem: calling sprintf may change errno, which means perror could print the wrong description.
Good catch. will fix.
> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c, line 1
> > <https://reviews.apache.org/r/6302/diff/1/?file=132737#file132737line1>
> >
> > See my previous comment re: splitting up this file.
> >
> > I think we should come up with a layered abstraction here, so we can not only support different platforms (win vs unix), but also different i/o mechs (poll, epoll, windows completions), and the use of different TLS/SSL impls.
Agreed. But I would like to get an initial Windows version going first.
> On Aug. 2, 2012, 2:58 p.m., Kenneth Giusti wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c, line 87
> > <https://reviews.apache.org/r/6302/diff/1/?file=132733#file132733line87>
> >
> > ditto
ditto
- Cliff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9755
-----------------------------------------------------------
On Aug. 6, 2012, 8:16 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2012, 8:16 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 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/php/php.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/driver.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.h 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1369190
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1369190
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>
Re: Review Request: patches for mingw
Posted by Kenneth Giusti <kg...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9755
-----------------------------------------------------------
Ship it!
This looks good. The only serious comment I have applies less to this patch, but more generally to the existing structure of the "driver.c" layer - it needs to be abstracted a bit more to ease portability across different networking api's and unix/win.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/6302/#comment20740>
Actually, I think driver.c should be split in two: there's quite a bit of platform generic code in this file that should be shared.
Take a look at the "driver_abstraction" branch upstream. I've split driver.c into two files - isolating (some) of the platform code into a separate "impl" file.
To support windows, we'd have to cleave off more code than I have in the branch, but it's a start.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c
<https://reviews.apache.org/r/6302/#comment20743>
The sematics are somewhat different here: 0.0.0.0 == all addresses. Is this change ok?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c
<https://reviews.apache.org/r/6302/#comment20741>
Problem: calling sprintf may change errno, which means perror could print the wrong description.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c
<https://reviews.apache.org/r/6302/#comment20742>
ditto
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c
<https://reviews.apache.org/r/6302/#comment20744>
See my previous comment re: splitting up this file.
I think we should come up with a layered abstraction here, so we can not only support different platforms (win vs unix), but also different i/o mechs (poll, epoll, windows completions), and the use of different TLS/SSL impls.
- Kenneth Giusti
On Aug. 1, 2012, 9:23 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 9:23 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 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
>
> 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>.
> On Aug. 2, 2012, 12:04 a.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h, line 40
> > <https://reviews.apache.org/r/6302/diff/1/?file=132727#file132727line40>
> >
> > I think it might be better to keep pn_socket_t out of a header file that will be widely included by arbitrary client code if it is going to include a windows system header file (with their name space polluting ways and general large size). Perhaps pn_socket_t is defined with the driver API?
>
> Cliff Jansen wrote:
> The latter is the problem. pn_listen_fd() allows the application to pass in a socket to use.
>
> I am not sure there is a way to avoid pollution and gain access to the Winsock SOCKET definition.
>
> Thoughts?
Well pn_listener_fd() is defined in driver.h so polluting the namespace of only things that include that would be an improvement - types.h is surely more widely used in enduser clients. Perhaps it would be better though to put the listener and connector APIs in their own interface to lessen this pollution.
Another possibility might be to use and intptr type on both platforms and have wrappers for creation and destruction that avoid the need for the real type - a bit heavy handed though it is close to what we do in qpid.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9728
-----------------------------------------------------------
On Aug. 1, 2012, 9:23 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 9:23 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 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
>
> 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>.
> On Aug. 2, 2012, 12:04 a.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt, line 39
> > <https://reviews.apache.org/r/6302/diff/1/?file=132723#file132723line39>
> >
> > Perhaps it makes sense to move src/driver.c into src/sys/posix/driver.c?
I meant to state in the notes that I would do so in a separate patch, depending on whether there was an alternate suggestion to managing the platform specific code.
> On Aug. 2, 2012, 12:04 a.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h, line 40
> > <https://reviews.apache.org/r/6302/diff/1/?file=132727#file132727line40>
> >
> > I think it might be better to keep pn_socket_t out of a header file that will be widely included by arbitrary client code if it is going to include a windows system header file (with their name space polluting ways and general large size). Perhaps pn_socket_t is defined with the driver API?
The latter is the problem. pn_listen_fd() allows the application to pass in a socket to use.
I am not sure there is a way to avoid pollution and gain access to the Winsock SOCKET definition.
Thoughts?
> On Aug. 2, 2012, 12:04 a.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h, line 29
> > <https://reviews.apache.org/r/6302/diff/1/?file=132736#file132736line29>
> >
> > It seems likely to me that you don't need to #include <windows.h> in this header file - just including it in the actual implementation file would stop it contaminating everything with windows symbols.
I can do that.
> On Aug. 2, 2012, 12:04 a.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c, line 27
> > <https://reviews.apache.org/r/6302/diff/1/?file=132740#file132740line27>
> >
> > Note that strings.h is a purely Unix thing not in the C standard library.
> >
> > So the there should be a test like #ifdef __unix__ around it, something like:
> > #ifdef __unix__
> > # include <strings.h>
> > #elif defined(_WIN32)
> > # define strcasecmp(a,b) _stricmp((a), (b))
> > #else
> > # error
> > #endif
> >
> > Although it might be simply better to avoid these not very standard calls and use tolower() and strcmp().
> >
> > In fact since I can see you've done that work lower down these lines can probably be removed completely.
Oops. This is cruft left behind when I switched to the plan B you noted. Will do.
- Cliff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6302/#review9728
-----------------------------------------------------------
On Aug. 1, 2012, 9:23 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 9:23 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 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
>
> 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/#review9728
-----------------------------------------------------------
I'm pretty happy with the way this is shaping up just a few nits below.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/CMakeLists.txt
<https://reviews.apache.org/r/6302/#comment20710>
Perhaps it makes sense to move src/driver.c into src/sys/posix/driver.c?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h
<https://reviews.apache.org/r/6302/#comment20712>
I think it might be better to keep pn_socket_t out of a header file that will be widely included by arbitrary client code if it is going to include a windows system header file (with their name space polluting ways and general large size). Perhaps pn_socket_t is defined with the driver API?
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h
<https://reviews.apache.org/r/6302/#comment20715>
It seems likely to me that you don't need to #include <windows.h> in this header file - just including it in the actual implementation file would stop it contaminating everything with windows symbols.
http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c
<https://reviews.apache.org/r/6302/#comment20713>
Note that strings.h is a purely Unix thing not in the C standard library.
So the there should be a test like #ifdef __unix__ around it, something like:
#ifdef __unix__
# include <strings.h>
#elif defined(_WIN32)
# define strcasecmp(a,b) _stricmp((a), (b))
#else
# error
#endif
Although it might be simply better to avoid these not very standard calls and use tolower() and strcmp().
In fact since I can see you've done that work lower down these lines can probably be removed completely.
- Andrew Stitcher
On Aug. 1, 2012, 9:23 p.m., Cliff Jansen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6302/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 9:23 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 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/CMakeLists.txt 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/bindings/python/python.i 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/driver.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/include/proton/types.h 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/codec/codec.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/dispatcher/dispatcher.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/framing/framing.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/message/message.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/messenger.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/proton-dump.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/network.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/posix/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/uuid.h PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/driver.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/time.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/sys/windows/uuid.c PRE-CREATION
> http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-c/src/util.c 1368240
> http://svn.apache.org/repos/asf/qpid/proton/trunk/tests/proton_tests/messenger.py 1368240
>
> Diff: https://reviews.apache.org/r/6302/diff/
>
>
> Testing
> -------
>
> Fedora, Windows
>
>
> Thanks,
>
> Cliff Jansen
>
>