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

[Michael Gottesman]

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.

Michael

On Aug 20, 2013, at 1:45 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:

> On Aug 20, 2013, at 1:22 PM, Reid Kleckner <rnk at google.com> wrote:
> 
>> Do the autoconf makefiles automatically recurse into subdirs of tools?
> 
> I'd prefer that they do but I hope getting the CMake functionality is not blocked on autoconf.
> 
>> Don't we already recurse into llvm/projects/?  Is there any reason to do this for tools/ as well?
> 
> Currently, there are more llvm_external_projects residing in llvm/tools than llvm/projects. Unless you'd like all of them to move to projects/ I'd suggest that we do it for tools/.
> 
>> 
>> 
>> On Tue, Aug 20, 2013 at 1:13 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>> On Tue, Aug 20, 2013 at 10:50 AM, Argyrios Kyrtzidis
>> <kyrtzidis at apple.com> wrote:
>> > The attached patch allows CMake to pick up external projects in llvm/tools without the need to modify the "llvm/tools/CMakeLists.txt" file.
>> > This makes it easier to work with projects that live in other repositories, without needing to specify each one in "llvm/tools/CMakeLists.txt".
>> 
>> Not a real review, but: I like the idea and I tested the patch on
>> Linux/CMake/ninja.
>> 
>> Dmitri
>> 
>> --
>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
>> _______________________________________________
>> 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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130820/1da4ef89/attachment.html>