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

[Argyrios Kyrtzidis]

On Aug 27, 2013, at 3:36 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:

> 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.

Not quite sure what you mean, if the lld source is not located in tools, there should be no difference.
Could you elaborate ?

> 
> - Michael Spencer

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