[THRIFT-2014] Change C++ lib includes to use <namespace/> style throughout

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 0.10.0
• Component/s: C++ - Library

Environment

All

• Assignee: Randy Abernethy
• Reporter: Randy Abernethy
• Created: 10/Jun/13 06:14
• Updated: 20/Feb/21 15:26
• Resolved: 08/Apr/16 05:45

Description

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?

Issue Links

• is part of: THRIFT-1552 Thrift (cpp) include files shouldn't require adding "thrift" to the compilers include path.