Randy Abernethy created THRIFT-2014:
---------------------------------------
Summary: Change C++ lib includes to use <namespace/> style
throughout
Key: THRIFT-2014
URL: https://issues.apache.org/jira/browse/THRIFT-2014
Project: Thrift
Issue Type: Improvement
Components: C++ - Library
Affects Versions: 1.0
Environment: All
Reporter: Randy Abernethy
Assignee: Randy Abernethy
Priority: Minor
Fix For: 1.0
Use #include<thrift/...> style include statements throughout C++ library (see
Exception below)
Rational: Improve consistency and predictability in C++ library and user
builds. Because Apache Thrift is a library, system style includes (<>) may be a
better fit than local style includes (“”). System style includes are presently
used predominantly (4:1) but not exclusively and the choice between the two
often appears to have no pattern. Making the library’s approach <thrift/>
consistent will improve build predictability (e.g local copies of include files
will not be picked up). Using explicit thrift relative paths (i.e. namespaces
like thrift/transport) also adds clarity and specificity (e.g. two files with
the same name can exist in separate directories and can be included with
obvious and predictable results, avoiding most path order dependencies). The “”
include is commonly designated as a user include (e.g.
gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html#Include-Syntax), the search
pattern for “” tends to vary from compiler to compiler a bit also.
bg: POSIX compliant compilers (and many others) search only –I directories and
then standard directories for system #include<> dependencies (good for
libraries). A local #include “” causes local directories (typically the
location of the including file) to be searched, then the –I, then built-in
paths. The POSIX spec is not adhered to by all platforms/compilers and more
complex behavior is common, making "" tricky on occasion.
Current situation:
447 instances of <thrift/...> e.g.:
lib/cpp/src/thrift/transport/TFileTransport.h:#include
<thrift/transport/TTransport.h>
lib/cpp/src/thrift/transport/TTransportUtils.h:#include
<thrift/transport/TTransport.h>
116 instances of “file.h” e.g.:
lib/cpp/src/thrift/transport/TSocket.h:#include "TTransport.h"
//Same header with “” and <> styles in different files
lib/cpp/src/thrift/transport/TFileTransport.cpp:#include "TTransportUtils.h"
//Single file including thrift headers in different ways (“” and <>)
Exceptions: A header presented with local style (“”) delimiters is a hint
indicating that the header is designed to be locally defined or potentially
overloaded. The config.h header (associated with HAVE_CONFIG_H) may have this
“locally defined” semantic. The config.h is also included with <> and “” in
different locations. Assuming we want people to be able to locally override
config.h, #include “config.h” is the right answer. That said config.h is a
pretty generic name to be including with "config.h" safely. If only the
config.h in the thrift root should be allowed then <thrift/config.h> may be the
right answer. Thoughts?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
