On 05/12/2016 03:49 PM, Roger Leigh wrote:
> I have made the suggested changes above where this was possible, and 
> merged the boost-component-headers branch into next for testing.

Thanks.  It looks pretty good, but there is one problem:

> +    find_path(Boost_${UPPERCOMPONENT}_HEADER
> +      NAMES         ${Boost_${UPPERCOMPONENT}_HEADER_NAME}
> +      HINTS         ${Boost_INCLUDE_DIR}

This leaves a bunch of Boost_${UPPERCOMPONENT}_HEADER values in
the cache.  The names look public, and they are publicly visible.
We should not expose this implementation detail.

Also, the find_path() approach means it might find the header somewhere
other than Boost_INCLUDE_DIR.  If it is not there, we cannot be confident
that it will match the library found.

Can the check use just if(EXISTS) instead?

Thanks,
-Brad

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to