[llvm] r242120 - [CMake] Unbreak add_llvm_external_project when external projects are specified.

[Chris Bieneman]

(cc’ing Brad in case he can shed some light where my CMake-foo is limited)

This is curious… According to the CMake documentation marking the variable as PATH has no impact on the way it is expanded, it only impacts how the variable is displayed in ccmake and cmake-gui. Maybe someone more adept at CMake can explain how this is wrong.

In general passing in relative paths as CMake variables on the command line is pretty flaky because there is no guarantee what the path will expand relative to. Should it be relative to the CMAKE_BINARY_DIRECTORY which is probably the working directory when you run cmake, or the CMAKE_SOURCE_DIRECTORY, or the CMAKE_CURRENT_SOURCE_DIRECTORY?

The only feature of CMake that I’m aware of that are designed to help make relative paths work is CMAKE_USE_RELATIVE_PATHS (http://www.cmake.org/cmake/help/v3.0/variable/CMAKE_USE_RELATIVE_PATHS.html <http://www.cmake.org/cmake/help/v3.0/variable/CMAKE_USE_RELATIVE_PATHS.html>), which right in the documentation says “May not work!”, so I would strongly advise against using it, or using relative paths at all.

If setting PATH has no impact other than the ccmake UI, I don’t think we should have this change here. Instead I think if users want to display the variable as a path chooser they should specify the type when the call into CMake, because that is the only place this value can be set (i.e. -DLLVM_EXTERNAL_*_SOURCE_DIR:PATH=…).

If PATH does have some impact on how the value is expanded, I have no objection to this change, but it still feels odd.

-Chris

> On Jul 13, 2015, at 10:12 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:
> 
> Author: chapuni
> Date: Tue Jul 14 00:12:53 2015
> New Revision: 242120
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=242120&view=rev
> Log:
> [CMake] Unbreak add_llvm_external_project when external projects are specified.
> 
> LLVM_EXTERNAL_*_SOURCE_DIR is reset as PATH with set(CACHE PATH).
> Then the CACHE PATH variable, LLVM_EXTERNAL_*_SOURCE_DIR, is normalized as
> ${CMAKE_SOURCE_DIR}/${path_var} if ${path_var} is relative.
> 
> Modified:
>    llvm/trunk/cmake/modules/AddLLVM.cmake
> 
> Modified: llvm/trunk/cmake/modules/AddLLVM.cmake
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/modules/AddLLVM.cmake?rev=242120&r1=242119&r2=242120&view=diff
> ==============================================================================
> --- llvm/trunk/cmake/modules/AddLLVM.cmake (original)
> +++ llvm/trunk/cmake/modules/AddLLVM.cmake Tue Jul 14 00:12:53 2015
> @@ -696,6 +696,9 @@ macro(add_llvm_external_project name)
>   endif()
>   if(NOT LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR)
>     set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")
> +  else()
> +    set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR
> +        CACHE PATH "Path to ${name} source directory")
>   endif()
>   if (EXISTS ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)
>     option(LLVM_EXTERNAL_${nameUPPER}_BUILD
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150714/31cbdbc4/attachment.html>