kou commented on code in PR #34563: URL: https://github.com/apache/arrow/pull/34563#discussion_r1139913862
########## matlab/tools/cmake/BuildMatlabArrowInterface.cmake: ########## @@ -0,0 +1,80 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# ------- +# Config +# ------- + +# Build configuration for libmexclass. +set(CUSTOM_PROXY_FACTORY_INCLUDE_DIR "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/proxy;${CMAKE_SOURCE_DIR}/src/cpp") +set(CUSTOM_PROXY_FACTORY_SOURCES "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/proxy/factory.cc") +set(CUSTOM_PROXY_SOURCES "${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/array/proxy/float64_array.cc") +set(CUSTOM_PROXY_INCLUDE_DIR "${CMAKE_SOURCE_DIR}/src/cpp;${ARROW_INCLUDE_DIR}") +set(CUSTOM_PROXY_LINK_LIBRARIES ${ARROW_LINK_LIB}) +# On Windows, arrow.dll must be installed regardless of +# whether Arrow_FOUND is true or false. Therefore, we explicitly +# copy ARROW_SHARED_LIB to the installation folder +libmexclass/+proxy. +set(CUSTOM_PROXY_RUNTIME_LIBRARIES ${ARROW_SHARED_LIB}) +set(CUSTOM_PROXY_FACTORY_HEADER_FILENAME "factory.h") +set(CUSTOM_PROXY_FACTORY_CLASS_NAME "arrow::matlab::proxy::Factory") Review Comment: Thanks for explaining the `libmexclass` build system! Can I confirm my understandings? * `libmexclass.so` can be shared from multiple MATALB modules. (Module A and module B can use the same `libmexclass.so`.) * `libmexclassclient.so` can't be shared from multiple MATAB modules. (Each module must build its `libmexclassclient.so`.) > One thing worth explicitly noting, is that `target_compile_definitions` is used to "dynamically" build a MEX binary from [predefined "template" code](https://github.com/mathworks/libmexclass/blob/484ade97176c611cbb7056b941df99f1f3a6a0dc/libmexclass/cpp/source/libmexclass/mex/mex_gateway_function.cpp#L62). Essentially, we use these compiler definitions to inform the compiler what the appropriate [header file](https://github.com/mathworks/libmexclass/blob/484ade97176c611cbb7056b941df99f1f3a6a0dc/libmexclass/cpp/source/libmexclass/mex/mex_gateway_function.cpp#L7) is for the client's subclass of `libmexclass::proxy::Factory` and what the [name of their subclass is](https://github.com/mathworks/libmexclass/blob/484ade97176c611cbb7056b941df99f1f3a6a0dc/libmexclass/cpp/source/libmexclass/mex/mex_gateway_function.cpp#L62). It seems that we can just use C++ template feature for it instead of macros when we build `libmexclient.so` in our CMake. BTW, who does instantiate `MexFunction` defined in `mex_gateway_function.cpp`? > are we OK if the client code still has to call a few macros to set up the appropriate target and link dependencies between the client shared library (i.e. their `Factory` and `Proxy` code) and the MEX `gateway` target? "The client code" is [`mex_gateway_function.cpp`](https://github.com/mathworks/libmexclass/blob/484ade97176c611cbb7056b941df99f1f3a6a0dc/libmexclass/cpp/source/libmexclass/mex/mex_gateway_function.cpp), right? Or our CMake code? "a few macros" are C/C++'s (C preprocessor's) macros, right? Or CMake's macro? "the appropriate target and link dependencies" is for CMake, right? Calling `add_library()`, `target_link_libraries()` and so on for `libmexclient.so` and `gateway.mexa64` are OK for me. Because it's one of natural CMake usages. > Our hope is that we could potentially preserve some of this simplicity by providing a few helper macros. "a few helper macros" are CMake's macros, right? +1 > Are we OK with continuing to build the MEX `gateway` from a [predefined "template" source file](https://github.com/mathworks/libmexclass/blob/484ade97176c611cbb7056b941df99f1f3a6a0dc/libmexclass/cpp/source/libmexclass/mex/mex_gateway_function.cpp#L62) and using "code injection" (i.e. `target_compile_definitions`) to inform the compiler what header file and class name to use for the client subclass of `libmexclass::proxy::Factory`? Can we use C++ template feature instead of the approach? I may provide a code example once I understand who instantiates `MexFunction`. > it seems like there are two different approaches being suggested here. One is to use `FetchContent` to essentially "embed" the build system of `libmexclass` into the client build system (i.e. make the targets and macros available). The other approach seems to be creating a CMake package and using it in conjunction with `find_package`. Right. > Are there any strong opinions on which approach is preferable? At first glance, it seems like using `FetchContent` might be a bit simpler to setup. I don't have a strong opinion. I think that the latter approach (installing `libmexclass` and use it as a CMake package) is better because multiple MATLAB modules can share one `libmexclass` installation. But I'm OK with the former approach (using `FetchContent`). -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
