[PATCH] CMake: Automatically pick up subdirectories in llvm/tools as 'external projects' if they contain a 'CMakeLists.txt' file

[Michael Spencer]

On Wed, Aug 21, 2013 at 1:46 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>wrote:

> Committed in r188921, thanks for reviewing!
>
> On Aug 21, 2013, at 11:43 AM, Michael Gottesman <mgottesman at apple.com>
> wrote:
>
> LGTM
>
> On Aug 21, 2013, at 9:43 AM, Argyrios Kyrtzidis <kyrtzidis at apple.com>
> wrote:
>
> On Aug 20, 2013, at 11:24 PM, Michael Gottesman <mgottesman at apple.com>
> wrote:
>
> Looks much better. One last nit then LGTM. I understand why you would make
> {add,ignore}_llvm_tool_subdirectory macros since they are so small,
>
>
> Actually these need to be macros so they can introduce
> the LLVM_IMPLICIT_PROJECT_IGNORE variable.
>
> but do you have any reason why you made
> add_llvm_implicit_external_projects a macro. It seems to me to be more of a
> function since it:
>
> 1. Takes no arguments.
> 2. Sets variables which are used only locally suggesting that you want to
> have the local scope of a function instead of the global scope of a macro.
>
>
> You're right; changed add_llvm_implicit_external_projects to a function
> and attached new patch.
>
> <auto-external-proj3.diff>
>
> -Argyrios
>
>
> Michael
>
> On Aug 20, 2013, at 6:00 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com>
> wrote:
>
> On Aug 20, 2013, at 5:27 PM, Michael Gottesman <mgottesman at apple.com>
> wrote:
>
> Hey Argyrios.
>
> I like the patch, but I think that the name of the variable
> LLVM_TOOL_SUBDIRS is misleading since you are appending to it
> subdirectories that are going to explicitly not be included as well as
> those that will be. I can easily see a tired befuddled programmer getting
> confused about the meaning of said variable since it violates the
> expectations that every directory in that variable will include a tool that
> will be built (I was confused and I am only slightly tired/befuddled).
>
> The real thing you are using said variable for is to act as a signal to
> add_llvm_implicit_external_projects that it should ignore said
> subdirectory. Perhaps something like LLVM_IMPLICIT_PROJECT_IGNORE? Even
> though that name sucks, it at least makes it absolutely clear about the
> purpose of the variable.
>
>
> Yes, that is better, thanks!
>
> Attached new patch.
>
> <auto-external-proj2.diff>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
This breaks my usecase of not having lld located in the tools directory.

- Michael Spencer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130827/db9309c7/attachment.html>