sgilmore10 commented on a change in pull request #10305:
URL: https://github.com/apache/arrow/pull/10305#discussion_r658013602



##########
File path: matlab/CMakeLists.txt
##########
@@ -29,22 +30,51 @@ if(EXISTS "${CPP_CMAKE_MODULES}")
   set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CPP_CMAKE_MODULES})
 endif()
 
-## Arrow is Required
+set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR}/cmake_modules)
+
+# Arrow is Required
 find_package(Arrow REQUIRED)
 
-## MATLAB is required to be installed to build MEX interfaces
-set(MATLAB_ADDITIONAL_VERSIONS "R2018a=9.4")
-find_package(Matlab REQUIRED MX_LIBRARY)
-
-# Build featherread mex file based on the arrow shared library
-matlab_add_mex(NAME featherreadmex
-               SRC src/featherreadmex.cc src/feather_reader.cc 
src/util/handle_status.cc
-                   src/util/unicode_conversion.cc
-               LINK_TO ${ARROW_SHARED_LIB})
-target_include_directories(featherreadmex PRIVATE ${ARROW_INCLUDE_DIR})
-
-# Build featherwrite mex file based on the arrow shared library
-matlab_add_mex(NAME featherwritemex
-               SRC src/featherwritemex.cc src/feather_writer.cc 
src/util/handle_status.cc
-               LINK_TO ${ARROW_SHARED_LIB})
-target_include_directories(featherwritemex PRIVATE ${ARROW_INCLUDE_DIR})
+# MATLAB is Required
+find_package(Matlab REQUIRED)
+
+# Construct the absolute path to featherread's source files
+set(featherread_sources featherreadmex.cc feather_reader.cc 
util/handle_status.cc
+                        util/unicode_conversion.cc)
+list(TRANSFORM featherread_sources PREPEND ${CMAKE_SOURCE_DIR}/src/)
+
+# Build featherreadmex MEX binary
+matlab_add_mex(R2018a
+               NAME
+               featherreadmex
+               SRC
+               ${featherread_sources}
+               LINK_TO
+               arrow_shared)
+
+# Construct the absolute path to featherwrite's source files
+set(featherwrite_sources featherwritemex.cc feather_writer.cc 
util/handle_status.cc
+                         util/unicode_conversion.cc)
+list(TRANSFORM featherwrite_sources PREPEND ${CMAKE_SOURCE_DIR}/src/)
+
+# Build featherwritemex MEX binary
+matlab_add_mex(R2018a
+               NAME
+               featherwritemex
+               SRC
+               ${featherwrite_sources}
+               LINK_TO
+               arrow_shared)
+
+# Ensure the MEX binaries are placed in the src directory on all platforms

Review comment:
       In order to execute the MEX function, it has to be discoverable on the 
MATLAB search path. We also have MATLAB code (featherread.m and featherwrite.m) 
that we also need to add to the MATLAB search path. Since we need to add the 
source directory to the path anyway, I thought it makes sense to put the MEX 
files there as well. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to