You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by Ben Craig <be...@ni.com> on 2012/11/09 21:58:30 UTC

How to post a multi-issue patch?

I have a patch that I am ready to post to JIRA.  It resolves a few 
problems that have already been reported, and it makes some additional 
improvements not currently on JIRA.  However, these improvements and bug 
fixes touch a lot of files, with parts of multiple bug fixes in the same 
file.  Separating them out isn't easy. 

What is the preferred way to report these "issues" and post this patch?




These are all in the C++ Library.  Here is a summary of what I changed. 
All of these fixes make a ~5000 line patch (though a lot of that is 
deleted lines).
*   General cleanup of the msvc project
*   Using HAVE_CONFIG_H instead of force including files
*   Getting rid of some unnecessary files (stdafx.*, TargetVersion.h)
*   Significant rework of windows portability.  No longer using config.h 
and force_inc.h to make Windows look like *nix.  Instead, making lots of 
Thrift specific #defines that are vaguely *nixy, and having those forward 
to *nix or Windows stuff appropriately.  For example, THRIFT_CTIME_R calls 
ctime_r on *nix, and a wrapper thrift_ctime_r on Windows.  The old 
approach doesn't work when multiple libraries attempt the same trick.  For 
example, if openssl #defined errno to ::WSAGetLastError() as well, then 
that would cause problems.
*   Adding preprocessor flag that can optionally squelch console output. 
Default behavior is unchanged.  Console output is great for home deployed 
server apps, but it looks unprofessional for consumer apps.
*   Adding THRIFT_UNUSED_VARIABLE helper macro, to aid in squelching 
warnings.
*   Adding redirector header for <functional> and <tr1/functional>.  Since 
namespaces aren't consistent (std vs std::tr1), I have added symbols to 
the apache::thrift::stdcxx namespace.  This is important for Clang / iOS 
support
*   usleep and sleep on Windows were both sleeping in milliseconds.  sleep 
now correctly sleeps for seconds, and usleep attempts to sleep for 
microseconds (after converting very coarsely to milliseconds).
*   Adding support for using C++11 std::thread (and mutex, and monitor). 
Thrift already supported boost::thread and posix threads.  Clients that 
use std::thread no longer need built boost libraries.  The boost headers 
are sufficient for them.  Switching from boost::thread to std::thread 
resulted in a ~50k reduction in exe size in my tests.  By default, msvc10 
and below will use boost::thread, msvc11 and up will use std::thread.
*   Fixing more 64-bit socket truncation issues in non-blocking server and 
ssl support.  openssl itself has socket truncation issues, so I could not 
fix them all.
*   Fixed THRIFT-1692 "SO_REUSEADDR allows for socket hijacking on Windows 
".  Now using SO_EXCLUSIVEADDRUSE on windows, and SO_REUSEADDR on *nix.
*   Making TFileTransport use thrift style threads instead of redoing all 
the pthread+boost stuff itself.
*   Includes, and builds upon THRIFT-1740 (Make C++ library build on OS X 
and iOS)
*   Moved several functions out of thrift/windows/config.h, and into other 
thrift/windows headers.
*   Using built-in stdint.h if available (by checking HAVE_STDINT_H) and 
using boost typedefs otherwise.

Re: How to post a multi-issue patch?

Posted by Henrique Mendonça <he...@apache.org>.
Hi Ben,

You can create the ticket and post the patch (against the trunk) and we'll
have a look.
I think some of the mentioned points were already committed to the trunk.

Cheers,
Henrique

On 9 November 2012 21:58, Ben Craig <be...@ni.com> wrote:

> I have a patch that I am ready to post to JIRA.  It resolves a few
> problems that have already been reported, and it makes some additional
> improvements not currently on JIRA.  However, these improvements and bug
> fixes touch a lot of files, with parts of multiple bug fixes in the same
> file.  Separating them out isn't easy.
>
> What is the preferred way to report these "issues" and post this patch?
>
>
>
>
> These are all in the C++ Library.  Here is a summary of what I changed.
> All of these fixes make a ~5000 line patch (though a lot of that is
> deleted lines).
> *   General cleanup of the msvc project
> *   Using HAVE_CONFIG_H instead of force including files
> *   Getting rid of some unnecessary files (stdafx.*, TargetVersion.h)
> *   Significant rework of windows portability.  No longer using config.h
> and force_inc.h to make Windows look like *nix.  Instead, making lots of
> Thrift specific #defines that are vaguely *nixy, and having those forward
> to *nix or Windows stuff appropriately.  For example, THRIFT_CTIME_R calls
> ctime_r on *nix, and a wrapper thrift_ctime_r on Windows.  The old
> approach doesn't work when multiple libraries attempt the same trick.  For
> example, if openssl #defined errno to ::WSAGetLastError() as well, then
> that would cause problems.
> *   Adding preprocessor flag that can optionally squelch console output.
> Default behavior is unchanged.  Console output is great for home deployed
> server apps, but it looks unprofessional for consumer apps.
> *   Adding THRIFT_UNUSED_VARIABLE helper macro, to aid in squelching
> warnings.
> *   Adding redirector header for <functional> and <tr1/functional>.  Since
> namespaces aren't consistent (std vs std::tr1), I have added symbols to
> the apache::thrift::stdcxx namespace.  This is important for Clang / iOS
> support
> *   usleep and sleep on Windows were both sleeping in milliseconds.  sleep
> now correctly sleeps for seconds, and usleep attempts to sleep for
> microseconds (after converting very coarsely to milliseconds).
> *   Adding support for using C++11 std::thread (and mutex, and monitor).
> Thrift already supported boost::thread and posix threads.  Clients that
> use std::thread no longer need built boost libraries.  The boost headers
> are sufficient for them.  Switching from boost::thread to std::thread
> resulted in a ~50k reduction in exe size in my tests.  By default, msvc10
> and below will use boost::thread, msvc11 and up will use std::thread.
> *   Fixing more 64-bit socket truncation issues in non-blocking server and
> ssl support.  openssl itself has socket truncation issues, so I could not
> fix them all.
> *   Fixed THRIFT-1692 "SO_REUSEADDR allows for socket hijacking on Windows
> ".  Now using SO_EXCLUSIVEADDRUSE on windows, and SO_REUSEADDR on *nix.
> *   Making TFileTransport use thrift style threads instead of redoing all
> the pthread+boost stuff itself.
> *   Includes, and builds upon THRIFT-1740 (Make C++ library build on OS X
> and iOS)
> *   Moved several functions out of thrift/windows/config.h, and into other
> thrift/windows headers.
> *   Using built-in stdint.h if available (by checking HAVE_STDINT_H) and
> using boost typedefs otherwise.