[clang-tools-extra] r202238 - [CMake] Use LINK_LIBS instead of target_link_libraries().

[Chandler Carruth]

I am really not certain about this approach.

One of the huge advantages of using target_link_libraries is that it isn't
yet-another magical LLVM-customized-CMake-ism that newcomers to the project
need to learn and internalize. Instead, they use the boring and expected
CMake mechanism for adding a library to the dependencies.

You don't provide *any* justification for this shift in the commit log, you
didn't ask for precommit review to gather feedback about this approach, and
you haven't replied (that I've seen) to both Rafael and my comments
expressing our concern over this approach.

Can we please actually have discussions about the direction for the cmake
build prior to making sweeping changes to its architecture?


On Tue, Feb 25, 2014 at 10:41 PM, NAKAMURA Takumi <geek4civic at gmail.com>wrote:

> Author: chapuni
> Date: Wed Feb 26 00:41:29 2014
> New Revision: 202238
>
> URL: http://llvm.org/viewvc/llvm-project?rev=202238&view=rev
> Log:
> [CMake] Use LINK_LIBS instead of target_link_libraries().
>
> Modified:
>     clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt
>     clang-tools-extra/trunk/clang-modernize/Core/CMakeLists.txt
>     clang-tools-extra/trunk/clang-query/CMakeLists.txt
>     clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
>     clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
>     clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
>
> Modified: clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt
> (original)
> +++ clang-tools-extra/trunk/clang-apply-replacements/CMakeLists.txt Wed
> Feb 26 00:41:29 2014
> @@ -4,8 +4,8 @@ set(LLVM_LINK_COMPONENTS
>
>  add_clang_library(clangApplyReplacements
>    lib/Tooling/ApplyReplacements.cpp
> -  )
> -target_link_libraries(clangApplyReplacements
> +
> +  LINK_LIBS
>    clangBasic
>    clangRewriteCore
>    clangTooling
>
> Modified: clang-tools-extra/trunk/clang-modernize/Core/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-modernize/Core/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-modernize/Core/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-modernize/Core/CMakeLists.txt Wed Feb 26
> 00:41:29 2014
> @@ -7,8 +7,8 @@ add_clang_library(modernizeCore
>    IncludeExcludeInfo.cpp
>    PerfSupport.cpp
>    IncludeDirectives.cpp
> -  )
> -target_link_libraries(modernizeCore
> +
> +  LINK_LIBS
>    clangAST
>    clangASTMatchers
>    clangBasic
>
> Modified: clang-tools-extra/trunk/clang-query/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-query/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-query/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-query/CMakeLists.txt Wed Feb 26 00:41:29
> 2014
> @@ -6,8 +6,8 @@ set(LLVM_LINK_COMPONENTS
>  add_clang_library(clangQuery
>    Query.cpp
>    QueryParser.cpp
> -  )
> -target_link_libraries(clangQuery
> +
> +  LINK_LIBS
>    clangAST
>    clangASTMatchers
>    clangBasic
>
> Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Wed Feb 26 00:41:29
> 2014
> @@ -9,8 +9,8 @@ add_clang_library(clangTidy
>
>    DEPENDS
>    ClangSACheckers
> -  )
> -target_link_libraries(clangTidy
> +
> +  LINK_LIBS
>    clangAST
>    clangASTMatchers
>    clangBasic
>
> Modified: clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/google/CMakeLists.txt Wed Feb 26
> 00:41:29 2014
> @@ -2,8 +2,8 @@ set(LLVM_LINK_COMPONENTS support)
>
>  add_clang_library(clangTidyGoogleModule
>    GoogleTidyModule.cpp
> -  )
> -target_link_libraries(clangTidyGoogleModule
> +
> +  LINK_LIBS
>    clangAST
>    clangASTMatchers
>    clangBasic
>
> Modified: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt?rev=202238&r1=202237&r2=202238&view=diff
>
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt Wed Feb 26
> 00:41:29 2014
> @@ -2,8 +2,8 @@ set(LLVM_LINK_COMPONENTS support)
>
>  add_clang_library(clangTidyLLVMModule
>    LLVMTidyModule.cpp
> -  )
> -target_link_libraries(clangTidyLLVMModule
> +
> +  LINK_LIBS
>    clangAST
>    clangASTMatchers
>    clangBasic
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140225/826f3d59/attachment.html>