Python Windows fixes (ready)

CMakeLists.txt

@@ -14,16 +14,15 @@ option(BUILD_TESTS "Build tests." ON)
 option(BUILD_CLI_EXECUTABLES "Build command-line executables." ON)
 option(DOWNLOAD_ENSMALLEN "If ensmallen is not found, download it." ON)
 option(DOWNLOAD_STB_IMAGE "Download stb_image for image loading." ON)
+option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON)
 
-# Currently Python bindings aren't known to build successfully on Windows, so
-# set BUILD_PYTHON_BINDINGS to OFF when the platform is Windows.
 if (WIN32)
-  option(BUILD_PYTHON_BINDINGS "Build Python bindings." OFF)
   option(BUILD_SHARED_LIBS
       "Compile shared libraries (if OFF, static libraries are compiled)." OFF)
-  message(WARNING "By default Python bindings are not compiled for Windows because they are not known to work.  Set BUILD_PYTHON_BINDINGS to ON if you want them built.")
+
+  set(DLL_COPY_DIRS "" CACHE STRING "List of directories (separated by ';') containing DLLs to copy for runtime.")
+  set(DLL_COPY_LIBS "" CACHE STRING "List of DLLs (separated by ';') that should be copied for runtime.")
 else ()
-  option(BUILD_PYTHON_BINDINGS "Build Python bindings." ON)
   option(BUILD_SHARED_LIBS
       "Compile shared libraries (if OFF, static libraries are compiled)." ON)
 endif()
@@ -308,7 +307,23 @@ if (WIN32)
 
   # Piggyback LAPACK and BLAS linking into Armadillo link.
   set(ARMADILLO_LIBRARIES
-       ${ARMADILLO_LIBRARIES} ${BLAS_LIBRARY} ${LAPACK_LIBRARY})
+      ${ARMADILLO_LIBRARIES} ${BLAS_LIBRARY} ${LAPACK_LIBRARY})
+
+  # Ensure that the libraries are added to the MSVC IDE runtime path.
+  get_filename_component(BLAS_DIR ${BLAS_LIBRARY} DIRECTORY)
+  get_filename_component(LAPACK_DIR ${LAPACK_LIBRARY} DIRECTORY)
+
+  # Sometimes, especially with an OpenBLAS install via nuget, the DLLs are
+  # actually in ../../bin/x64/.  Automatically add these.
+  if (EXISTS "${BLAS_DIR}/../../bin/x64/")
+    get_filename_component(BLAS_DLL_DIR "${BLAS_DIR}/../../bin/x64" ABSOLUTE)
+    set(DLL_COPY_DIRS ${DLL_COPY_DIRS} "${BLAS_DLL_DIR}")
+  endif ()
+
+  if (EXISTS "${LAPACK_DIR}/../../bin/x64/")
+    get_filename_component(LAPACK_DLL_DIR "${LAPACK_DIR}/../../bin/x64" ABSOLUTE)
+    set(DLL_COPY_DIRS ${DLL_COPY_DIRS} "${BLAS_DLL_DIR}")
+  endif ()
 endif ()
 
 # Include directories for the previous dependencies.
@@ -321,7 +336,7 @@ find_package(StbImage)
 if (NOT STB_IMAGE_FOUND)
   if (DOWNLOAD_STB_IMAGE)
     set(STB_DIR "stb")
