[ 
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)

Reply via email to