Thrift
  1. Thrift
  2. THRIFT-591

Make the C++ runtime library be compatible with Windows and Visual Studio

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.9
    • Fix Version/s: 0.9.1
    • Component/s: C++ - Library
    • Labels:
      None
    • Environment:

      Windows XP and above, Visual Studio 2005 and above

    • Patch Info:
      Patch Available

      Description

      Modify the C++ runtime library to be compatible with Windows and able to be built by Visual Studio.

      The work has been done and a patch is available. I will attach it soon.

      Note that this issue and the attached patch supercedes the patches that I wrongly attached to JIRA 311. That issue is about making the C++ library support async client/server interaction.

      1. MsvcPatchSupportScripts.zip
        13 kB
        Rush Manbert
      2. MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch
        2.34 MB
        Steven Knight
      3. THRIFT-591_MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch_error.log
        30 kB
        Roger Meier
      4. thrift-818530-patched.zip
        3.07 MB
        Rush Manbert
      5. ThriftMsvcPatchForSvnRev818530.txt.zip
        298 kB
        Rush Manbert
      6. thriftWindowsRev818530BugFix.zip
        11 kB
        Rush Manbert

        Issue Links

          Activity

          Hide
          Rush Manbert added a comment -

          This patch makes the C++ runtime library buildable in Windows using Visual Studio. No fake Unix environment is needed. It uses Boost and Asio to add work-alike functionality for the existing POSIX-based sockets and thread code. It additionally adds support for Unix Domain Sockets and Windows Named Pipes, so that local client/server communication can be done without the use of localhost. This can simplify the use of Thrift on a single machine because it doesn't run into problems with firewalls, etc. It can also be much faster than using the TCP stack.

          Note that all of the new socket and thread code acts exactly like the existing implementations. On a *nix platform, the new socket code can either exist alongside or replace the existing code. The new thread implementation using Boost can only replace the POSIX implementation. The new code does NOT add asynchronous client/server communication.

          The first two iterations of this patch were originally attached to JIRA-311. This new version, for JIRA-591, supersedes those, and is applicable to the near-latest Thrift distribution tree. These comments started as a duplicate of my original comment for 311, but they have been modified so if you're interested, please read through them.

          The changes from the first patch version are:

          • Added the ability to select static or DLL runtime library linkage on Windows
          • Added the ability to specify the Boost and Zlib name patterns on a per-config basis for Windows
          • Added the ability to select stdint.h implementation on Windows
          • Fixed some really stupid bugs in formatted output that I had written. Since Windows doesn't define the PRId64 and PRIu64 formatting strings, I tried using the %j sprintf format strings, casting all the values to be output to either intmax_t or uintmax_t. I was sure that I tested that successfully in Windows before I did my first patch, but I must have been hallucinating, because when I ran the test this time all I saw printed was"ju", not a number. So I undid all that and just defined macros that work for Windows. These were mainly in the CppClient and CppServer test programs, but there was also one in TFileTransport.cpp.

          The patch file was generated against the SVN repository revision 818530. It patches the entire distribution tree, but only touches things that are C++ related.

          The patch addresses the following unresolved Jira issues:
          Thrift-456 (https://issues.apache.org/jira/browse/THRIFT-456)
          Thrift-457 (https://issues.apache.org/jira/browse/THRIFT-457)
          Thrift-487 (https://issues.apache.org/jira/browse/THRIFT-487)
          Thrift-488 (https://issues.apache.org/jira/browse/THRIFT-488)

          It also makes changes that are not issues in Jira. For instance:
          1. Autoconf support has been extended to all of the C++ tests, as well as the tutorial.
          2. The stress-test and stress-test-nb have been extensively reworked to make them more versatile and reliable. I relied on these a lot during my development to test the new socket and server code.
          3. The concurrency test has been extensively reworked to make it be reliable and more versatile. I work on a 8 core machine and the test as distributed just doesn't work reliably in that environment. I used this test to verify all of my Boost threading changes.
          4. All of the sources have been touched to add Windows-specific support, but I suppose that could be seen as part of this Jira issue.

          Disclaimers:
          -----------
          I had never done anything with autoconf before this. What I have done may not be correct. If changes are required, please let me know.

          The configure script displays information at the end that is related to the new configuration stuff. It was useful for me while developing and I thought it might be a good idea to leave it in for a while.

          I am really not a Windows guy, I just do what needs doing. If any of you are Windows experts, and you have suggestions or corrections, please let me know.

          See the "Known Issues" section in the msvc/README file.

          Licensing:
          ---------
          Obviously, I checked the box that says I grant the license to Apache for this code. I also added the Apache license header to all my new files.

          However. there are two files that I brought into the source tree that were already open-sourced, with license headers of their own.

          The first file is available at http://msinttypes.googlecode.com/svn/trunk/stdint.h, and I have brought it in as the file lib/cpp/src/VisualStudioStdint.h. It has the following notice:

          // ISO C9x compliant stdint.h for Miscrosoft Visual Studio
          // Based on ISO/IEC 9899:TC2 Committee draft (May 6, 2005) WG14/N1124
          //
          // Copyright (c) 2006 Alexander Chemeris
          //
          // Redistribution and use in source and binary forms, with or without
          // modification, are permitted provided that the following conditions are met:
          //
          // 1. Redistributions of source code must retain the above copyright notice,
          // this list of conditions and the following disclaimer.
          //
          // 2. Redistributions in binary form must reproduce the above copyright
          // notice, this list of conditions and the following disclaimer in the
          // documentation and/or other materials provided with the distribution.
          //
          // 3. The name of the author may be used to endorse or promote products
          // derived from this software without specific prior written permission.
          //
          // THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
          // WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
          // MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
          // EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
          // SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
          // PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
          // OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
          // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
          // OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
          // ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
          //
          ///////////////////////////////////////////////////////////////////////////////

          The second file is available at http://www.azillionmonkeys.com/qed/pstdint.h and I have brought it in as the file lib/cpp/src/pstdint.h. It has the following notice:

          ****************************************************************************

          • BSD License:
            ****************************************************************************
            *
          • Copyright (c) 2005-2007 Paul Hsieh
          • All rights reserved.
          • Redistribution and use in source and binary forms, with or without
          • modification, are permitted provided that the following conditions
          • are met:
          • 1. Redistributions of source code must retain the above copyright
          • notice, this list of conditions and the following disclaimer.
          • 2. Redistributions in binary form must reproduce the above copyright
          • notice, this list of conditions and the following disclaimer in the
          • documentation and/or other materials provided with the distribution.
          • 3. The name of the author may not be used to endorse or promote products
          • derived from this software without specific prior written permission.
          • THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
          • IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
          • OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
          • IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
          • INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
          • NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
          • DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
          • THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
          • (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
          • THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
            *
            ****************************************************************************

          I did not add an Apache license header to either file. I do not know what should be done. They can obviously be used, but there must be some requirement for dual licensing.

          To apply the patch, do the following:
          ------------------------------------
          1. Checkout from SVN:
          svn co --revision 818530 http://svn.apache.org/repos/asf/incubator/thrift/trunk thrift

          Note the --revision argument. The patch might work against other revisions, or it might not.

          2. Make a copy of your distribution directory, because patch will patch the directory it works on in place.

          3. Get the patch file from Jira by following this link:
          https://issues.apache.org/jira/browse/THRIFT-591

          Use the links there to download the patch zip file, which is named ThriftMsvcPatchForSvnRev818530.txt.zip.

          4. Unzip the patch file

          5. cd to the renamed distribution directory

          6. Apply the patch by execution:
          patch -p 8 <patchFile

          where patchFile is the path to your downloaded patch file.

          After applying the patch, the top level README file will have information about the new "msvc" subdirectory. The new file CPP_LIB_CFG has information about the configurations that are possible on *nix systems. Configurations can keep things the way they are currently, or can add-to and/or replace existing functionality with new functionality provided by Asio/Boost. (I had to do it this way so that I could use the existing test code with my new classes.)

          The "msvc" subdirectory has its own README file that contains information particular to the msvc port. There are other msvc subdirectories spread through the tree. They follow a pattern, and they are all documented in the new README.

          Distribution with Patch Pre-Applied:
          -----------------------------------
          It's pretty hard to apply the patch if all you have is a Windows system. (Get a Mac and use Boot Camp!) To help those poor souls, I have also attached a zip file that contains the 818530 svn checkout with the patch applied. Just download it to your Windows box and unzip it. It is named thrift-818530-patched.zip.

          Tested Platforms:
          ----------------
          Mac OS X 10.5.7, 10.5.8, and 10.6.1 (Snow Leopard) (me)
          Gentoo Linux (Bruce Simpson)
          FreeBSD (Bruce Simpson)
          Windows XP (me)

          Tested Boost and Asio versions:
          ------------------------------
          Standalone Asio 1.2.0 and 1.4.1
          On the Mac: Boost 1.36.0, 1.37.0, 1.38.0, and 1.39.0
          On Windows XP: Boost 1.36.0

          Along with the patch, I will attach a zip file called "MsvcPatchSupportScripts.zip" that contains a number of scripts that are useful for testing all of the variations (there are 10) on a *nix system. These scripts assume a very specific directory structure. It is documented in only one of them. Sorry. If you run "./buildAll.sh --help" at the command line, it will show you what is expected. It also documents two environment variables you can use with the scripts. They are called THRIFT_DEV_ROOT and THRIFT_DISTRO_DIR. Note that the scripts are designed to be run from the directory ABOVE your patched distribution directory.

          The various scripts are:
          buildAll.sh - Builds all the possible configuration variants by copying the distribution directory, then configuring and building. If you are on a Mac, be sure to copy your pkg.m4 file into the aclocal subdirectory of the distribution tree before you run this script. The build results are written into files called Log_* in the same directory as the script.

          buildTutorialAll.sh - After buildAll.sh has run, this script iterates over all of the directories that were created and verifies that the C++ tutorial can be built both using the auto-generated Makefile and the Tutorial.mk makefile. It cleans up after itself.

          clean.sh - Finds and removes all of the subdirectory trees and log files that are written by buildAll.sh.

          concurrencyTest.sh - After buildAll.sh has run, this script iterates over all the directories that were created, runs the concurrency test in each of them and verifies that the test runs successfully.

          cpCfgAndBuild.sh - This script knows how to configure and build Thrift in a single directory, but all it cares about is the C++ stuff. It also runs "make check". This script is used by the buildAll.sh script. This script currently defines the macro THRIFT_ENABLE_CONFIG_WARNINGS via CPPFLAGS. That generates warning messages at library compile time that tell you about the configuration. Remove the macro definition from the script if you don't want the warnings. (Just search for the macro name in the script file. What to do is pretty obvious once you find it.)

          doTest.sh - This script runs the standard "make check" test suite on a single directory. It is used by the testAll.sh script.

          findThriftDistro.sh - This script attempts to locate the Thrift distribution directory. It looks at the directory passed as the script argument if there is one, or it examines the environment variables THRIFT_DEV_ROOT and THRIFT_DISTRO_DIR if they are set. This script is used by many of the others, and you shouldn't need to mess with it.

          testAll.sh - After buildAll.sh has run, this script iterates over all of the directories that were created and runs the standard test suite in each of them.

          I hope others find this to be useful.

          Show
          Rush Manbert added a comment - This patch makes the C++ runtime library buildable in Windows using Visual Studio. No fake Unix environment is needed. It uses Boost and Asio to add work-alike functionality for the existing POSIX-based sockets and thread code. It additionally adds support for Unix Domain Sockets and Windows Named Pipes, so that local client/server communication can be done without the use of localhost. This can simplify the use of Thrift on a single machine because it doesn't run into problems with firewalls, etc. It can also be much faster than using the TCP stack. Note that all of the new socket and thread code acts exactly like the existing implementations. On a *nix platform, the new socket code can either exist alongside or replace the existing code. The new thread implementation using Boost can only replace the POSIX implementation. The new code does NOT add asynchronous client/server communication. The first two iterations of this patch were originally attached to JIRA-311. This new version, for JIRA-591, supersedes those, and is applicable to the near-latest Thrift distribution tree. These comments started as a duplicate of my original comment for 311, but they have been modified so if you're interested, please read through them. The changes from the first patch version are: Added the ability to select static or DLL runtime library linkage on Windows Added the ability to specify the Boost and Zlib name patterns on a per-config basis for Windows Added the ability to select stdint.h implementation on Windows Fixed some really stupid bugs in formatted output that I had written. Since Windows doesn't define the PRId64 and PRIu64 formatting strings, I tried using the %j sprintf format strings, casting all the values to be output to either intmax_t or uintmax_t. I was sure that I tested that successfully in Windows before I did my first patch, but I must have been hallucinating, because when I ran the test this time all I saw printed was"ju", not a number. So I undid all that and just defined macros that work for Windows. These were mainly in the CppClient and CppServer test programs, but there was also one in TFileTransport.cpp. The patch file was generated against the SVN repository revision 818530. It patches the entire distribution tree, but only touches things that are C++ related. The patch addresses the following unresolved Jira issues: Thrift-456 ( https://issues.apache.org/jira/browse/THRIFT-456 ) Thrift-457 ( https://issues.apache.org/jira/browse/THRIFT-457 ) Thrift-487 ( https://issues.apache.org/jira/browse/THRIFT-487 ) Thrift-488 ( https://issues.apache.org/jira/browse/THRIFT-488 ) It also makes changes that are not issues in Jira. For instance: 1. Autoconf support has been extended to all of the C++ tests, as well as the tutorial. 2. The stress-test and stress-test-nb have been extensively reworked to make them more versatile and reliable. I relied on these a lot during my development to test the new socket and server code. 3. The concurrency test has been extensively reworked to make it be reliable and more versatile. I work on a 8 core machine and the test as distributed just doesn't work reliably in that environment. I used this test to verify all of my Boost threading changes. 4. All of the sources have been touched to add Windows-specific support, but I suppose that could be seen as part of this Jira issue. Disclaimers: ----------- I had never done anything with autoconf before this. What I have done may not be correct. If changes are required, please let me know. The configure script displays information at the end that is related to the new configuration stuff. It was useful for me while developing and I thought it might be a good idea to leave it in for a while. I am really not a Windows guy, I just do what needs doing. If any of you are Windows experts, and you have suggestions or corrections, please let me know. See the "Known Issues" section in the msvc/README file. Licensing: --------- Obviously, I checked the box that says I grant the license to Apache for this code. I also added the Apache license header to all my new files. However. there are two files that I brought into the source tree that were already open-sourced, with license headers of their own. The first file is available at http://msinttypes.googlecode.com/svn/trunk/stdint.h , and I have brought it in as the file lib/cpp/src/VisualStudioStdint.h. It has the following notice: // ISO C9x compliant stdint.h for Miscrosoft Visual Studio // Based on ISO/IEC 9899:TC2 Committee draft (May 6, 2005) WG14/N1124 // // Copyright (c) 2006 Alexander Chemeris // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are met: // // 1. Redistributions of source code must retain the above copyright notice, // this list of conditions and the following disclaimer. // // 2. Redistributions in binary form must reproduce the above copyright // notice, this list of conditions and the following disclaimer in the // documentation and/or other materials provided with the distribution. // // 3. The name of the author may be used to endorse or promote products // derived from this software without specific prior written permission. // // THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED // WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF // MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO // EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, // SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, // PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; // OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, // WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR // OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF // ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. // /////////////////////////////////////////////////////////////////////////////// The second file is available at http://www.azillionmonkeys.com/qed/pstdint.h and I have brought it in as the file lib/cpp/src/pstdint.h. It has the following notice: **************************************************************************** BSD License: **************************************************************************** * Copyright (c) 2005-2007 Paul Hsieh All rights reserved. Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. 3. The name of the author may not be used to endorse or promote products derived from this software without specific prior written permission. THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * **************************************************************************** I did not add an Apache license header to either file. I do not know what should be done. They can obviously be used, but there must be some requirement for dual licensing. To apply the patch, do the following: ------------------------------------ 1. Checkout from SVN: svn co --revision 818530 http://svn.apache.org/repos/asf/incubator/thrift/trunk thrift Note the --revision argument. The patch might work against other revisions, or it might not. 2. Make a copy of your distribution directory, because patch will patch the directory it works on in place. 3. Get the patch file from Jira by following this link: https://issues.apache.org/jira/browse/THRIFT-591 Use the links there to download the patch zip file, which is named ThriftMsvcPatchForSvnRev818530.txt.zip. 4. Unzip the patch file 5. cd to the renamed distribution directory 6. Apply the patch by execution: patch -p 8 <patchFile where patchFile is the path to your downloaded patch file. After applying the patch, the top level README file will have information about the new "msvc" subdirectory. The new file CPP_LIB_CFG has information about the configurations that are possible on *nix systems. Configurations can keep things the way they are currently, or can add-to and/or replace existing functionality with new functionality provided by Asio/Boost. (I had to do it this way so that I could use the existing test code with my new classes.) The "msvc" subdirectory has its own README file that contains information particular to the msvc port. There are other msvc subdirectories spread through the tree. They follow a pattern, and they are all documented in the new README. Distribution with Patch Pre-Applied: ----------------------------------- It's pretty hard to apply the patch if all you have is a Windows system. (Get a Mac and use Boot Camp!) To help those poor souls, I have also attached a zip file that contains the 818530 svn checkout with the patch applied. Just download it to your Windows box and unzip it. It is named thrift-818530-patched.zip. Tested Platforms: ---------------- Mac OS X 10.5.7, 10.5.8, and 10.6.1 (Snow Leopard) (me) Gentoo Linux (Bruce Simpson) FreeBSD (Bruce Simpson) Windows XP (me) Tested Boost and Asio versions: ------------------------------ Standalone Asio 1.2.0 and 1.4.1 On the Mac: Boost 1.36.0, 1.37.0, 1.38.0, and 1.39.0 On Windows XP: Boost 1.36.0 Along with the patch, I will attach a zip file called "MsvcPatchSupportScripts.zip" that contains a number of scripts that are useful for testing all of the variations (there are 10) on a *nix system. These scripts assume a very specific directory structure. It is documented in only one of them. Sorry. If you run "./buildAll.sh --help" at the command line, it will show you what is expected. It also documents two environment variables you can use with the scripts. They are called THRIFT_DEV_ROOT and THRIFT_DISTRO_DIR. Note that the scripts are designed to be run from the directory ABOVE your patched distribution directory. The various scripts are: buildAll.sh - Builds all the possible configuration variants by copying the distribution directory, then configuring and building. If you are on a Mac, be sure to copy your pkg.m4 file into the aclocal subdirectory of the distribution tree before you run this script. The build results are written into files called Log_* in the same directory as the script. buildTutorialAll.sh - After buildAll.sh has run, this script iterates over all of the directories that were created and verifies that the C++ tutorial can be built both using the auto-generated Makefile and the Tutorial.mk makefile. It cleans up after itself. clean.sh - Finds and removes all of the subdirectory trees and log files that are written by buildAll.sh. concurrencyTest.sh - After buildAll.sh has run, this script iterates over all the directories that were created, runs the concurrency test in each of them and verifies that the test runs successfully. cpCfgAndBuild.sh - This script knows how to configure and build Thrift in a single directory, but all it cares about is the C++ stuff. It also runs "make check". This script is used by the buildAll.sh script. This script currently defines the macro THRIFT_ENABLE_CONFIG_WARNINGS via CPPFLAGS. That generates warning messages at library compile time that tell you about the configuration. Remove the macro definition from the script if you don't want the warnings. (Just search for the macro name in the script file. What to do is pretty obvious once you find it.) doTest.sh - This script runs the standard "make check" test suite on a single directory. It is used by the testAll.sh script. findThriftDistro.sh - This script attempts to locate the Thrift distribution directory. It looks at the directory passed as the script argument if there is one, or it examines the environment variables THRIFT_DEV_ROOT and THRIFT_DISTRO_DIR if they are set. This script is used by many of the others, and you shouldn't need to mess with it. testAll.sh - After buildAll.sh has run, this script iterates over all of the directories that were created and runs the standard test suite in each of them. I hope others find this to be useful.
          Hide
          Rush Manbert added a comment -

          The patch file, the patched distribution file, and the support scripts file.

          Show
          Rush Manbert added a comment - The patch file, the patched distribution file, and the support scripts file.
          Hide
          Carl Steinbach added a comment -

          Is this ticket still valid? Instructions for installing the Thrift compiler and runtime on Win32 are located here: http://wiki.apache.org/thrift/ThriftInstallationWin32

          Show
          Carl Steinbach added a comment - Is this ticket still valid? Instructions for installing the Thrift compiler and runtime on Win32 are located here: http://wiki.apache.org/thrift/ThriftInstallationWin32
          Hide
          Rush Manbert added a comment -

          It is as far as I'm concerned. Those instructions are for those who choose to go the Cygwin/Mingw route. What I wrote is pure Visual Studio compatible.

          I know of two users beside us and when I get some time (we're trying to get our product out the door that uses this code) I'm going to update the patch for whatever the current Thrift release version is and start seeing if I can get it accepted back into the tree.

          Show
          Rush Manbert added a comment - It is as far as I'm concerned. Those instructions are for those who choose to go the Cygwin/Mingw route. What I wrote is pure Visual Studio compatible. I know of two users beside us and when I get some time (we're trying to get our product out the door that uses this code) I'm going to update the patch for whatever the current Thrift release version is and start seeing if I can get it accepted back into the tree.
          Hide
          Rush Manbert added a comment -

          These are modified files that fix 2 bugs I found in the code patch for Thrift revision 818530.

          Thrift.h fhas a really stupid bug that will corrupt the stack when an error is reported in a Unix environment. I know that this patch is intended for use on Windows, but it fully supports Unix as well and we use it there. I don't know why I ever changed the buffer length, but I did and it was wrong and I'm sorry.

          The two server socket files fix a race condition in the case where a server is run as a separate thread in an application, and the server close() method is called, or if the process finishes and the server socket destructor is called. I have never seen this problem when running the Thrift stress test, which explicitly closes the server socket connection in my version, but we saw a problem here in a very unusual situation where the client side was doing something strange at the same time.

          You should replace your patched files with these versions. My apologies fort any inconvenience.

          Show
          Rush Manbert added a comment - These are modified files that fix 2 bugs I found in the code patch for Thrift revision 818530. Thrift.h fhas a really stupid bug that will corrupt the stack when an error is reported in a Unix environment. I know that this patch is intended for use on Windows, but it fully supports Unix as well and we use it there. I don't know why I ever changed the buffer length, but I did and it was wrong and I'm sorry. The two server socket files fix a race condition in the case where a server is run as a separate thread in an application, and the server close() method is called, or if the process finishes and the server socket destructor is called. I have never seen this problem when running the Thrift stress test, which explicitly closes the server socket connection in my version, but we saw a problem here in a very unusual situation where the client side was doing something strange at the same time. You should replace your patched files with these versions. My apologies fort any inconvenience.
          Hide
          kirill M added a comment -

          Hello, Rush.
          Do you think this patch will be applicable for thrift 0.4.0.
          Or, could you provide patched 0.4.0 files to build runtime c++ thrift libs with VS 2005.
          Thanks.

          Show
          kirill M added a comment - Hello, Rush. Do you think this patch will be applicable for thrift 0.4.0. Or, could you provide patched 0.4.0 files to build runtime c++ thrift libs with VS 2005. Thanks.
          Hide
          Richard Swift added a comment -

          HI Rush,

          I too am interested in this patch. I'm evaluating Thrift for inclusion in a new product and much prefer the version with your patches. I'm about to update the patch to work with the current version but wanted to know if you'd already done this before I spent the time. I'm also curious about your thoughts on the possibility of getting this included in the trunk.

          /richard

          Show
          Richard Swift added a comment - HI Rush, I too am interested in this patch. I'm evaluating Thrift for inclusion in a new product and much prefer the version with your patches. I'm about to update the patch to work with the current version but wanted to know if you'd already done this before I spent the time. I'm also curious about your thoughts on the possibility of getting this included in the trunk. /richard
          Hide
          Bryan Duxbury added a comment -

          If the patch gets synced to trunk, and everyone agrees it's working well, I'll commit it.

          Show
          Bryan Duxbury added a comment - If the patch gets synced to trunk, and everyone agrees it's working well, I'll commit it.
          Hide
          Steven Knight added a comment -

          Richard--

          I spent some time about a month ago updating Rush' patch for the API changes in Thrift 0.4.0. I got it to the point of building successfully on Linux (and Mac), but I got pulled off onto other priorities before I could test it thoroughly. I also actually didn't get to the MSVC side of things yet, so there may still be some updating necessary there. Nevertheless, I'm attaching a patch of what I currently have, in the hopes that it may save you some work updating the code.

          Bryan--

          +1 for including this patch once the consensus is that it's in reasonable shape. This would really help our use of Thrift.

          Thanks,

          Show
          Steven Knight added a comment - Richard-- I spent some time about a month ago updating Rush' patch for the API changes in Thrift 0.4.0. I got it to the point of building successfully on Linux (and Mac), but I got pulled off onto other priorities before I could test it thoroughly. I also actually didn't get to the MSVC side of things yet, so there may still be some updating necessary there. Nevertheless, I'm attaching a patch of what I currently have, in the hopes that it may save you some work updating the code. Bryan-- +1 for including this patch once the consensus is that it's in reasonable shape. This would really help our use of Thrift. Thanks,
          Hide
          Roger Meier added a comment -

          I would really like to have C++ Thrift runtime library on Windows, so I applied the patch MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch on Revision: 1005211.

          But it does not apply cleanly, see THRIFT-591_MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch_error.log

          From my perspective this is a must for a 2.3 MB patch, so currently a -1 from my side.

          issues and questions:

          • patch should apply without any issue
          • gen-cpp folders should not be a part of Thrift source
          • why a new top level folder (msvc) for a language specific thing? e.g similar lib/csharp/ThriftMSBuildTask/
          • what about splitting the patch into several separate features? e.g.
            • Local socket types, we have Unix Socket (THRIFT-900)
            • boost asio support
            • msvc support
            • msvc tutorial
            • probably there are better options to split...just a proposal
          • etc.
          Show
          Roger Meier added a comment - I would really like to have C++ Thrift runtime library on Windows, so I applied the patch MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch on Revision: 1005211. But it does not apply cleanly, see THRIFT-591_MSVCSupportUpdatedButUntestedForThrift-0.4.0.patch_error.log From my perspective this is a must for a 2.3 MB patch, so currently a -1 from my side. issues and questions: patch should apply without any issue gen-cpp folders should not be a part of Thrift source why a new top level folder (msvc) for a language specific thing? e.g similar lib/csharp/ThriftMSBuildTask/ what about splitting the patch into several separate features? e.g. Local socket types, we have Unix Socket ( THRIFT-900 ) boost asio support msvc support msvc tutorial probably there are better options to split...just a proposal etc.
          Hide
          Steven Knight added a comment -

          Roger--

          My fault for bad wording.

          When I said "this patch," I really meant to say, "The work that Rush Manbert did on this JIRA issue"--that is, Visual Studio C++ support.

          As I indicated in the previous comment, what I added is not in a finished or tested state, as I only got part way through that process before being told to work on other things. I supplied the patch here in the hope that it would help Richard get a head start, since he indicated he was about to start work on updating Rush' original patch for Thrift 0.4.0. I did not mean to imply I thought the patch was ready to apply as-is to the code base.

          Sorry for the confusion,

          Show
          Steven Knight added a comment - Roger-- My fault for bad wording. When I said "this patch," I really meant to say, "The work that Rush Manbert did on this JIRA issue"--that is, Visual Studio C++ support. As I indicated in the previous comment, what I added is not in a finished or tested state, as I only got part way through that process before being told to work on other things. I supplied the patch here in the hope that it would help Richard get a head start, since he indicated he was about to start work on updating Rush' original patch for Thrift 0.4.0. I did not mean to imply I thought the patch was ready to apply as-is to the code base. Sorry for the confusion,
          Hide
          Richard Swift added a comment -

          Thanks to everyone for the rapid responses and thanks to Steve for the leg up. As I mentioned, Thrift is still under evaluation for us and, although things are looking very good so far (Thanks Thrift team), I won't be able to put any concerted effort into this until we make a decision. In the interim, time permitting I will take Steve's work and use it as my starting point.

          I agree with Roger's comments and will take them into consideration going forwards. The breakout he suggests makes sense to me and I'll watch this thread for additional comments on that topic. I assume that the initial direction taken by Rush for the first two bullets is satisfactory.

          Finally, If anyone else has the time and wants to claim some part that would be great. I'm new to Thrift and only recently returned to Windows development so I won't be as quick as someone with a smaller learning curve From our perspective, removing the Cygwin/MinGW dependencies is the critical part. the MSVC stuff is just nice-to-have.

          Also new to the process. If there are specific steps to follow please point me to them. I'll go and look over the docs/wiki when I have a chance.

          Thanks,

          /richard

          Show
          Richard Swift added a comment - Thanks to everyone for the rapid responses and thanks to Steve for the leg up. As I mentioned, Thrift is still under evaluation for us and, although things are looking very good so far (Thanks Thrift team), I won't be able to put any concerted effort into this until we make a decision. In the interim, time permitting I will take Steve's work and use it as my starting point. I agree with Roger's comments and will take them into consideration going forwards. The breakout he suggests makes sense to me and I'll watch this thread for additional comments on that topic. I assume that the initial direction taken by Rush for the first two bullets is satisfactory. Finally, If anyone else has the time and wants to claim some part that would be great. I'm new to Thrift and only recently returned to Windows development so I won't be as quick as someone with a smaller learning curve From our perspective, removing the Cygwin/MinGW dependencies is the critical part. the MSVC stuff is just nice-to-have. Also new to the process. If there are specific steps to follow please point me to them. I'll go and look over the docs/wiki when I have a chance. Thanks, /richard
          Hide
          Rush Manbert added a comment -

          It's nice to see that the interest in this is picking up and I really appreciate people trying to bring it up to date.

          I'm going to address Roger Meier's comments first.

          If the goal is to see how it works, I would suggest applying the original patch, plus the supplemental bugfix patch, to the version of the software that it was intended for. That should apply cleanly. Then you could use the supplied test scripts to build and test it in all the variations. If you then configured it and ran make check on Windows, you would see the tests get run on that platform.

          As to the organization and the top level msvc folder, I copied the pattern I saw in other cross platform distros that we use (libxml2, libxslt, sqlite, zlib). Usually those have platform-specific directories at the top level. It can be reorganized, but I also put msvc folders everywhere in the tree that I needed a Visual Studio project, in order to keep them near the makefiles to which they correspond. That top level msvc directory contains the top level Visual Studio project that the top level make uses.

          I would not split the patch into separate features. The only reason to do that is if we wanted to keep around a bunch of separate patches that could be independently applied to the base Thrift sources in order to add features. But I think that it would be far better to get the result of the patch committed to the trunk. Then no one needs to apply patches to get an ASIO implementation, or to add Visual Studio support. (I also think that maintaining layered patches seems like a very difficult proposition.)

          Now for more general comments.

          At this point it's been more than a year since I submitted the original patch. At that time, it added a complete ASIO implementation of the Transport and Server subsystems to Thrift. To make it palatable to the Facebook guys, it was written in such a way that you could choose which implementation you used on a *nix platform. That way they could still use the original code. Someone who was going to use the Windows code might opt to use the ASIO implementation on their *nix platform because they wanted to use the same code across the board. Committing it to the trunk then would have been really easy.

          Now a year has passed. I have been very busy at my job, and I haven't have time to keep up with the changes here, so I really don't know the extent of the C++ library changes in that time. I know that a couple of batches of changes have come in from Facebook. People are writing async clients and servers. Someone has been fixing up the http stuff. I read this in an email n the developer thread yesterday:
          "I'm hoping to get time and permission to begin contributing some changes
          we have. One of them is a boost::asio/boost::bind based async c++
          implementation."
          I'm guessing that this isn't based on my ASIO implementation.

          There is the basic task of updating the code to match the current release. But after that, there is a continuing maintenance burden that people need to think about. At the very least, all new C++ library code will need to support the original Unix-specific version, plus the ASIO-based version in at least the configuration that uses ASIO plus Boost threads, plus the Windows version in at least one configuration. What should really be true is that the code supports the Unix-specific, ASIO, and Windows versions in ALL configurations. This will be a problem because the people who do the most work on the C++ library are from Facebook and, as near as I can tell, they will only care about the Unix-specific implementation because that's all they use. This is not a criticism, I'm just observing how things seem to be.

          We could simplify the permutations by getting rid of the Unix-specific implementation entirely, in favor of the ASIO implementation. But that would be bad for Facebook. They need the best speed they can get, which is probably the Unix-specific implementation.

          I know how things go at my real job, and I know that I don't have the bandwidth to be the one who makes sure that every change works across all platforms and configurations. This code only got written because we needed it and no one else had done the work. My company paid my salary while I worked on this full time, but that was probably a one-shot deal. We're just too small to be able to afford to have me be the maintainer.

          I guess the bottom line is this. I worry that the ASIO and Windows code will just not be kept up with the current state of the Unix-specific code, even if the patch result is brought into the trunk. Any comments?

          Show
          Rush Manbert added a comment - It's nice to see that the interest in this is picking up and I really appreciate people trying to bring it up to date. I'm going to address Roger Meier's comments first. If the goal is to see how it works, I would suggest applying the original patch, plus the supplemental bugfix patch, to the version of the software that it was intended for. That should apply cleanly. Then you could use the supplied test scripts to build and test it in all the variations. If you then configured it and ran make check on Windows, you would see the tests get run on that platform. As to the organization and the top level msvc folder, I copied the pattern I saw in other cross platform distros that we use (libxml2, libxslt, sqlite, zlib). Usually those have platform-specific directories at the top level. It can be reorganized, but I also put msvc folders everywhere in the tree that I needed a Visual Studio project, in order to keep them near the makefiles to which they correspond. That top level msvc directory contains the top level Visual Studio project that the top level make uses. I would not split the patch into separate features. The only reason to do that is if we wanted to keep around a bunch of separate patches that could be independently applied to the base Thrift sources in order to add features. But I think that it would be far better to get the result of the patch committed to the trunk. Then no one needs to apply patches to get an ASIO implementation, or to add Visual Studio support. (I also think that maintaining layered patches seems like a very difficult proposition.) Now for more general comments. At this point it's been more than a year since I submitted the original patch. At that time, it added a complete ASIO implementation of the Transport and Server subsystems to Thrift. To make it palatable to the Facebook guys, it was written in such a way that you could choose which implementation you used on a *nix platform. That way they could still use the original code. Someone who was going to use the Windows code might opt to use the ASIO implementation on their *nix platform because they wanted to use the same code across the board. Committing it to the trunk then would have been really easy. Now a year has passed. I have been very busy at my job, and I haven't have time to keep up with the changes here, so I really don't know the extent of the C++ library changes in that time. I know that a couple of batches of changes have come in from Facebook. People are writing async clients and servers. Someone has been fixing up the http stuff. I read this in an email n the developer thread yesterday: "I'm hoping to get time and permission to begin contributing some changes we have. One of them is a boost::asio/boost::bind based async c++ implementation." I'm guessing that this isn't based on my ASIO implementation. There is the basic task of updating the code to match the current release. But after that, there is a continuing maintenance burden that people need to think about. At the very least, all new C++ library code will need to support the original Unix-specific version, plus the ASIO-based version in at least the configuration that uses ASIO plus Boost threads, plus the Windows version in at least one configuration. What should really be true is that the code supports the Unix-specific, ASIO, and Windows versions in ALL configurations. This will be a problem because the people who do the most work on the C++ library are from Facebook and, as near as I can tell, they will only care about the Unix-specific implementation because that's all they use. This is not a criticism, I'm just observing how things seem to be. We could simplify the permutations by getting rid of the Unix-specific implementation entirely, in favor of the ASIO implementation. But that would be bad for Facebook. They need the best speed they can get, which is probably the Unix-specific implementation. I know how things go at my real job, and I know that I don't have the bandwidth to be the one who makes sure that every change works across all platforms and configurations. This code only got written because we needed it and no one else had done the work. My company paid my salary while I worked on this full time, but that was probably a one-shot deal. We're just too small to be able to afford to have me be the maintainer. I guess the bottom line is this. I worry that the ASIO and Windows code will just not be kept up with the current state of the Unix-specific code, even if the patch result is brought into the trunk. Any comments?
          Hide
          Richard Swift added a comment -

          Can someone from Facebook comment on Rush's last post? He raises some good questions about having two versions, unix-specific and ASIO-based, if the patch is approved and committed to the trunk. Would this work for the broader Thrift community? Is there any chance of moving Thrift onto a single multi-platofrm codebase? I think I may be able find some resources to help with the effort but need to know what the end result will look like before I ask.

          Show
          Richard Swift added a comment - Can someone from Facebook comment on Rush's last post? He raises some good questions about having two versions, unix-specific and ASIO-based, if the patch is approved and committed to the trunk. Would this work for the broader Thrift community? Is there any chance of moving Thrift onto a single multi-platofrm codebase? I think I may be able find some resources to help with the effort but need to know what the end result will look like before I ask.
          Hide
          David Reiss added a comment -

          I can confirm that we only use Thrift on Unix and would not be able to commit resources to maintaining a re-implementation of the stack or validating the asio implementation and migrating our internal use to it in favor of the existing stack. We have spent a lot of time tackling performance corner cases in the existing code and wouldn't want to have to start all over with asio.

          Does that answer your questions?

          Show
          David Reiss added a comment - I can confirm that we only use Thrift on Unix and would not be able to commit resources to maintaining a re-implementation of the stack or validating the asio implementation and migrating our internal use to it in favor of the existing stack. We have spent a lot of time tackling performance corner cases in the existing code and wouldn't want to have to start all over with asio. Does that answer your questions?
          Hide
          Bruce Lowekamp added a comment -

          I think an asio implementation should live as a parallel alternative, even if imported. Bridging between asio and non-asio (in either direction) is hard to do without a context switch unless the code is really designed for it, and so not really a great option.

          But it also wouldn't be the first example of a language having multiple transports.

          Continuous integration being fully deployed will also make such an option much safer to import.

          ---------------------

          (I accidentally sent this reply via email earlier, so reposting so everything is in one place)

          (as the author of Rush's quote)

          I remember we looked at the patch, though I didn't do it at the time and don't remember if ours was similar. We were trying to add out-of-order in addition to asynchronous, so I think it's likely significantly different.

          Regardless, I think an asio version should be written on top of the recent async/restructure that fb contributed. I don't think it would be that much work, but don't know if I'll get time soon.

          Bruce

          Show
          Bruce Lowekamp added a comment - I think an asio implementation should live as a parallel alternative, even if imported. Bridging between asio and non-asio (in either direction) is hard to do without a context switch unless the code is really designed for it, and so not really a great option. But it also wouldn't be the first example of a language having multiple transports. Continuous integration being fully deployed will also make such an option much safer to import. --------------------- (I accidentally sent this reply via email earlier, so reposting so everything is in one place) (as the author of Rush's quote) I remember we looked at the patch, though I didn't do it at the time and don't remember if ours was similar. We were trying to add out-of-order in addition to asynchronous, so I think it's likely significantly different. Regardless, I think an asio version should be written on top of the recent async/restructure that fb contributed. I don't think it would be that much work, but don't know if I'll get time soon. Bruce
          Hide
          Richard Swift added a comment -

          Thanks everyone. That answers my questions. We're in project startup mode right now so I'll be silent on this topic for a bit while we finish figuring a few things out. I'll get back with our conclusions once they are reached.

          Show
          Richard Swift added a comment - Thanks everyone. That answers my questions. We're in project startup mode right now so I'll be silent on this topic for a bit while we finish figuring a few things out. I'll get back with our conclusions once they are reached.
          Hide
          Jean-Charles Jorel added a comment - - edited

          Hi,
          We are currently investigating use of Thrift in our company as we consider it as a real valuable breakthrough in terms of sw interoperability.

          But, the fact that Thrift doesn't use natively MSVC is really blocking for adoption of this piece of technology in my company.

          So, I voted for this patch with the hope that it could be incoporated into the trunk (even if there is an ASIO + Pthread concurrent implementation) : All is a matter of testing capability to detect regressions between the 2 versions.

          Thanks for your excellent job. BR.

          Show
          Jean-Charles Jorel added a comment - - edited Hi, We are currently investigating use of Thrift in our company as we consider it as a real valuable breakthrough in terms of sw interoperability. But, the fact that Thrift doesn't use natively MSVC is really blocking for adoption of this piece of technology in my company. So, I voted for this patch with the hope that it could be incoporated into the trunk (even if there is an ASIO + Pthread concurrent implementation) : All is a matter of testing capability to detect regressions between the 2 versions. Thanks for your excellent job. BR.
          Hide
          Aditya Pandit added a comment -

          By any chance is this patched version available for thrift revision 917130. I am trying to build a windows c++ client for cassandra and cassandra's 0.6 branch uses that particular revision of thrift. Thanks.

          Show
          Aditya Pandit added a comment - By any chance is this patched version available for thrift revision 917130. I am trying to build a windows c++ client for cassandra and cassandra's 0.6 branch uses that particular revision of thrift. Thanks.
          Hide
          Rush Manbert added a comment -

          Not off the shelf. If you have access to a *nix system, you could try doing a 3-way diff/merge and see how bad it is. That's what I did when I moved between versions during development.

          Show
          Rush Manbert added a comment - Not off the shelf. If you have access to a *nix system, you could try doing a 3-way diff/merge and see how bad it is. That's what I did when I moved between versions during development.
          Hide
          Aditya Pandit added a comment -

          Thanks for your response Rush. I have been able to do a 3-way merge and build a simple test app on windows that is able to communicate with cassandra. Thanks a ton for sharing your code.

          Show
          Aditya Pandit added a comment - Thanks for your response Rush. I have been able to do a 3-way merge and build a simple test app on windows that is able to communicate with cassandra. Thanks a ton for sharing your code.
          Hide
          Todd Lipcon added a comment -

          Hey David. To clarify, you are -1 on changing over the current C++ implementation to ASIO, and also won't commit FB resources to testing/maintaining an ASIO-based implementation. But, if someone contributes a parallel set of C++ code based on ASIO that can work on Windows, as suggested by Bruce above, you're cool with that, right?

          Show
          Todd Lipcon added a comment - Hey David. To clarify, you are -1 on changing over the current C++ implementation to ASIO, and also won't commit FB resources to testing/maintaining an ASIO-based implementation. But, if someone contributes a parallel set of C++ code based on ASIO that can work on Windows, as suggested by Bruce above, you're cool with that, right?
          Hide
          David Reiss added a comment -

          Yeah. You should just be able to create a TAsioSocket and TAsioServer class and have the rest of the C++ implementation (framing, serialization, generated code) work the same.

          Show
          David Reiss added a comment - Yeah. You should just be able to create a TAsioSocket and TAsioServer class and have the rest of the C++ implementation (framing, serialization, generated code) work the same.
          Hide
          Carl Steinbach added a comment -

          Maybe there was some miscommunication earlier, because I think that's actually what this patch does. Here's an excerpt from the CPP_LIB_CFG doc that was included in the patch:

          The Thrift C++ libraries can be configured to use one or more socket and server implementations, and either of two threading implementations. The reason that this capability exists is because the code was modified in 2009 so that it could be built in Visual Studio on Windows. That effort added Asio-based socket and server implementations that exist alongside the existing implementations. It also added a Boost.thread threading implementation that can replace the pthread implementation.

          The original classes are TSocket, TServerSocket, TSocketPool, and TNonblockingServer. The Asio-based
          parallel classes are called TAsioSocket, TAsioServerSocket, TAsioSocketPool, and TAsioNonblockingServer.
          The source code headers have been set up in such a way that:

          • Both implementation sets may be available (i.e. TSocket and TAsioSocket will be separate classes)
          • The TAsio* family can be synonyms for the TSocket family (i.e. #define TAsioSocket TSocket, etc.)
          • The TSocket family can be synonyms for the TAsio* family (i.e. #define TSocket TAsioSocket, etc.)

          The Boost threading was added to support threading in Windows without Cygwin. Either the pthread
          or Boost.thread implementation may be chosed.

          So if someone were to rebase this patch to trunk, do you think it could get committed?

          Show
          Carl Steinbach added a comment - Maybe there was some miscommunication earlier, because I think that's actually what this patch does. Here's an excerpt from the CPP_LIB_CFG doc that was included in the patch: The Thrift C++ libraries can be configured to use one or more socket and server implementations, and either of two threading implementations. The reason that this capability exists is because the code was modified in 2009 so that it could be built in Visual Studio on Windows. That effort added Asio-based socket and server implementations that exist alongside the existing implementations. It also added a Boost.thread threading implementation that can replace the pthread implementation. The original classes are TSocket, TServerSocket, TSocketPool, and TNonblockingServer. The Asio-based parallel classes are called TAsioSocket, TAsioServerSocket, TAsioSocketPool, and TAsioNonblockingServer. The source code headers have been set up in such a way that: Both implementation sets may be available (i.e. TSocket and TAsioSocket will be separate classes) The TAsio* family can be synonyms for the TSocket family (i.e. #define TAsioSocket TSocket, etc.) The TSocket family can be synonyms for the TAsio* family (i.e. #define TSocket TAsioSocket, etc.) The Boost threading was added to support threading in Windows without Cygwin. Either the pthread or Boost.thread implementation may be chosed. So if someone were to rebase this patch to trunk, do you think it could get committed?
          Hide
          Rush Manbert added a comment -

          Carl is correct in general. If the patch were brought up to date and applied to the tree, then all existing C++ users could configure so that nothing changes for them. They would use the same classes as before, which would use the same underlying code. No boost threads and no ASIO involved. We configure it so that the TSocket, etc. classes are synonyms for TAsioSocket and friends, so we use the original class names but the ASIO implementation, as well as Boost threads. That's so that we run the same configurations on Mac and Windows.

          The patch also had fixes for many of the C++ tests (particularly the stress test) as well as the scheduler. Those might be somewhat controversial.

          Show
          Rush Manbert added a comment - Carl is correct in general. If the patch were brought up to date and applied to the tree, then all existing C++ users could configure so that nothing changes for them. They would use the same classes as before, which would use the same underlying code. No boost threads and no ASIO involved. We configure it so that the TSocket, etc. classes are synonyms for TAsioSocket and friends, so we use the original class names but the ASIO implementation, as well as Boost threads. That's so that we run the same configurations on Mac and Windows. The patch also had fixes for many of the C++ tests (particularly the stress test) as well as the scheduler. Those might be somewhat controversial.
          Hide
          alexandre parenteau added a comment -

          I have created a fork of latest thrift 0.6.1 on github, which comprises part of this patch, and also uses THRIFT-923 (non-blocking server with libevent). If interested, please refer to https://github.com/aubonbeurre/thrift/blob/alex-0.6.1/README.non.blocking.Windows.

          Show
          alexandre parenteau added a comment - I have created a fork of latest thrift 0.6.1 on github, which comprises part of this patch, and also uses THRIFT-923 (non-blocking server with libevent). If interested, please refer to https://github.com/aubonbeurre/thrift/blob/alex-0.6.1/README.non.blocking.Windows .
          Hide
          Jake Farrell added a comment -

          Carl: Rush: if either of you can rebase this patch set against the current trunk i will help review and get it committed

          Show
          Jake Farrell added a comment - Carl: Rush: if either of you can rebase this patch set against the current trunk i will help review and get it committed
          Hide
          Roger Meier added a comment -
          Show
          Roger Meier added a comment - see THRIFT-1123 and THRIFT-1031

            People

            • Assignee:
              Unassigned
              Reporter:
              Rush Manbert
            • Votes:
              16 Vote for this issue
              Watchers:
              23 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development