[ https://issues.apache.org/jira/browse/THRIFT-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jake Farrell closed THRIFT-2014. -------------------------------- Resolution: Fixed > 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: 0.10.0 > > Attachments: 0004-Changed-C-lib-includes-to-use-namespace-form.patch > > > 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 was sent by Atlassian JIRA (v6.3.4#6332)