[jira] [Commented] (THRIFT-1031) Patch to compile Thrift for vc++ 9.0 and 10.0

["James Dickson (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/THRIFT-1031?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13014478#comment-13014478 ] 

James Dickson commented on THRIFT-1031:
---------------------------------------

Thank you for the quick feedback.

@Roger + David: I agree that the extra dependency is not ideal, especially as such a small part of it is used. The alternative I see would be to simply use Winsock2 on Windows as the previous patch did. If that would be the way to go then incorporating previous comments, changes to the previous patch would need to be:

1. Create a "platform.h" header, which on *nix machines includes "config.h" (generated by ./configure) and header files which are not found on Windows. This new header would be used in code where "config.h" had been included directly previously. (This is based on Roger's comment on 29/Mar/11 19:10, THRIFT-1123 part a.)

2. (1.) Would not fix situations in which *nix only headers have been included in a file. Replacing these headers with "platform.h" would pull in headers that file would not require, which has been a point of contention previously. However, if this is the only work around then maybe it is acceptable?

@Roger: As regards part b. of your comment in THRIFT-1123 on 29/Mar/11 19:10, I like that solution and will make the change.

@Roger: As regards part c. of your comment in THRIFT-1123 on 29/Mar/11 19:10, it is a yes and no answer. Yes in that the Winsock2 equivalent of those functions are named completely differently. For instance fcntl(...) becomes ioctlsocket(...). However, as far as requiring them to be macros then no. Maybe an alternative is to make TSocket a template class where one of the template parameters is an OS socket layer policy. This would mean the implementation of low level socket routines could exist in platform specific folders (along with gettimeofday(...) on Windows) and leave the TSocket class almost the same.

Just to avoid any confusion in the above statement, TSocket would become:

{code}

template< class SOCKET_POLICY >
class TSocket : public TVirtualTransport<TSocket>, private SOCKET_POLICY {

...

}

Where SOCKET_POLICY implements the following:

close(...)
usleep(...)
poll(...)
reset(...)
blocking(...)
non_blocking(...)

{code}

If people are happy with the proposal I will start those changes, but would appreciate any comments before hand.

> Patch to compile Thrift for vc++ 9.0 and 10.0
> ---------------------------------------------
>
>                 Key: THRIFT-1031
>                 URL: https://issues.apache.org/jira/browse/THRIFT-1031
>             Project: Thrift
>          Issue Type: Improvement
>          Components: C++ - Library
>         Environment: Windows XP 32bit, vc++ 9.0, 10.0
>            Reporter: James Dickson
>            Priority: Trivial
>             Fix For: 0.7
>
>         Attachments: thrift_msvc.patch, thrift_msvc_v0_1.patch
>
>
> At our company we need clients running on Windows being able to connect to our linux servers running hypertable. The attached patch enables the parts needed by Hypertable to be compiled on Windows using either the VC++ 9.0 or 10.0 compilers.
> Having read previous posts about ports using boost::asio we found these to be too intrusive for our needs. This version uses pthreads_win32 and winsock2 and is as designed to be as un-intrusive as is possible to the original unix code base. It is mostly #defines between unix sockets and winsock2 sockets. We also tried to follow the folder structuring of the C# runtime that has visual studio solutions to be consistent.
> More details are in the README as not all the functionality of the original unix code base is available to windows users. We will add the missing functionality, we just wanted to share what we had as for a Windows based client for us it is sufficient.
> The patch is based on the latest revision in SVN, we would love feedback and any code reviews. If there is any possibility of this being added to the main trunk then that would be much appreciated, however we don't expect that.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira