[PATCH] D28086: [CMake] Subsume LLVMBuild.txt. Disabled by default.

[bryant via Phabricator via llvm-commits]

bryant added a comment.

> I don't understand why you suggest WITHOUT_LLVMBUILD. We can just prune
>  llvmbuild.

It's a flag to toggle between the old behavior (that uses LLVMBuild) and new
(that uses functionality added by this patch).

> You may do the job step-by-step,
> 
>   Let llvm-config free from llvmbuild
>   Migrate deps from LLVMBuild.txt(s)
>   Prune llvmbuild

The first two is exactly what this patch does with `WITHOUT_LLVMBUILD=ON`. The
third -- deleting llvmbuild-related files -- can be done in a separate patch
once it's been verified that this patch adequately replicates the existing
behavior in pure CMake.

> I also think LINK_COMPONENTS would be redundant since it comes from
>  compatibility for autoconf.  We may take LLVMSupport instead of Support, for
>  example.

When telling CMake to build a library LLVMxyz, there needs to be a distinction
between 1) stuff that needs to be linked into with xyz _that are themselves
LLVM*_, and 2) other stuff that xyz needs but aren't themselves LLVM*.

For instance, LLVMCodeGen requires LLVM{Analysis,BitReader,...} (first category;
call them LLVM-internal stuff) and ${PTHREAD_LIB} (second category).

This distinction is needed in order to generate LibraryDependencies.inc, which
records dependencies of the first but not the second kind.

> That said, I am not a fan of direct use of target_link_libraries. It is not
>  controllable.  I suggest to use LINK_LIBS in llvm_add_library().

You're right: It's possible to entirely deprecate
LINK_COMPONENTS/LLVM_LINK_COMPONENTS in favor of LINK_LIBS. In that case, we'd
need to look out for LINK_LIBs that match the regex `^LLVM.+$` and handle them
slightly differently for LibraryDependencies.inc. But such special handling
machinery already exists in the form of LINK_COMPONENTS.

In other words, this patch expects all LLVM-internal dependencies to be
specified in LINK_COMPONENTS and all other deps under LINK_LIBS.

Please let me know if that made sense to you.


Repository:
  rL LLVM

https://reviews.llvm.org/D28086