qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Stitcher" <astitc...@apache.org>
Subject Re: Review Request: patches for mingw
Date Thu, 02 Aug 2012 19:46:37 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message