Github user nsuke commented on a diff in the pull request:

    https://github.com/apache/thrift/pull/598#discussion_r38807939
  
    --- Diff: build/cmake/DefineOptions.cmake ---
    @@ -62,8 +62,10 @@ endif()
     find_package(OpenSSL QUIET)
     CMAKE_DEPENDENT_OPTION(WITH_OPENSSL "Build with OpenSSL support" ON
                            "OPENSSL_FOUND" OFF)
    -option(WITH_BOOSTTHREADS "Build with Boost thread support" OFF)
     option(WITH_STDTHREADS "Build with C++ std::thread support" OFF)
    +CMAKE_DEPENDENT_OPTION(WITH_BOOSTTHREADS "Build with Boost threads 
support" ON
    --- End diff --
    
    According to 
http://www.cmake.org/cmake/help/v3.0/module/CMakeDependentOption.html
    
    > This macro presents an option to the user only if a set of other 
conditions are true. 
    
    This would hide the option itself unless the user set it in the first 
place, like a chicken out of no egg for GUI users.
    
    How about literally changing the default value only:
    
        CMAKE_DEPENDENT_OPTION(WITH_BOOSTTHREADS "Build with Boost threads 
support" OFF
             "NOT WITH_STDTHREADS;Boost_FOUND" OFF)
    
    
    About the config.h, I think your concern is right.
    Long story short, it originates from my past patch, and in its current 
state I don't see any reason not to replace *add_definitions* with *set*.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to