-    install(DIRECTORY DESTINATION ${CMAKE_BINARY_DIR}/deps/${STB_DIR})
+    install(DIRECTORY DESTINATION "${CMAKE_BINARY_DIR}/deps/${STB_DIR}")
     file(DOWNLOAD http://mlpack.org/files/stb/stb_image.h
         "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h"
         STATUS STB_IMAGE_DOWNLOAD_STATUS_LIST LOG STB_IMAGE_DOWNLOAD_LOG
@@ -341,8 +356,8 @@ if (NOT STB_IMAGE_FOUND)
       message(STATUS
           "Successfully downloaded stb into ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/")
       # Now we have to also ensure these header files get installed.
-      install(FILES ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
-      install(FILES ${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image_write.h DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+      install(FILES "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
+      install(FILES "${CMAKE_BINARY_DIR}/deps/${STB_DIR}/stb_image_write.h" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
       add_definitions(-DHAS_STB)
     else ()
       list(GET STB_IMAGE_DOWNLOAD_STATUS_LIST 1 STB_DOWNLOAD_ERROR)
@@ -389,8 +404,8 @@ if (NOT ENSMALLEN_FOUND)
             "Successfully downloaded ensmallen into ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/")
 
         # Now we have to also ensure these header files get installed.
-        install(DIRECTORY ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen_bits/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/ensmallen_bits)
-        install(FILES ${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen.hpp DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+        install(DIRECTORY "${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen_bits/" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/ensmallen_bits")
+        install(FILES "${CMAKE_BINARY_DIR}/deps/${ENSMALLEN_INCLUDE_DIR}/include/ensmallen.hpp" DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
       else ()
         message(FATAL_ERROR "Problem unpacking ensmallen!  Expected only one directory ensmallen-x.y.z/; found ${ENS_DIRECTORIES}. Try removing the directory ${CMAKE_BINARY_DIR}/deps and reconfiguring.")
       endif ()
@@ -453,6 +468,8 @@ link_directories(${Boost_LIBRARY_DIRS})
 # handle it.
 if (MSVC)
   link_directories(${Boost_LIBRARY_DIRS})
+  set(CMAKE_MSVCIDE_RUN_PATH ${CMAKE_MSVCIDE_RUN_PATH} ${Boost_LIBRARY_DIRS})
+  message("boost lib dirs ${Boost_LIBRARY_DIRS}")
   set(Boost_LIBRARIES "")
 endif ()
 
@@ -505,6 +522,21 @@ if (WIN32)
   set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR})
   set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR})
   set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR})
+
+  # Copy all necessary DLLs for runtime to the build directory.
+  # This is a little hackish, but I can't figure out clear ways to make CMake
+  # consistently link everything 100% statically across platforms or set the
+  # runtime path right always, so this is the best I know how to do for now.
+  foreach(dir ${DLL_COPY_DIRS})
+    file(GLOB dir_dll_list "${dir}/*.dll")
+    file(COPY ${dir_dll_list} DESTINATION ${CMAKE_BINARY_DIR}/Release/)
+    file(COPY ${dir_dll_list} DESTINATION ${CMAKE_BINARY_DIR}/Debug/)
+  endforeach ()
+
+  foreach(file ${DLL_COPY_LIBS})
+    file(COPY ${file} DESTINATION ${CMAKE_BINARY_DIR}/Release/)
+    file(COPY ${file} DESTINATION ${CMAKE_BINARY_DIR}/Debug/)
+  endforeach()
 else ()
   # If not on Windows, put them under more standard UNIX-like places.  This is
   # necessary, otherwise they would all end up in
@@ -578,8 +610,8 @@ if (BUILD_CLI_EXECUTABLES AND UNIX)
     )
 
     # Set the rules to install the documentation.
-    install(DIRECTORY ${CMAKE_BINARY_DIR}/share/man/
-        DESTINATION ${CMAKE_INSTALL_MANDIR})
+    install(DIRECTORY "${CMAKE_BINARY_DIR}/share/man/"
+        DESTINATION "${CMAKE_INSTALL_MANDIR}")
   endif ()
 endif ()
 
@@ -630,8 +662,8 @@ if (DOXYGEN_FOUND)
       COMMENT "Generating API documentation with Doxygen"
   )
 
-  install(DIRECTORY ${CMAKE_BINARY_DIR}/doc/html
-      DESTINATION ${CMAKE_INSTALL_DOCDIR}
+  install(DIRECTORY "${CMAKE_BINARY_DIR}/doc/html"
+      DESTINATION "${CMAKE_INSTALL_DOCDIR}"
       COMPONENT doc
       OPTIONAL
   )
@@ -710,6 +742,6 @@ if (PKG_CONFIG_FOUND)
       DEPENDS mlpack_headers
       COMMENT "Generating mlpack.pc (pkg-config) file.")
 
-  install(FILES ${CMAKE_CURRENT_BINARY_DIR}/lib/pkgconfig/mlpack.pc
-      DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig/)
+  install(FILES "${CMAKE_CURRENT_BINARY_DIR}/lib/pkgconfig/mlpack.pc"
+      DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig/")
 endif ()

src/mlpack/CMakeLists.txt

@@ -90,21 +90,21 @@ endforeach()
 
 # At install time, we simply install that directory of header files we
 # collected to include/.
-install(DIRECTORY ${CMAKE_BINARY_DIR}/include/mlpack DESTINATION
-    ${CMAKE_INSTALL_INCLUDEDIR})
+install(DIRECTORY "${CMAKE_BINARY_DIR}/include/mlpack" DESTINATION
+    "${CMAKE_INSTALL_INCLUDEDIR}")
 
 # Set generated executables to be installed.  Unfortunately they must manually
 # be entered...
 install(TARGETS mlpack
-    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
-    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
+    RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
+    LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
+    ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}")
 
 add_dependencies(mlpack mlpack_headers)
 
 # If we are building Python bindings, we have to configure setup.py but only
 # after we've recursed into methods/.
-if (BUILD_PYTHON_BINDINGS)
+if (BUILDING_PYTHON_BINDINGS)
   # Extract the version number.
   file(READ "${CMAKE_SOURCE_DIR}/src/mlpack/core/util/version.hpp"
       VERSION_HPP_CONTENTS)
@@ -120,8 +120,26 @@ if (BUILD_PYTHON_BINDINGS)
 
   get_property(CYTHON_INCLUDE_DIRECTORIES DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
       PROPERTY INCLUDE_DIRECTORIES)
-  configure_file(${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in
-                 ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py)
+  add_custom_target(python_configure
+      COMMAND ${CMAKE_COMMAND}
+          -D SETUP_PY_IN="${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/setup.py.in"
+          -D SETUP_PY_OUT="${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py"
+          -D PACKAGE_VERSION="${PACKAGE_VERSION}"
+          -D Boost_SERIALIZATION_LIBRARY="${Boost_SERIALIZATION_LIBRARY_RELEASE}"
+          -D Boost_LIBRARY_DIRS="${Boost_LIBRARY_DIRS}"
+          -D ARMADILLO_LIBRARIES="${ARMADILLO_LIBRARIES}"
+          -D MLPACK_LIBRARY="$<TARGET_LINKER_FILE:mlpack>"
+          -D MLPACK_LIBDIR="$<TARGET_LINKER_FILE_DIR:mlpack>"
+          -D MLPACK_PYXS="${MLPACK_PYXS}"
+          -D OpenMP_CXX_FLAGS="${OpenMP_CXX_FLAGS}"
+          -D DISABLE_CFLAGS="${DISABLE_CFLAGS}"
+          -D CYTHON_INCLUDE_DIRECTORIES="${CYTHON_INCLUDE_DIRECTORIES}"
+          -D OUTPUT_DIR="${CMAKE_BINARY_DIR}"
+          -P "${CMAKE_SOURCE_DIR}/src/mlpack/bindings/python/ConfigureSetup.cmake"
+      BYPRODUCTS "${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py"
+      COMMENT "Configuring setup.py...")
+  add_dependencies(python_configure python_copy)
+  add_dependencies(python_configured python_configure)
 endif ()
 
 # If we are building Markdown documentation, we have to run some setup after we

src/mlpack/bindings/CMakeLists.txt

@@ -14,3 +14,4 @@ set(MARKDOWN_CATEGORIES ${MARKDOWN_CATEGORIES} PARENT_SCOPE)
 set(MLPACK_SRCS ${MLPACK_SRCS} PARENT_SCOPE)
 set(MLPACK_PYXS ${MLPACK_PYXS} PARENT_SCOPE)
 set(DISABLE_CFLAGS ${DISABLE_CFLAGS} PARENT_SCOPE)
+set(BUILDING_PYTHON_BINDINGS ${BUILDING_PYTHON_BINDINGS} PARENT_SCOPE)

src/mlpack/bindings/cli/CMakeLists.txt

@@ -59,7 +59,7 @@ if (BUILD_CLI_EXECUTABLES)
   # compiled with the correct int main() call.
   set_target_properties(mlpack_${name} PROPERTIES COMPILE_FLAGS
       -DBINDING_TYPE=BINDING_TYPE_CLI)
-  install(TARGETS mlpack_${name} RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})
+  install(TARGETS mlpack_${name} RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
 
   # If man documentation is being generated, make sure this is a dependency.
   if (TXT2MAN)

src/mlpack/bindings/python/CMakeLists.txt

@@ -40,6 +40,8 @@ if (NOT PY_PANDAS)
   not_found_return("pandas not found; not building Python bindings.")
 endif ()
 
+set(BUILDING_PYTHON_BINDINGS ON PARENT_SCOPE)
+
 # Nothing in this directory will be compiled into mlpack.
 set(BINDING_SOURCES
   default_param.hpp
@@ -105,6 +107,8 @@ endif ()
 
 add_custom_target(python ALL DEPENDS mlpack)
 add_custom_target(python_copy ALL DEPENDS mlpack)
+# The python_configure target is added later; this is a dummy target.
+add_custom_target(python_configured ALL)
 
 # Copy necessary files after making the mlpack/ directory.
 add_custom_command(TARGET python_copy PRE_BUILD
@@ -137,7 +141,7 @@ if (BUILD_TESTS)
 endif ()
 
 # Install any dependencies via setuptools automatically.
-add_custom_command(TARGET python_copy POST_BUILD
+add_custom_command(TARGET python_configured POST_BUILD
     COMMAND ${CMAKE_COMMAND} -E env NO_BUILD=1 ${PYTHON_EXECUTABLE}
         ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/setup.py build
     WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/)
@@ -162,7 +166,7 @@ add_custom_command(TARGET python POST_BUILD
         ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/copy_artifacts.py
     WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/)
 
-add_dependencies(python python_copy)
+add_dependencies(python python_configured)
 
 # Configure installation script file.
 execute_process(COMMAND ${PYTHON_EXECUTABLE}
@@ -174,12 +178,32 @@ install(CODE "set(PYTHON_EXECUTABLE \"${PYTHON_EXECUTABLE}\")")
 install(CODE "set(CMAKE_BINARY_DIR \"${CMAKE_BINARY_DIR}\")")
 install(CODE "set(CMAKE_INSTALL_PREFIX \"${CMAKE_INSTALL_PREFIX}\")")
 install(CODE "execute_process(COMMAND mkdir -p $ENV{DESTDIR}${CMAKE_PYTHON_PATH})")
-install(SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/PythonInstall.cmake)
+install(SCRIPT "${CMAKE_CURRENT_SOURCE_DIR}/PythonInstall.cmake")
 
 # Prepare __init__.py for having all of the convenience imports appended to it.
 file(COPY mlpack/__init__.py DESTINATION
     ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
 
+if (WIN32)
+  # Copy all necessary DLLs to the Python build directory.
+  foreach (dir ${DLL_COPY_DIRS})
+    file(GLOB dll_dir_files "${dir}/*.dll")
+    file(COPY ${dll_dir_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
+  endforeach ()
+
+  foreach (dll ${DLL_COPY_LIBS})
+    file(COPY ${dll} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
+  endforeach ()
+
+  # We also need to copy the boost DLLs over.
+  file(GLOB boost_ser_dll_files "${Boost_LIBRARY_DIRS}/*serialization*.dll")
+  file(COPY ${boost_ser_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
+  file(GLOB boost_po_dll_files "${Boost_LIBRARY_DIRS}/*program*options*.dll")
+  file(COPY ${boost_po_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
+  file(GLOB boost_utf_dll_files "${Boost_LIBRARY_DIRS}/*unit*test*framework*.dll")
+  file(COPY ${boost_utf_dll_files} DESTINATION ${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/)
+endif ()
+
 # Add a macro to build a python binding.
 macro (add_python_binding name)
 if (BUILD_PYTHON_BINDINGS)
@@ -203,7 +227,7 @@ if (BUILD_PYTHON_BINDINGS)
       -DBINDING_TYPE=BINDING_TYPE_PYX)
   add_custom_command(TARGET generate_pyx_${name} POST_BUILD
       COMMAND ${CMAKE_COMMAND}
-          -DPROGRAM=${CMAKE_BINARY_DIR}/bin/generate_pyx_${name}
+          -DPROGRAM=$<TARGET_FILE:generate_pyx_${name}>
           -DOUTPUT_FILE=${CMAKE_BINARY_DIR}/src/mlpack/bindings/python/mlpack/${name}.pyx
           -P ${CMAKE_SOURCE_DIR}/CMake/RunProgram.cmake)
 
@@ -220,7 +244,7 @@ if (BUILD_PYTHON_BINDINGS)
 
   add_dependencies(python build_pyx_${name})
   add_dependencies(build_pyx_${name} generate_pyx_${name})
-  add_dependencies(generate_pyx_${name} python_copy)
+  add_dependencies(generate_pyx_${name} python_configured)
 
   # Add the convenience import to __init__.py.  Note that this happens during
   # configuration.

src/mlpack/bindings/python/ConfigureSetup.cmake

@@ -0,0 +1,46 @@
+# ConfigureSetup.cmake: generate the setup.py file given several environment
+# variables.
+#
+# This file depends on the following variables being set:
+#
+# - SETUP_PY_IN: location of input file
+# - SETUP_PY_OUT: location of output file
+# - PACKAGE_VERSION: version of package
+# - Boost_SERIALIZATION_LIBRARY: location of Boost serialization library
+# - Boost_LIBRARY_DIRS: paths to boost libraries
+# - ARMADILLO_LIBRARIES: space-separated list of Armadillo dependencies
+# - MLPACK_LIBRARY: location of mlpack library
+# - MLPACK_PYXS: list of pyx files
+# - OpenMP_CXX_FLAGS: OpenMP C++ compilation flags
+# - DISABLE_CFLAGS: list of CFLAGS or CXXFLAGS to be disabled
+# - CYTHON_INCLUDE_DIRECTORIES: include directories for Cython
+# - MLPACK_LIBDIR: path to mlpack libraries
+# - OUTPUT_DIR: binary output directory for CMake
+
+# It's possible that the FindBoost CMake script may have returned a Boost
+# library with "lib" improperly prepended to it.  So we need to see if the file
+# exists, and if it doesn't, but it has a "lib" in it, then we will try
+# stripping the "lib" off the front.
+message(STATUS "Run with ${Boost_SERIALIZATION_LIBRARY}.")
+if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}")
+  message(STATUS "Did not find serialization library ${Boost_SERIALIZATION_LIBRARY}!")
+  # Split the filename to see if it starts with lib.
+  set(Boost_SERIALIZATION_LIBRARY_ORIG "${Boost_SERIALIZATION_LIBRARY}")
+  get_filename_component(SER_LIB_DIRECTORY "${Boost_SERIALIZATION_LIBRARY}"
+      DIRECTORY)
+  get_filename_component(SER_LIB_FILENAME "${Boost_SERIALIZATION_LIBRARY}" NAME)
+  message(STATUS "Name component is ${SER_LIB_FILENAME}, and directory is ${SER_LIB_DIRECTORY}.")
+
+  # Strip any preceding "lib/".
+  string(REGEX REPLACE "^lib" "" STRIPPED_FILENAME "${SER_LIB_FILENAME}")
+  message(STATUS "Regex gave us ${STRIPPED_FILENAME}.")
+  set(Boost_SERIALIZATION_LIBRARY "${SER_LIB_DIRECTORY}/${STRIPPED_FILENAME}")
+  message(STATUS "New library ${Boost_SERIALIZATION_LIBRARY}.")
+
+  if (NOT EXISTS "${Boost_SERIALIZATION_LIBRARY}")
+    # We didn't find it, so for ease of debugging just revert to the original.
+    set (Boost_SERIALIZATION_LIBRARY "${Boost_SERIALIZATION_LIBRARY_ORIG}")
+  endif ()
+endif ()
+
+configure_file("${SETUP_PY_IN}" "${SETUP_PY_OUT}")