[PATCH] D14561: Add CMakeLists for External/{Nurbs|Povray|skidmarks10|HMMER}

[James Molloy via llvm-commits]

jmolloy requested changes to this revision.
jmolloy added a comment.
This revision now requires changes to proceed.

Hi Matthias,

Thanks for pushing this forward. I've got some comments below.

Cheers,

James


================
Comment at: CMakeLists.txt:59
@@ +58,3 @@
+  if(CMAKE_SIZEOF_VOID_P EQUAL 8)
+    set(ARCH_LP64 TRUE CACHE BOOL "Whether target architecture has 64bit pointers")
+  endif()
----------------
Don't we want something like this instead?:

  if(CMAKE_SIZEOF_VOID_P EQUAL 8)
    set(TMP_LP64 TRUE)
  else()
    set(TMP_LP64 FALSE)
  endif()
  set(ARCH_LP64 ${TMP_LP64} CACHE BOOL "Does the target architecture has 64-bit pointers?")

I'll defer to Chris B on this

================
Comment at: CMakeLists.txt:73
@@ -65,1 +72,3 @@
 add_subdirectory(MultiSource)
+if(EXTERNALS_DIR)
+	add_subdirectory(External)
----------------
I'd prefer if we had a "LLVM_" prefix on all variables we expect to be set by the user. Can we stick to that, like in the other projects' builds?

================
Comment at: External/CMakeLists.txt:1
@@ +1,1 @@
+llvm_add_subdirectories(HMMER Nurbs Povray skidmarks10)
----------------
I think this relies on a CMake module from LLVM, not from test-suite, so it'll fail unless we're in /projects.

Just doing "add_subdirectory()" several times isn't much harder.


http://reviews.llvm.org/D14561