[PATCH] D13215: [CMake] Make the bindir and libdir arguments to set_output_directory optional

[Chris Bieneman via llvm-commits]

beanz added a comment.

I think all you need to do for handling unset values is something like this:

  diff --git a/cmake/modules/AddLLVM.cmake b/cmake/modules/AddLLVM.cmake
  index 993bc35..63047e6 100644
  --- a/cmake/modules/AddLLVM.cmake
  +++ b/cmake/modules/AddLLVM.cmake
  @@ -211,17 +211,33 @@ function(set_output_directory target)
     if(NOT "${CMAKE_CFG_INTDIR}" STREQUAL ".")
       foreach(build_mode ${CMAKE_CONFIGURATION_TYPES})
         string(TOUPPER "${build_mode}" CONFIG_SUFFIX)
  -      string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} bi ${ARG_BINDIR})
  -      string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} li ${ARG_LIBDIR})
  -      string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} mi ${moddir})
  -      set_target_properties(${target} PROPERTIES "RUNTIME_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${bi})
  -      set_target_properties(${target} PROPERTIES "ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${li})
  -      set_target_properties(${target} PROPERTIES "LIBRARY_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${mi})
  +      if(ARG_BINDIR)
  +        string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} bi ${ARG_BINDIR})
  +        set_target_properties(${target} PROPERTIES "RUNTIME_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${bi})
  +      endif()
  +
  +      if(ARG_LIBDIR)
  +        string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} li ${ARG_LIBDIR})
  +        set_target_properties(${target} PROPERTIES "ARCHIVE_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${li})
  +      endif()
  +
  +      if(moddir)
  +        string(REPLACE ${CMAKE_CFG_INTDIR} ${build_mode} mi ${moddir})
  +        set_target_properties(${target} PROPERTIES "LIBRARY_OUTPUT_DIRECTORY_${CONFIG_SUFFIX}" ${mi})
  +      endif()
       endforeach()
     else()
  -    set_target_properties(${target} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${ARG_BINDIR})
  -    set_target_properties(${target} PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${ARG_LIBDIR})
  -    set_target_properties(${target} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${moddir})
  +    if(ARG_BINDIR)
  +      set_target_properties(${target} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${ARG_BINDIR})
  +    endif()
  +
  +    if(ARG_LIBDIR)
  +      set_target_properties(${target} PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${ARG_LIBDIR})
  +    endif()
  +
  +    if(moddir)
  +      set_target_properties(${target} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${moddir})
  +    endif()
     endif()
   endfunction()

I have one other comment below.

Thanks,
-Chris


================
Comment at: cmake/modules/AddLLVM.cmake:197
@@ -197,1 +196,3 @@
+function(set_output_directory target)
+  cmake_parse_arguments(ARG "" "BINDIR;LIBDIR;" ${ARGN})
   # Do nothing if *_OUTPUT_INTDIR is empty.
----------------
Two things here.

1) You need another empty string before ${ARGN} otherwise the first argument from ARGN will be treated as your multi-variable option list.

2) Maybe make the options more explicitly worded now that they are externally facing. I'd suggest BINARY_DIR and LIBRARY_DIR.


Repository:
  rL LLVM

http://reviews.llvm.org/D13215