From ae3eb0a94e33aec94fa9db45c1c976c4329c2cd4 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Thu, 9 Sep 2021 11:15:21 -0400 Subject: [PATCH 01/33] Create a single mex "dispatcher" function that invokes either featherreadmex or featherwritemex --- matlab/src/featherread.m | 2 +- matlab/src/featherreadmex.cc | 5 +- matlab/src/featherreadmex.h | 20 ++++++++ matlab/src/featherwrite.m | 3 +- matlab/src/featherwritemex.cc | 5 +- matlab/src/featherwritemex.h | 20 ++++++++ matlab/src/mexDispatcher.cc | 88 +++++++++++++++++++++++++++++++++++ 7 files changed, 135 insertions(+), 8 deletions(-) create mode 100644 matlab/src/featherreadmex.h create mode 100644 matlab/src/featherwritemex.h create mode 100644 matlab/src/mexDispatcher.cc diff --git a/matlab/src/featherread.m b/matlab/src/featherread.m index 31bc426b877..7a0eca2338c 100644 --- a/matlab/src/featherread.m +++ b/matlab/src/featherread.m @@ -46,7 +46,7 @@ % Read table variables and metadata from the given Feather file using % libarrow. -[variables, metadata] = featherreadmex(filename); +[variables, metadata] = mexDispatcher('featherread', filename); % Make valid MATLAB table variable names out of any of the % Feather table column names that are not valid MATLAB table diff --git a/matlab/src/featherreadmex.cc b/matlab/src/featherreadmex.cc index b52b8a98f1d..874c26682dc 100644 --- a/matlab/src/featherreadmex.cc +++ b/matlab/src/featherreadmex.cc @@ -17,13 +17,12 @@ #include -#include - +#include "featherreadmex.h" #include "feather_reader.h" #include "util/handle_status.h" // MEX gateway function. This is the entry point for featherreadmex.cpp. -void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { +void featherreadmex(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; // Read the given Feather file into memory. diff --git a/matlab/src/featherreadmex.h b/matlab/src/featherreadmex.h new file mode 100644 index 00000000000..d80b7c143f1 --- /dev/null +++ b/matlab/src/featherreadmex.h @@ -0,0 +1,20 @@ +// 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. + +#include + +void featherreadmex(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file diff --git a/matlab/src/featherwrite.m b/matlab/src/featherwrite.m index eeedf26d0be..ded3cef1703 100644 --- a/matlab/src/featherwrite.m +++ b/matlab/src/featherwrite.m @@ -39,6 +39,7 @@ function featherwrite(filename, t) [variables, metadata] = table2mlarrow(t); % Write the table to a Feather file. -featherwritemex(filename, variables, metadata); +mexDispatcher('featherwrite', filename, variables, metadata); +%featherwritemex(filename, variables, metadata); end diff --git a/matlab/src/featherwritemex.cc b/matlab/src/featherwritemex.cc index d8f90baafc5..cade9079d4a 100644 --- a/matlab/src/featherwritemex.cc +++ b/matlab/src/featherwritemex.cc @@ -17,13 +17,12 @@ #include -#include - +#include "featherwritemex.h" #include "feather_writer.h" #include "util/handle_status.h" // MEX gateway function. This is the entry point for featherwritemex.cc. -void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { +void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; // Open a Feather file at the provided file path for writing. diff --git a/matlab/src/featherwritemex.h b/matlab/src/featherwritemex.h new file mode 100644 index 00000000000..205db515c4e --- /dev/null +++ b/matlab/src/featherwritemex.h @@ -0,0 +1,20 @@ +// 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. + +#include + +void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file diff --git a/matlab/src/mexDispatcher.cc b/matlab/src/mexDispatcher.cc new file mode 100644 index 00000000000..ba1d9928ece --- /dev/null +++ b/matlab/src/mexDispatcher.cc @@ -0,0 +1,88 @@ +// 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. + +#include +#include + +#include + +#include "featherreadmex.h" +#include "featherwritemex.h" + +enum class OperationCode +{ + FEATHER_READ, + FEATHER_WRITE, + INVALID_OPERATION +}; + +static const std::unordered_map OPERATION_CODE_MAP = +{ + {u8"featherread", OperationCode::FEATHER_READ}, + {u8"featherwrite", OperationCode::FEATHER_WRITE} +}; + + +OperationCode opname_to_opcode(const std::string& opname) +{ + auto kv_pair = OPERATION_CODE_MAP.find(opname); + if (kv_pair == OPERATION_CODE_MAP.end()) { + return OperationCode::INVALID_OPERATION; + } + return kv_pair->second; +} + +std::string get_opname(const mxArray* input) +{ + std::string opname; + if (!mxIsChar(input)) { + mexErrMsgIdAndTxt("MATLAB:arrow:OperationNameDataType", + "The first input to 'mexDispatcher' must be a char vector."); + } + const char* c_str = mxArrayToUTF8String(input); + if (!c_str) { + mexErrMsgIdAndTxt("MATLAB:arrow:NullOperationName", + "The function name passed to ''mexDispatcher' cannot be null."); + } + return std::string{c_str}; +} + +void checkNumArgs(int nrhs) +{ + if (nrhs < 1) { + mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", + "'mexDispatcher' requires at least one input argument."); + } +} + +// MEX gateway function. +void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) +{ + checkNumArgs(nrhs); + const std::string opname = get_opname(prhs[0]); + OperationCode opcode = opname_to_opcode(opname); + switch (opcode) { + case OperationCode::FEATHER_READ: + featherreadmex(nlhs, plhs, nrhs - 1, ++prhs); + break; + case OperationCode::FEATHER_WRITE: + featherwrite(nlhs, plhs, nrhs - 1, ++prhs); + break; + case OperationCode::INVALID_OPERATION: + break; + } +} From bcde02fbb35dbb2673cef415dc82236506e693aa Mon Sep 17 00:00:00 2001 From: sgilmore Date: Thu, 9 Sep 2021 11:18:37 -0400 Subject: [PATCH 02/33] Update CMakeLists.txt to build one mex function (mexDispatcher) that added to the build folder - not the source folder. --- matlab/CMakeLists.txt | 39 ++++++++------------------------------- 1 file changed, 8 insertions(+), 31 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index e667500c466..4c6e7fc70e7 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -228,41 +228,18 @@ endif() # 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/) +# Construct the absolute path to mexDispatcher's souce files +set(mexDispatcher_sources mexDispatcher.cc featherreadmex.cc feather_reader.cc + featherwritemex.cc feather_writer.cc util/handle_status.cc + util/unicode_conversion.cc) +list(TRANSFORM mexDispatcher_sources PREPEND ${CMAKE_SOURCE_DIR}/src/) -# Build featherreadmex MEX binary +# Build mexDispatcher MEX binary matlab_add_mex(R2018a - NAME featherreadmex - SRC ${featherread_sources} + NAME mexDispatcher + SRC ${mexDispatcher_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 -if(WIN32) - set_target_properties(featherreadmex PROPERTIES RUNTIME_OUTPUT_DIRECTORY - $<1:${CMAKE_SOURCE_DIR}/src>) - set_target_properties(featherwritemex PROPERTIES RUNTIME_OUTPUT_DIRECTORY - $<1:${CMAKE_SOURCE_DIR}/src>) -else() - set_target_properties(featherreadmex PROPERTIES LIBRARY_OUTPUT_DIRECTORY - $<1:${CMAKE_SOURCE_DIR}/src>) - set_target_properties(featherwritemex PROPERTIES LIBRARY_OUTPUT_DIRECTORY - $<1:${CMAKE_SOURCE_DIR}/src>) -endif() - # ############################################################################## # C++ Tests # ############################################################################## From 9e9da32e28e9ff0f919f66679b48d3a6ad744ac2 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Thu, 9 Sep 2021 11:21:52 -0400 Subject: [PATCH 03/33] Update tfeather and tfeathermex to add the build folder to the MATLAB Search Path. --- matlab/test/tfeather.m | 14 +++++--------- matlab/test/tfeathermex.m | 19 +++++++------------ matlab/test/util/featherMEXRoundTrip.m | 4 ++-- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/matlab/test/tfeather.m b/matlab/test/tfeather.m index 625a3a52544..163a1b3ccc5 100755 --- a/matlab/test/tfeather.m +++ b/matlab/test/tfeather.m @@ -24,14 +24,12 @@ function addFeatherFunctionsToMATLABPath(testCase) testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'src'))); - % featherreadmex must be on the MATLAB path. - testCase.assertTrue(~isempty(which('featherreadmex')), ... - '''featherreadmex'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); - % featherwritemex must be on the MATLAB path. - testCase.assertTrue(~isempty(which('featherwritemex')), ... - '''featherwritemex'' must be on to the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); + % Add mexDispatcher to the MATLAB path. + testCase.applyFixture(PathFixture(fullfile('..', 'build'))); + % mexDispatcher must be on the MATLAB path. + testCase.assertTrue(~isempty(which('mexDispatcher')), ... + '''mexDispatcher'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); end - end methods(TestMethodSetup) @@ -226,7 +224,5 @@ function NumericComplexUnsupported(testCase) expectedTable = featherRoundTrip(filename, actualTable); testCase.verifyNotEqual(actualTable, expectedTable); end - end - end diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index 77070ad1421..3862e0b7b7f 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -24,14 +24,12 @@ function addFeatherFunctionsToMATLABPath(testCase) testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'src'))); - % featherreadmex must be on the MATLAB path. - testCase.assertTrue(~isempty(which('featherreadmex')), ... - '''featherreadmex'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); - % featherwritemex must be on the MATLAB path. - testCase.assertTrue(~isempty(which('featherwritemex')), ... - '''featherwritemex'' must be on to the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); + % Add mexDispatcher to the MATLAB path. + testCase.applyFixture(PathFixture(fullfile('..', 'build'))); + % mexDispatcher must be on the MATLAB path. + testCase.assertTrue(~isempty(which('mexDispatcher')), ... + '''mexDispatcher'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); end - end methods(TestMethodSetup) @@ -40,7 +38,6 @@ function setupTempWorkingDirectory(testCase) import matlab.unittest.fixtures.WorkingFolderFixture; testCase.applyFixture(WorkingFolderFixture); end - end methods(Test) @@ -61,7 +58,7 @@ function InvalidMATLABTableVariableNames(testCase) validVariable = mlarrow.util.createVariableStruct('double', 1, true, 'Valid'); variables = [invalidVariable, validVariable]; metadata = mlarrow.util.createMetadataStruct(1, 2); - featherwritemex(filename, variables, metadata); + mexDispatcher('featherwrite', filename, variables, metadata); t = featherread(filename); testCase.verifyEqual(t.Properties.VariableNames{1}, 'x_'); @@ -69,8 +66,6 @@ function InvalidMATLABTableVariableNames(testCase) testCase.verifyEqual(t.Properties.VariableDescriptions{1}, 'Original variable name: ''@'''); testCase.verifyEqual(t.Properties.VariableDescriptions{2}, ''); - end - + end end - end diff --git a/matlab/test/util/featherMEXRoundTrip.m b/matlab/test/util/featherMEXRoundTrip.m index 49ab183edab..f3a1b93e4bd 100644 --- a/matlab/test/util/featherMEXRoundTrip.m +++ b/matlab/test/util/featherMEXRoundTrip.m @@ -17,6 +17,6 @@ % implied. See the License for the specific language governing % permissions and limitations under the License. -featherwritemex(filename, variablesIn, metadataIn); -[variablesOut, metadataOut] = featherreadmex(filename); +mexDispatcher('featherwrite', filename, variablesIn, metadataIn); +[variablesOut, metadataOut] = mexDispatcher('featherread', filename); end \ No newline at end of file From 578d92cbcab8e3138fcacc3b61e0fcd75e917f9e Mon Sep 17 00:00:00 2001 From: sgilmore Date: Thu, 9 Sep 2021 15:52:31 -0400 Subject: [PATCH 04/33] Rename mexDispatcher to mexfcn Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 10 +++++----- matlab/src/featherread.m | 2 +- matlab/src/featherwrite.m | 4 +--- matlab/src/{mexDispatcher.cc => mexfcn.cc} | 0 matlab/test/tfeathermex.m | 2 +- matlab/test/util/featherMEXRoundTrip.m | 2 +- 6 files changed, 9 insertions(+), 11 deletions(-) rename matlab/src/{mexDispatcher.cc => mexfcn.cc} (100%) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 4c6e7fc70e7..9d2866c5e4f 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -228,16 +228,16 @@ endif() # MATLAB is Required find_package(Matlab REQUIRED) -# Construct the absolute path to mexDispatcher's souce files -set(mexDispatcher_sources mexDispatcher.cc featherreadmex.cc feather_reader.cc +# Construct the absolute path to mexfcn's souce files +set(mexfcn_sources mexDispatcher.cc featherreadmex.cc feather_reader.cc featherwritemex.cc feather_writer.cc util/handle_status.cc util/unicode_conversion.cc) -list(TRANSFORM mexDispatcher_sources PREPEND ${CMAKE_SOURCE_DIR}/src/) +list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/) # Build mexDispatcher MEX binary matlab_add_mex(R2018a - NAME mexDispatcher - SRC ${mexDispatcher_sources} + NAME mexfcn + SRC ${mexfcn_sources} LINK_TO arrow_shared) # ############################################################################## diff --git a/matlab/src/featherread.m b/matlab/src/featherread.m index 7a0eca2338c..deb462c3f93 100644 --- a/matlab/src/featherread.m +++ b/matlab/src/featherread.m @@ -46,7 +46,7 @@ % Read table variables and metadata from the given Feather file using % libarrow. -[variables, metadata] = mexDispatcher('featherread', filename); +[variables, metadata] = mexfcn('featherread', filename); % Make valid MATLAB table variable names out of any of the % Feather table column names that are not valid MATLAB table diff --git a/matlab/src/featherwrite.m b/matlab/src/featherwrite.m index ded3cef1703..e858ac9c297 100644 --- a/matlab/src/featherwrite.m +++ b/matlab/src/featherwrite.m @@ -39,7 +39,5 @@ function featherwrite(filename, t) [variables, metadata] = table2mlarrow(t); % Write the table to a Feather file. -mexDispatcher('featherwrite', filename, variables, metadata); -%featherwritemex(filename, variables, metadata); - +mexfcn('featherwrite', filename, variables, metadata); end diff --git a/matlab/src/mexDispatcher.cc b/matlab/src/mexfcn.cc similarity index 100% rename from matlab/src/mexDispatcher.cc rename to matlab/src/mexfcn.cc diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index 3862e0b7b7f..accaf4ecdeb 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -58,7 +58,7 @@ function InvalidMATLABTableVariableNames(testCase) validVariable = mlarrow.util.createVariableStruct('double', 1, true, 'Valid'); variables = [invalidVariable, validVariable]; metadata = mlarrow.util.createMetadataStruct(1, 2); - mexDispatcher('featherwrite', filename, variables, metadata); + mexfcn('featherwrite', filename, variables, metadata); t = featherread(filename); testCase.verifyEqual(t.Properties.VariableNames{1}, 'x_'); diff --git a/matlab/test/util/featherMEXRoundTrip.m b/matlab/test/util/featherMEXRoundTrip.m index f3a1b93e4bd..6c22b8387e1 100644 --- a/matlab/test/util/featherMEXRoundTrip.m +++ b/matlab/test/util/featherMEXRoundTrip.m @@ -17,6 +17,6 @@ % implied. See the License for the specific language governing % permissions and limitations under the License. -mexDispatcher('featherwrite', filename, variablesIn, metadataIn); +mexfcn('featherwrite', filename, variablesIn, metadataIn); [variablesOut, metadataOut] = mexDispatcher('featherread', filename); end \ No newline at end of file From ed8f3cf8a84344292409f0b3df652ff7d5c459a2 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Thu, 9 Sep 2021 15:57:51 -0400 Subject: [PATCH 05/33] Rename mexDispatcher to mexfcn (part 2) Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 2 +- matlab/test/tfeather.m | 8 ++++---- matlab/test/tfeathermex.m | 8 ++++---- matlab/test/util/featherMEXRoundTrip.m | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 9d2866c5e4f..340e6d74082 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -229,7 +229,7 @@ endif() find_package(Matlab REQUIRED) # Construct the absolute path to mexfcn's souce files -set(mexfcn_sources mexDispatcher.cc featherreadmex.cc feather_reader.cc +set(mexfcn_sources mexfcn.cc featherreadmex.cc feather_reader.cc featherwritemex.cc feather_writer.cc util/handle_status.cc util/unicode_conversion.cc) list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/) diff --git a/matlab/test/tfeather.m b/matlab/test/tfeather.m index 163a1b3ccc5..0aa884dcfb2 100755 --- a/matlab/test/tfeather.m +++ b/matlab/test/tfeather.m @@ -24,11 +24,11 @@ function addFeatherFunctionsToMATLABPath(testCase) testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'src'))); - % Add mexDispatcher to the MATLAB path. + % Add mexfcn to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'build'))); - % mexDispatcher must be on the MATLAB path. - testCase.assertTrue(~isempty(which('mexDispatcher')), ... - '''mexDispatcher'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); + % mexfcn must be on the MATLAB path. + testCase.assertTrue(~isempty(which('mexfcn')), ... + '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); end end diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index accaf4ecdeb..97277fdc031 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -24,11 +24,11 @@ function addFeatherFunctionsToMATLABPath(testCase) testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'src'))); - % Add mexDispatcher to the MATLAB path. + % Add mexfcn to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'build'))); - % mexDispatcher must be on the MATLAB path. - testCase.assertTrue(~isempty(which('mexDispatcher')), ... - '''mexDispatcher'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); + % mexfcn must be on the MATLAB path. + testCase.assertTrue(~isempty(which('mexfcn')), ... + '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); end end diff --git a/matlab/test/util/featherMEXRoundTrip.m b/matlab/test/util/featherMEXRoundTrip.m index 6c22b8387e1..021e0f6e2cf 100644 --- a/matlab/test/util/featherMEXRoundTrip.m +++ b/matlab/test/util/featherMEXRoundTrip.m @@ -18,5 +18,5 @@ % permissions and limitations under the License. mexfcn('featherwrite', filename, variablesIn, metadataIn); -[variablesOut, metadataOut] = mexDispatcher('featherread', filename); +[variablesOut, metadataOut] = mexfcn('featherread', filename); end \ No newline at end of file From 878585d089cd16a0d9a81cdae14d47808d8bc347 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 09:49:56 -0400 Subject: [PATCH 06/33] Move feather cc and h files to cpp/matlab/arrow/feather --- matlab/CMakeLists.txt | 8 ++++---- .../src/{ => cpp/matlab/arrow/feather}/feather_reader.cc | 0 .../src/{ => cpp/matlab/arrow/feather}/feather_reader.h | 0 .../src/{ => cpp/matlab/arrow/feather}/feather_writer.cc | 0 .../src/{ => cpp/matlab/arrow/feather}/feather_writer.h | 0 .../src/{ => cpp/matlab/arrow/feather}/featherreadmex.cc | 0 .../src/{ => cpp/matlab/arrow/feather}/featherreadmex.h | 0 .../src/{ => cpp/matlab/arrow/feather}/featherwritemex.cc | 0 .../src/{ => cpp/matlab/arrow/feather}/featherwritemex.h | 0 matlab/src/{ => cpp/matlab/arrow/feather}/matlab_traits.h | 0 .../{ => cpp/matlab/arrow/feather}/util/handle_status.cc | 0 .../{ => cpp/matlab/arrow/feather}/util/handle_status.h | 0 .../matlab/arrow/feather}/util/unicode_conversion.cc | 0 .../matlab/arrow/feather}/util/unicode_conversion.h | 0 matlab/src/{ => cpp/matlab/arrow/mex}/mexfcn.cc | 0 15 files changed, 4 insertions(+), 4 deletions(-) rename matlab/src/{ => cpp/matlab/arrow/feather}/feather_reader.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/feather_reader.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/feather_writer.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/feather_writer.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/featherreadmex.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/featherreadmex.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/featherwritemex.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/featherwritemex.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/matlab_traits.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/util/handle_status.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/util/handle_status.h (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/util/unicode_conversion.cc (100%) rename matlab/src/{ => cpp/matlab/arrow/feather}/util/unicode_conversion.h (100%) rename matlab/src/{ => cpp/matlab/arrow/mex}/mexfcn.cc (100%) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 340e6d74082..3142aca0e70 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -229,10 +229,10 @@ endif() find_package(Matlab REQUIRED) # Construct the absolute path to mexfcn's souce files -set(mexfcn_sources mexfcn.cc featherreadmex.cc feather_reader.cc - featherwritemex.cc feather_writer.cc util/handle_status.cc - util/unicode_conversion.cc) -list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/) +set(mexfcn_sources mex/mexfcn.cc feather/featherreadmex.cc feather/feather_reader.cc + feather/featherwritemex.cc feather/feather_writer.cc feather/util/handle_status.cc + feather/util/unicode_conversion.cc) +list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/matlab/arrow/) # Build mexDispatcher MEX binary matlab_add_mex(R2018a diff --git a/matlab/src/feather_reader.cc b/matlab/src/cpp/matlab/arrow/feather/feather_reader.cc similarity index 100% rename from matlab/src/feather_reader.cc rename to matlab/src/cpp/matlab/arrow/feather/feather_reader.cc diff --git a/matlab/src/feather_reader.h b/matlab/src/cpp/matlab/arrow/feather/feather_reader.h similarity index 100% rename from matlab/src/feather_reader.h rename to matlab/src/cpp/matlab/arrow/feather/feather_reader.h diff --git a/matlab/src/feather_writer.cc b/matlab/src/cpp/matlab/arrow/feather/feather_writer.cc similarity index 100% rename from matlab/src/feather_writer.cc rename to matlab/src/cpp/matlab/arrow/feather/feather_writer.cc diff --git a/matlab/src/feather_writer.h b/matlab/src/cpp/matlab/arrow/feather/feather_writer.h similarity index 100% rename from matlab/src/feather_writer.h rename to matlab/src/cpp/matlab/arrow/feather/feather_writer.h diff --git a/matlab/src/featherreadmex.cc b/matlab/src/cpp/matlab/arrow/feather/featherreadmex.cc similarity index 100% rename from matlab/src/featherreadmex.cc rename to matlab/src/cpp/matlab/arrow/feather/featherreadmex.cc diff --git a/matlab/src/featherreadmex.h b/matlab/src/cpp/matlab/arrow/feather/featherreadmex.h similarity index 100% rename from matlab/src/featherreadmex.h rename to matlab/src/cpp/matlab/arrow/feather/featherreadmex.h diff --git a/matlab/src/featherwritemex.cc b/matlab/src/cpp/matlab/arrow/feather/featherwritemex.cc similarity index 100% rename from matlab/src/featherwritemex.cc rename to matlab/src/cpp/matlab/arrow/feather/featherwritemex.cc diff --git a/matlab/src/featherwritemex.h b/matlab/src/cpp/matlab/arrow/feather/featherwritemex.h similarity index 100% rename from matlab/src/featherwritemex.h rename to matlab/src/cpp/matlab/arrow/feather/featherwritemex.h diff --git a/matlab/src/matlab_traits.h b/matlab/src/cpp/matlab/arrow/feather/matlab_traits.h similarity index 100% rename from matlab/src/matlab_traits.h rename to matlab/src/cpp/matlab/arrow/feather/matlab_traits.h diff --git a/matlab/src/util/handle_status.cc b/matlab/src/cpp/matlab/arrow/feather/util/handle_status.cc similarity index 100% rename from matlab/src/util/handle_status.cc rename to matlab/src/cpp/matlab/arrow/feather/util/handle_status.cc diff --git a/matlab/src/util/handle_status.h b/matlab/src/cpp/matlab/arrow/feather/util/handle_status.h similarity index 100% rename from matlab/src/util/handle_status.h rename to matlab/src/cpp/matlab/arrow/feather/util/handle_status.h diff --git a/matlab/src/util/unicode_conversion.cc b/matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.cc similarity index 100% rename from matlab/src/util/unicode_conversion.cc rename to matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.cc diff --git a/matlab/src/util/unicode_conversion.h b/matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.h similarity index 100% rename from matlab/src/util/unicode_conversion.h rename to matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.h diff --git a/matlab/src/mexfcn.cc b/matlab/src/cpp/matlab/arrow/mex/mexfcn.cc similarity index 100% rename from matlab/src/mexfcn.cc rename to matlab/src/cpp/matlab/arrow/mex/mexfcn.cc From 01bb903c7fe469155babba25139a28d865ba4021 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 10:59:48 -0400 Subject: [PATCH 07/33] Rename matlab/src/cpp/matlab/arrow -> matlab/src/cpp/arrow/matlab --- .../matlab}/feather/feather_reader.cc | 0 .../matlab}/feather/feather_reader.h | 0 .../matlab}/feather/feather_writer.cc | 0 .../matlab}/feather/feather_writer.h | 0 .../matlab}/feather/featherreadmex.cc | 0 .../matlab}/feather/featherreadmex.h | 0 .../matlab}/feather/featherwritemex.cc | 0 .../matlab}/feather/featherwritemex.h | 0 .../matlab}/feather/matlab_traits.h | 0 .../matlab}/feather/util/handle_status.cc | 0 .../matlab}/feather/util/handle_status.h | 0 .../feather/util/unicode_conversion.cc | 0 .../matlab}/feather/util/unicode_conversion.h | 0 matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 69 +++++++++++++++ matlab/src/cpp/matlab/arrow/mex/mexfcn.cc | 88 ------------------- matlab/src/{ => matlab}/featherread.m | 0 matlab/src/{ => matlab}/featherwrite.m | 0 17 files changed, 69 insertions(+), 88 deletions(-) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/feather_reader.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/feather_reader.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/feather_writer.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/feather_writer.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/featherreadmex.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/featherreadmex.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/featherwritemex.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/featherwritemex.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/matlab_traits.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/util/handle_status.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/util/handle_status.h (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/util/unicode_conversion.cc (100%) rename matlab/src/cpp/{matlab/arrow => arrow/matlab}/feather/util/unicode_conversion.h (100%) create mode 100644 matlab/src/cpp/arrow/matlab/mex/mexfcn.cc delete mode 100644 matlab/src/cpp/matlab/arrow/mex/mexfcn.cc rename matlab/src/{ => matlab}/featherread.m (100%) rename matlab/src/{ => matlab}/featherwrite.m (100%) diff --git a/matlab/src/cpp/matlab/arrow/feather/feather_reader.cc b/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/feather_reader.cc rename to matlab/src/cpp/arrow/matlab/feather/feather_reader.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/feather_reader.h b/matlab/src/cpp/arrow/matlab/feather/feather_reader.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/feather_reader.h rename to matlab/src/cpp/arrow/matlab/feather/feather_reader.h diff --git a/matlab/src/cpp/matlab/arrow/feather/feather_writer.cc b/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/feather_writer.cc rename to matlab/src/cpp/arrow/matlab/feather/feather_writer.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/feather_writer.h b/matlab/src/cpp/arrow/matlab/feather/feather_writer.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/feather_writer.h rename to matlab/src/cpp/arrow/matlab/feather/feather_writer.h diff --git a/matlab/src/cpp/matlab/arrow/feather/featherreadmex.cc b/matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/featherreadmex.cc rename to matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/featherreadmex.h b/matlab/src/cpp/arrow/matlab/feather/featherreadmex.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/featherreadmex.h rename to matlab/src/cpp/arrow/matlab/feather/featherreadmex.h diff --git a/matlab/src/cpp/matlab/arrow/feather/featherwritemex.cc b/matlab/src/cpp/arrow/matlab/feather/featherwritemex.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/featherwritemex.cc rename to matlab/src/cpp/arrow/matlab/feather/featherwritemex.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/featherwritemex.h b/matlab/src/cpp/arrow/matlab/feather/featherwritemex.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/featherwritemex.h rename to matlab/src/cpp/arrow/matlab/feather/featherwritemex.h diff --git a/matlab/src/cpp/matlab/arrow/feather/matlab_traits.h b/matlab/src/cpp/arrow/matlab/feather/matlab_traits.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/matlab_traits.h rename to matlab/src/cpp/arrow/matlab/feather/matlab_traits.h diff --git a/matlab/src/cpp/matlab/arrow/feather/util/handle_status.cc b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/util/handle_status.cc rename to matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/util/handle_status.h b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/util/handle_status.h rename to matlab/src/cpp/arrow/matlab/feather/util/handle_status.h diff --git a/matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.cc b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.cc rename to matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc diff --git a/matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.h b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h similarity index 100% rename from matlab/src/cpp/matlab/arrow/feather/util/unicode_conversion.h rename to matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc new file mode 100644 index 00000000000..be2301185cf --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -0,0 +1,69 @@ +// 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. + +#include +#include + +#include + +#include "../feather/featherreadmex.h" +#include "../feather/featherwritemex.h" + +using mex_fcn_t = std::function; + +static const std::unordered_map FUNCTION_MAP = +{ + {"featherread", featherreadmex}, + {"featherwrite", featherwrite} +}; + +std::string get_function_name(const mxArray* input) +{ + std::string opname; + if (!mxIsChar(input)) { + mexErrMsgIdAndTxt("MATLAB:arrow:OperationNameDataType", + "The first input argument to 'mexfcn' must be a character vector."); + } + const char* c_str = mxArrayToUTF8String(input); + return std::string{c_str}; +} + +void checkNumArgs(int nrhs) +{ + if (nrhs < 1) { + mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", + "'mexfcn' requires at least one input argument, which must be the name of the C++ MEX to invoke."); + } +} + +mex_fcn_t lookup_function(const std::string& function_name) +{ + auto kv_pair = FUNCTION_MAP.find(function_name); + if (kv_pair == FUNCTION_MAP.end()) { + mexErrMsgIdAndTxt("MATLAB:arrow:UnknownMEXFunction", + "Unrecognized MEX function '%s'", function_name.c_str()); + } + return kv_pair->second; +} + +// MEX gateway function. +void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) +{ + checkNumArgs(nrhs); + auto fcn = lookup_function(get_function_name(prhs[0])); + fcn(nlhs, plhs, nrhs - 1, ++prhs); +} diff --git a/matlab/src/cpp/matlab/arrow/mex/mexfcn.cc b/matlab/src/cpp/matlab/arrow/mex/mexfcn.cc deleted file mode 100644 index ba1d9928ece..00000000000 --- a/matlab/src/cpp/matlab/arrow/mex/mexfcn.cc +++ /dev/null @@ -1,88 +0,0 @@ -// 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. - -#include -#include - -#include - -#include "featherreadmex.h" -#include "featherwritemex.h" - -enum class OperationCode -{ - FEATHER_READ, - FEATHER_WRITE, - INVALID_OPERATION -}; - -static const std::unordered_map OPERATION_CODE_MAP = -{ - {u8"featherread", OperationCode::FEATHER_READ}, - {u8"featherwrite", OperationCode::FEATHER_WRITE} -}; - - -OperationCode opname_to_opcode(const std::string& opname) -{ - auto kv_pair = OPERATION_CODE_MAP.find(opname); - if (kv_pair == OPERATION_CODE_MAP.end()) { - return OperationCode::INVALID_OPERATION; - } - return kv_pair->second; -} - -std::string get_opname(const mxArray* input) -{ - std::string opname; - if (!mxIsChar(input)) { - mexErrMsgIdAndTxt("MATLAB:arrow:OperationNameDataType", - "The first input to 'mexDispatcher' must be a char vector."); - } - const char* c_str = mxArrayToUTF8String(input); - if (!c_str) { - mexErrMsgIdAndTxt("MATLAB:arrow:NullOperationName", - "The function name passed to ''mexDispatcher' cannot be null."); - } - return std::string{c_str}; -} - -void checkNumArgs(int nrhs) -{ - if (nrhs < 1) { - mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", - "'mexDispatcher' requires at least one input argument."); - } -} - -// MEX gateway function. -void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) -{ - checkNumArgs(nrhs); - const std::string opname = get_opname(prhs[0]); - OperationCode opcode = opname_to_opcode(opname); - switch (opcode) { - case OperationCode::FEATHER_READ: - featherreadmex(nlhs, plhs, nrhs - 1, ++prhs); - break; - case OperationCode::FEATHER_WRITE: - featherwrite(nlhs, plhs, nrhs - 1, ++prhs); - break; - case OperationCode::INVALID_OPERATION: - break; - } -} diff --git a/matlab/src/featherread.m b/matlab/src/matlab/featherread.m similarity index 100% rename from matlab/src/featherread.m rename to matlab/src/matlab/featherread.m diff --git a/matlab/src/featherwrite.m b/matlab/src/matlab/featherwrite.m similarity index 100% rename from matlab/src/featherwrite.m rename to matlab/src/matlab/featherwrite.m From 4e74b5c60ecef15e0a93830248ab96f865a88316 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 11:11:58 -0400 Subject: [PATCH 08/33] Update the tests to add the right folder to the MATLAB Search Path --- matlab/src/{ => matlab}/+mlarrow/+util/createMetadataStruct.m | 0 matlab/src/{ => matlab}/+mlarrow/+util/createVariableStruct.m | 0 .../+mlarrow/+util/makeValidMATLABTableVariableNames.m | 0 matlab/src/{ => matlab}/+mlarrow/+util/table2mlarrow.m | 0 matlab/test/tfeather.m | 2 +- matlab/test/tfeathermex.m | 2 +- 6 files changed, 2 insertions(+), 2 deletions(-) rename matlab/src/{ => matlab}/+mlarrow/+util/createMetadataStruct.m (100%) rename matlab/src/{ => matlab}/+mlarrow/+util/createVariableStruct.m (100%) rename matlab/src/{ => matlab}/+mlarrow/+util/makeValidMATLABTableVariableNames.m (100%) rename matlab/src/{ => matlab}/+mlarrow/+util/table2mlarrow.m (100%) diff --git a/matlab/src/+mlarrow/+util/createMetadataStruct.m b/matlab/src/matlab/+mlarrow/+util/createMetadataStruct.m similarity index 100% rename from matlab/src/+mlarrow/+util/createMetadataStruct.m rename to matlab/src/matlab/+mlarrow/+util/createMetadataStruct.m diff --git a/matlab/src/+mlarrow/+util/createVariableStruct.m b/matlab/src/matlab/+mlarrow/+util/createVariableStruct.m similarity index 100% rename from matlab/src/+mlarrow/+util/createVariableStruct.m rename to matlab/src/matlab/+mlarrow/+util/createVariableStruct.m diff --git a/matlab/src/+mlarrow/+util/makeValidMATLABTableVariableNames.m b/matlab/src/matlab/+mlarrow/+util/makeValidMATLABTableVariableNames.m similarity index 100% rename from matlab/src/+mlarrow/+util/makeValidMATLABTableVariableNames.m rename to matlab/src/matlab/+mlarrow/+util/makeValidMATLABTableVariableNames.m diff --git a/matlab/src/+mlarrow/+util/table2mlarrow.m b/matlab/src/matlab/+mlarrow/+util/table2mlarrow.m similarity index 100% rename from matlab/src/+mlarrow/+util/table2mlarrow.m rename to matlab/src/matlab/+mlarrow/+util/table2mlarrow.m diff --git a/matlab/test/tfeather.m b/matlab/test/tfeather.m index 0aa884dcfb2..e21f3f3b4fa 100755 --- a/matlab/test/tfeather.m +++ b/matlab/test/tfeather.m @@ -23,7 +23,7 @@ function addFeatherFunctionsToMATLABPath(testCase) % Add Feather test utilities to the MATLAB path. testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'src'))); + testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); % Add mexfcn to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'build'))); % mexfcn must be on the MATLAB path. diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index 97277fdc031..6dfaeed0746 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -23,7 +23,7 @@ function addFeatherFunctionsToMATLABPath(testCase) % Add Feather test utilities to the MATLAB path. testCase.applyFixture(PathFixture('util')); % Add featherread and featherwrite to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'src'))); + testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); % Add mexfcn to the MATLAB path. testCase.applyFixture(PathFixture(fullfile('..', 'build'))); % mexfcn must be on the MATLAB path. From e7c628bfa811e723e8e9d868026fed1d98d5887f Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 11:15:11 -0400 Subject: [PATCH 09/33] Fix CMakeLists.txt --- matlab/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 3142aca0e70..76455594c97 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -232,7 +232,7 @@ find_package(Matlab REQUIRED) set(mexfcn_sources mex/mexfcn.cc feather/featherreadmex.cc feather/feather_reader.cc feather/featherwritemex.cc feather/feather_writer.cc feather/util/handle_status.cc feather/util/unicode_conversion.cc) -list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/matlab/arrow/) +list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/) # Build mexDispatcher MEX binary matlab_add_mex(R2018a From 55ad79c3de699ce0049c4c81cd40a57b5b84a864 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 11:22:11 -0400 Subject: [PATCH 10/33] Rename +mlarrow package to +arrow --- .../{+mlarrow => +arrow}/+util/createMetadataStruct.m | 0 .../{+mlarrow => +arrow}/+util/createVariableStruct.m | 0 .../+util/makeValidMATLABTableVariableNames.m | 0 .../src/matlab/{+mlarrow => +arrow}/+util/table2mlarrow.m | 2 +- matlab/src/matlab/featherread.m | 2 +- matlab/src/matlab/featherwrite.m | 2 +- matlab/test/tfeathermex.m | 6 +++--- matlab/test/util/createVariablesAndMetadataStructs.m | 2 +- 8 files changed, 7 insertions(+), 7 deletions(-) rename matlab/src/matlab/{+mlarrow => +arrow}/+util/createMetadataStruct.m (100%) rename matlab/src/matlab/{+mlarrow => +arrow}/+util/createVariableStruct.m (100%) rename matlab/src/matlab/{+mlarrow => +arrow}/+util/makeValidMATLABTableVariableNames.m (100%) rename matlab/src/matlab/{+mlarrow => +arrow}/+util/table2mlarrow.m (99%) diff --git a/matlab/src/matlab/+mlarrow/+util/createMetadataStruct.m b/matlab/src/matlab/+arrow/+util/createMetadataStruct.m similarity index 100% rename from matlab/src/matlab/+mlarrow/+util/createMetadataStruct.m rename to matlab/src/matlab/+arrow/+util/createMetadataStruct.m diff --git a/matlab/src/matlab/+mlarrow/+util/createVariableStruct.m b/matlab/src/matlab/+arrow/+util/createVariableStruct.m similarity index 100% rename from matlab/src/matlab/+mlarrow/+util/createVariableStruct.m rename to matlab/src/matlab/+arrow/+util/createVariableStruct.m diff --git a/matlab/src/matlab/+mlarrow/+util/makeValidMATLABTableVariableNames.m b/matlab/src/matlab/+arrow/+util/makeValidMATLABTableVariableNames.m similarity index 100% rename from matlab/src/matlab/+mlarrow/+util/makeValidMATLABTableVariableNames.m rename to matlab/src/matlab/+arrow/+util/makeValidMATLABTableVariableNames.m diff --git a/matlab/src/matlab/+mlarrow/+util/table2mlarrow.m b/matlab/src/matlab/+arrow/+util/table2mlarrow.m similarity index 99% rename from matlab/src/matlab/+mlarrow/+util/table2mlarrow.m rename to matlab/src/matlab/+arrow/+util/table2mlarrow.m index 36e4d1d15a9..391b0603ea2 100644 --- a/matlab/src/matlab/+mlarrow/+util/table2mlarrow.m +++ b/matlab/src/matlab/+arrow/+util/table2mlarrow.m @@ -43,7 +43,7 @@ % implied. See the License for the specific language governing % permissions and limitations under the License. -import mlarrow.util.*; +import arrow.util.*; % Struct array representing the underlying data of each variable % in the given table. diff --git a/matlab/src/matlab/featherread.m b/matlab/src/matlab/featherread.m index deb462c3f93..1d77cd30ed0 100644 --- a/matlab/src/matlab/featherread.m +++ b/matlab/src/matlab/featherread.m @@ -23,7 +23,7 @@ % specific language governing permissions and limitations % under the License. -import mlarrow.util.*; +import arrow.util.*; % Validate input arguments. narginchk(1, 1); diff --git a/matlab/src/matlab/featherwrite.m b/matlab/src/matlab/featherwrite.m index e858ac9c297..361d7b62180 100644 --- a/matlab/src/matlab/featherwrite.m +++ b/matlab/src/matlab/featherwrite.m @@ -23,7 +23,7 @@ function featherwrite(filename, t) % specific language governing permissions and limitations % under the License. -import mlarrow.util.table2mlarrow; +import arrow.util.table2mlarrow; % Validate input arguments. narginchk(2, 2); diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index 6dfaeed0746..62139a8d472 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -54,10 +54,10 @@ function InvalidMATLABTableVariableNames(testCase) filename = fullfile(pwd, 'temp.feather'); % Create a table with an invalid MATLAB table variable name. - invalidVariable = mlarrow.util.createVariableStruct('double', 1, true, '@'); - validVariable = mlarrow.util.createVariableStruct('double', 1, true, 'Valid'); + invalidVariable = arrow.util.createVariableStruct('double', 1, true, '@'); + validVariable = arrow.util.createVariableStruct('double', 1, true, 'Valid'); variables = [invalidVariable, validVariable]; - metadata = mlarrow.util.createMetadataStruct(1, 2); + metadata = arrow.util.createMetadataStruct(1, 2); mexfcn('featherwrite', filename, variables, metadata); t = featherread(filename); diff --git a/matlab/test/util/createVariablesAndMetadataStructs.m b/matlab/test/util/createVariablesAndMetadataStructs.m index 0c60cbfbbcc..a26bfe45a3e 100644 --- a/matlab/test/util/createVariablesAndMetadataStructs.m +++ b/matlab/test/util/createVariablesAndMetadataStructs.m @@ -17,7 +17,7 @@ % implied. See the License for the specific language governing % permissions and limitations under the License. -import mlarrow.util.*; +import arrow.util.*; type = 'uint8'; data = uint8([1; 2; 3]); From 291db9b7fbe0c3ab65858beecc255f5f099e76fd Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 11:40:31 -0400 Subject: [PATCH 11/33] Add misssing include statement (#include ) --- matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index be2301185cf..886324b1487 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -16,6 +16,7 @@ // under the License. #include +#include #include #include From bbd576098ee2b20f9ad76531e8ca9e47d2566ab4 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 13:03:38 -0400 Subject: [PATCH 12/33] Add unit tests for mexfcn --- matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 2 +- matlab/test/tmexfcn.m | 65 +++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 matlab/test/tmexfcn.m diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index 886324b1487..dc87aa90c74 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -36,7 +36,7 @@ std::string get_function_name(const mxArray* input) { std::string opname; if (!mxIsChar(input)) { - mexErrMsgIdAndTxt("MATLAB:arrow:OperationNameDataType", + mexErrMsgIdAndTxt("MATLAB:arrow:FunctionNameDataType", "The first input argument to 'mexfcn' must be a character vector."); } const char* c_str = mxArrayToUTF8String(input); diff --git a/matlab/test/tmexfcn.m b/matlab/test/tmexfcn.m new file mode 100644 index 00000000000..ef0ca686969 --- /dev/null +++ b/matlab/test/tmexfcn.m @@ -0,0 +1,65 @@ +classdef tmexfcn < matlab.unittest.TestCase + % Tests for mexfcn. + + % 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. + + methods(TestClassSetup) + function addFeatherFunctionsToMATLABPath(testCase) + import matlab.unittest.fixtures.PathFixture + % Add Feather test utilities to the MATLAB path. + testCase.applyFixture(PathFixture('util')); + % Add featherread and featherwrite to the MATLAB path. + testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); + % Add mexfcn to the MATLAB path. + testCase.applyFixture(PathFixture(fullfile('..', 'build'))); + % mexfcn must be on the MATLAB path. + testCase.assertTrue(~isempty(which('mexfcn')), ... + '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); + end + end + + methods(Test) + function UnknownMEXFunctionError(testCase) + % Verifies mexfcn throws an error if an unkown MEX function name + % is passed to it. + errID = "MATLAB:arrow:UnknownMEXFunction"; + fcn = @()mexfcn('NotAFunction'); + testCase.verifyError(fcn, errID); + end + + function TooFewInputArguementsError(testCase) + % Verifies mexfcn throws an error if zero input arguments are + % passed it. + errID = "MATLAB:arrow:minrhs"; + fcn = @()mexfcn; + testCase.verifyError(fcn, errID); + end + + function InvalidFunctionNameDataTypeError(testCase) + % Verifies mexfcn throws an error if the first input argument is + % not a character vector. + errID = "MATLAB:arrow:FunctionNameDataType"; + fcn = @()mexfcn(1); + testCase.verifyError(fcn, errID); + + fcn = @()mexfcn(categorical); + testCase.verifyError(fcn, errID); + + fcn = @()mexfcn(datetime(2021, 9, 10)); + testCase.verifyError(fcn, errID); + end + end +end From 234f1555855ba6f1425962f62da995154e71d1a3 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 13:11:53 -0400 Subject: [PATCH 13/33] Consolidate featherreadmex, featherreadwritemex cc and h files into feather_functions.cc and feather_functions.h --- ...eatherwritemex.cc => feather_functions.cc} | 17 +++++++-- .../{featherreadmex.h => feather_functions.h} | 2 ++ .../arrow/matlab/feather/featherreadmex.cc | 36 ------------------- .../arrow/matlab/feather/featherwritemex.h | 20 ----------- matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 5 ++- 5 files changed, 19 insertions(+), 61 deletions(-) rename matlab/src/cpp/arrow/matlab/feather/{featherwritemex.cc => feather_functions.cc} (70%) rename matlab/src/cpp/arrow/matlab/feather/{featherreadmex.h => feather_functions.h} (91%) delete mode 100644 matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc delete mode 100644 matlab/src/cpp/arrow/matlab/feather/featherwritemex.h diff --git a/matlab/src/cpp/arrow/matlab/feather/featherwritemex.cc b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc similarity index 70% rename from matlab/src/cpp/arrow/matlab/feather/featherwritemex.cc rename to matlab/src/cpp/arrow/matlab/feather/feather_functions.cc index cade9079d4a..a0fa4117067 100644 --- a/matlab/src/cpp/arrow/matlab/feather/featherwritemex.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc @@ -17,11 +17,11 @@ #include -#include "featherwritemex.h" #include "feather_writer.h" +#include "feather_reader.h" +#include "feather_functions.h" #include "util/handle_status.h" -// MEX gateway function. This is the entry point for featherwritemex.cc. void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; @@ -33,3 +33,16 @@ void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { // Write the Feather file table variables and table metadata from MATLAB. arrow::matlab::util::HandleStatus(feather_writer->WriteVariables(prhs[1], prhs[2])); } + +void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { + const std::string filename{mxArrayToUTF8String(prhs[0])}; + + // Read the given Feather file into memory. + std::shared_ptr feather_reader{nullptr}; + arrow::matlab::util::HandleStatus( + arrow::matlab::FeatherReader::Open(filename, &feather_reader)); + + // Return the Feather file table variables and table metadata to MATLAB. + plhs[0] = feather_reader->ReadVariables(); + plhs[1] = feather_reader->ReadMetadata(); +} \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/featherreadmex.h b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h similarity index 91% rename from matlab/src/cpp/arrow/matlab/feather/featherreadmex.h rename to matlab/src/cpp/arrow/matlab/feather/feather_functions.h index d80b7c143f1..a3a069d22e6 100644 --- a/matlab/src/cpp/arrow/matlab/feather/featherreadmex.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h @@ -17,4 +17,6 @@ #include +void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); + void featherreadmex(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc b/matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc deleted file mode 100644 index 874c26682dc..00000000000 --- a/matlab/src/cpp/arrow/matlab/feather/featherreadmex.cc +++ /dev/null @@ -1,36 +0,0 @@ -// 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. - -#include - -#include "featherreadmex.h" -#include "feather_reader.h" -#include "util/handle_status.h" - -// MEX gateway function. This is the entry point for featherreadmex.cpp. -void featherreadmex(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { - const std::string filename{mxArrayToUTF8String(prhs[0])}; - - // Read the given Feather file into memory. - std::shared_ptr feather_reader{nullptr}; - arrow::matlab::util::HandleStatus( - arrow::matlab::FeatherReader::Open(filename, &feather_reader)); - - // Return the Feather file table variables and table metadata to MATLAB. - plhs[0] = feather_reader->ReadVariables(); - plhs[1] = feather_reader->ReadMetadata(); -} diff --git a/matlab/src/cpp/arrow/matlab/feather/featherwritemex.h b/matlab/src/cpp/arrow/matlab/feather/featherwritemex.h deleted file mode 100644 index 205db515c4e..00000000000 --- a/matlab/src/cpp/arrow/matlab/feather/featherwritemex.h +++ /dev/null @@ -1,20 +0,0 @@ -// 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. - -#include - -void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index dc87aa90c74..fe2a34cdf46 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -21,14 +21,13 @@ #include -#include "../feather/featherreadmex.h" -#include "../feather/featherwritemex.h" +#include "../feather/feather_functions.h" using mex_fcn_t = std::function; static const std::unordered_map FUNCTION_MAP = { - {"featherread", featherreadmex}, + {"featherread", featherread}, {"featherwrite", featherwrite} }; From cefbb852b5361a31f4503c9861c62d4707eba2f3 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 13:15:54 -0400 Subject: [PATCH 14/33] Update the souce files list in CMakeLists.txt and rename featherreadmex to featherread in feather_functions.h --- matlab/CMakeLists.txt | 6 +++--- matlab/src/cpp/arrow/matlab/feather/feather_functions.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 76455594c97..43aaeb2e02c 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -229,9 +229,9 @@ endif() find_package(Matlab REQUIRED) # Construct the absolute path to mexfcn's souce files -set(mexfcn_sources mex/mexfcn.cc feather/featherreadmex.cc feather/feather_reader.cc - feather/featherwritemex.cc feather/feather_writer.cc feather/util/handle_status.cc - feather/util/unicode_conversion.cc) +set(mexfcn_sources mex/mexfcn.cc feather/feather_reader.cc feather/feather_writer.cc + feather/feather_functions.cc feather/util/handle_status.cc + feather/util/unicode_conversion.cc) list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/) # Build mexDispatcher MEX binary diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h index a3a069d22e6..1c884aa3975 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h @@ -19,4 +19,4 @@ void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); -void featherreadmex(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file +void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file From 963735582e725e1b035b668e6aaaeeeb7c37097f Mon Sep 17 00:00:00 2001 From: sgilmore Date: Fri, 10 Sep 2021 15:45:16 -0400 Subject: [PATCH 15/33] Add feather as a namespace within arrow::matlab --- .../src/cpp/arrow/matlab/feather/feather_functions.cc | 6 +++++- .../src/cpp/arrow/matlab/feather/feather_functions.h | 6 +++++- matlab/src/cpp/arrow/matlab/feather/feather_reader.cc | 6 ++---- matlab/src/cpp/arrow/matlab/feather/feather_reader.h | 6 ++---- matlab/src/cpp/arrow/matlab/feather/feather_writer.cc | 6 ++---- matlab/src/cpp/arrow/matlab/feather/feather_writer.h | 6 ++---- .../src/cpp/arrow/matlab/feather/util/handle_status.h | 11 +++++------ .../arrow/matlab/feather/util/unicode_conversion.cc | 8 ++------ .../arrow/matlab/feather/util/unicode_conversion.h | 10 ++++------ matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 4 ++-- 10 files changed, 31 insertions(+), 38 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc index a0fa4117067..b177cb03909 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc @@ -22,6 +22,8 @@ #include "feather_functions.h" #include "util/handle_status.h" +namespace arrow::matlab::feather { + void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; @@ -45,4 +47,6 @@ void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { // Return the Feather file table variables and table metadata to MATLAB. plhs[0] = feather_reader->ReadVariables(); plhs[1] = feather_reader->ReadMetadata(); -} \ No newline at end of file +} + +} // namespace arrow::matlab::feather \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h index 1c884aa3975..8de8537e097 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h @@ -17,6 +17,10 @@ #include +namespace arrow::matlab::feather { + void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); -void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); \ No newline at end of file +void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); + +} // namespace arrow::matlab::father \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc b/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc index 1cbb50541e7..604189dcfc0 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc @@ -37,8 +37,7 @@ #include "util/handle_status.h" #include "util/unicode_conversion.h" -namespace arrow { -namespace matlab { +namespace arrow::matlab::feather { namespace internal { // Read the name of variable i from the Feather file as a mxArray*. @@ -273,5 +272,4 @@ mxArray* FeatherReader::ReadVariables() { return variables; } -} // namespace matlab -} // namespace arrow +} // namespace arrow::matlab::feather diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_reader.h b/matlab/src/cpp/arrow/matlab/feather/feather_reader.h index 197e470bf6e..0ee43eb0055 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_reader.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_reader.h @@ -25,8 +25,7 @@ #include #include -namespace arrow { -namespace matlab { +namespace arrow::matlab::feather { class FeatherReader { public: @@ -71,5 +70,4 @@ class FeatherReader { std::string description_; }; -} // namespace matlab -} // namespace arrow +} // namespace arrow::matlab::feather diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc b/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc index 1a76ada1995..e4abacf0d0f 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc @@ -36,8 +36,7 @@ #include "matlab_traits.h" #include "util/handle_status.h" -namespace arrow { -namespace matlab { +namespace arrow::matlab::feather { namespace internal { // Returns the arrow::DataType that corresponds to the input type string @@ -362,5 +361,4 @@ Status FeatherWriter::WriteVariables(const mxArray* variables, const mxArray* me return ipc::feather::WriteTable(*table, file_output_stream_.get(), write_props); } -} // namespace matlab -} // namespace arrow +} // namespace arrow::matlab::feather diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_writer.h b/matlab/src/cpp/arrow/matlab/feather/feather_writer.h index a35b1434340..e314ab00bcb 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_writer.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_writer.h @@ -25,8 +25,7 @@ #include #include -namespace arrow { -namespace matlab { +namespace arrow::matlab::feather { class FeatherWriter { public: @@ -64,5 +63,4 @@ class FeatherWriter { std::shared_ptr file_output_stream_; }; -} // namespace matlab -} // namespace arrow +} // namespace arrow::matlab::feather diff --git a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h index 7212114a1d1..7649bd62f82 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h +++ b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h @@ -19,14 +19,13 @@ #include -namespace arrow { -namespace matlab { -namespace util { +namespace arrow::matlab::feather::util { + // Terminates execution and returns to the MATLAB prompt, // displaying an error message if the given status // indicates that an error has occurred. void HandleStatus(const Status& status); -} // namespace util -} // namespace matlab -} // namespace arrow + +} // namespace arrow::matlab::feather::util + diff --git a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc index 01c2e4b9451..bd139f2ed16 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc +++ b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc @@ -20,9 +20,7 @@ #include "unicode_conversion.h" -namespace arrow { -namespace matlab { -namespace util { +namespace namespace arrow::matlab::feather::util { mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string) { // Get pointers to the start and end of the std::string data. @@ -58,6 +56,4 @@ mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string) { return character_matrix; } -} // namespace util -} // namespace matlab -} // namespace arrow +} // namespace arrow::matlab::feather::util diff --git a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h index fa905cbf0ba..6805cb71906 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h +++ b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h @@ -20,13 +20,11 @@ #include #include -namespace arrow { -namespace matlab { -namespace util { +namespace arrow::matlab::feather::util { + // Converts a UTF-8 encoded std::string to a heap-allocated UTF-16 encoded // mxCharArray. mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string); -} // namespace util -} // namespace matlab -} // namespace arrow + +} // namespace arrow::matlab::feather::util diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index fe2a34cdf46..8ca4bcddbf0 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -27,8 +27,8 @@ using mex_fcn_t = std::function FUNCTION_MAP = { - {"featherread", featherread}, - {"featherwrite", featherwrite} + {"featherread", arrow::matlab::feather::featherread}, + {"featherwrite", arrow::matlab::feather::featherwrite} }; std::string get_function_name(const mxArray* input) From 0991d51f60c8ec31f65b9a0a87b3560c304fcf3c Mon Sep 17 00:00:00 2001 From: sgilmore Date: Mon, 13 Sep 2021 09:51:13 -0400 Subject: [PATCH 16/33] Fix compiler errors and clang-format the cc and h files --- .../arrow/matlab/feather/feather_functions.cc | 25 ++++--- .../arrow/matlab/feather/feather_functions.h | 8 ++- .../arrow/matlab/feather/feather_reader.cc | 23 ++++--- .../cpp/arrow/matlab/feather/feather_reader.h | 14 ++-- .../arrow/matlab/feather/feather_writer.cc | 25 ++++--- .../cpp/arrow/matlab/feather/feather_writer.h | 14 ++-- .../cpp/arrow/matlab/feather/matlab_traits.h | 2 - .../matlab/feather/util/handle_status.cc | 4 +- .../arrow/matlab/feather/util/handle_status.h | 12 ++-- .../matlab/feather/util/unicode_conversion.cc | 24 ++++--- .../matlab/feather/util/unicode_conversion.h | 14 ++-- matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 68 +++++++++---------- 12 files changed, 134 insertions(+), 99 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc index b177cb03909..f5b5a394083 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.cc @@ -15,38 +15,41 @@ // specific language governing permissions and limitations // under the License. +#include "feather_functions.h" + #include -#include "feather_writer.h" #include "feather_reader.h" -#include "feather_functions.h" +#include "feather_writer.h" #include "util/handle_status.h" -namespace arrow::matlab::feather { +namespace arrow { +namespace matlab { +namespace feather { void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; // Open a Feather file at the provided file path for writing. - std::shared_ptr feather_writer{nullptr}; - arrow::matlab::util::HandleStatus( - arrow::matlab::FeatherWriter::Open(filename, &feather_writer)); + std::shared_ptr feather_writer{nullptr}; + util::HandleStatus(FeatherWriter::Open(filename, &feather_writer)); // Write the Feather file table variables and table metadata from MATLAB. - arrow::matlab::util::HandleStatus(feather_writer->WriteVariables(prhs[1], prhs[2])); + util::HandleStatus(feather_writer->WriteVariables(prhs[1], prhs[2])); } void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { const std::string filename{mxArrayToUTF8String(prhs[0])}; // Read the given Feather file into memory. - std::shared_ptr feather_reader{nullptr}; - arrow::matlab::util::HandleStatus( - arrow::matlab::FeatherReader::Open(filename, &feather_reader)); + std::shared_ptr feather_reader{nullptr}; + util::HandleStatus(FeatherReader::Open(filename, &feather_reader)); // Return the Feather file table variables and table metadata to MATLAB. plhs[0] = feather_reader->ReadVariables(); plhs[1] = feather_reader->ReadMetadata(); } -} // namespace arrow::matlab::feather \ No newline at end of file +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h index 8de8537e097..4e47243b3ae 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h @@ -17,10 +17,14 @@ #include -namespace arrow::matlab::feather { +namespace arrow { +namespace matlab { +namespace feather { void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); -} // namespace arrow::matlab::father \ No newline at end of file +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc b/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc index 604189dcfc0..413b7f153e9 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_reader.cc @@ -15,9 +15,6 @@ // specific language governing permissions and limitations // under the License. -#include -#include - #include "feather_reader.h" #include @@ -33,16 +30,21 @@ #include #include +#include +#include + #include "matlab_traits.h" #include "util/handle_status.h" #include "util/unicode_conversion.h" -namespace arrow::matlab::feather { +namespace arrow { +namespace matlab { +namespace feather { namespace internal { // Read the name of variable i from the Feather file as a mxArray*. mxArray* ReadVariableName(const std::string& column_name) { - return matlab::util::ConvertUTF8StringToUTF16CharMatrix(column_name); + return util::ConvertUTF8StringToUTF16CharMatrix(column_name); } template @@ -56,8 +58,7 @@ mxArray* ReadNumericVariableData(const std::shared_ptr& column) { mxArray* variable_data = mxCreateNumericMatrix(column->length(), 1, matlab_class_id, mxREAL); - auto arrow_numeric_array = - std::static_pointer_cast(column); + auto arrow_numeric_array = std::static_pointer_cast(column); // Get a raw pointer to the Arrow array data. const MatlabType* source = arrow_numeric_array->raw_values(); @@ -181,10 +182,10 @@ Status FeatherReader::Open(const std::string& filename, // Open file with given filename as a ReadableFile. ARROW_ASSIGN_OR_RAISE(auto readable_file, io::ReadableFile::Open(filename)); - + // Open the Feather file for reading with a TableReader. ARROW_ASSIGN_OR_RAISE(auto reader, ipc::feather::Reader::Open(readable_file)); - + // Set the internal reader_ object. (*feather_reader)->reader_ = reader; @@ -272,4 +273,6 @@ mxArray* FeatherReader::ReadVariables() { return variables; } -} // namespace arrow::matlab::feather +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_reader.h b/matlab/src/cpp/arrow/matlab/feather/feather_reader.h index 0ee43eb0055..0e2089d3672 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_reader.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_reader.h @@ -17,15 +17,17 @@ #pragma once -#include -#include - #include #include #include #include -namespace arrow::matlab::feather { +#include +#include + +namespace arrow { +namespace matlab { +namespace feather { class FeatherReader { public: @@ -70,4 +72,6 @@ class FeatherReader { std::string description_; }; -} // namespace arrow::matlab::feather +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc b/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc index e4abacf0d0f..f67d8dc2fba 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc +++ b/matlab/src/cpp/arrow/matlab/feather/feather_writer.cc @@ -15,10 +15,6 @@ // specific language governing permissions and limitations // under the License. -#include -#include /* for std::multiplies */ -#include /* for std::accumulate */ - #include "feather_writer.h" #include @@ -33,10 +29,16 @@ #include #include +#include +#include /* for std::multiplies */ +#include /* for std::accumulate */ + #include "matlab_traits.h" #include "util/handle_status.h" -namespace arrow::matlab::feather { +namespace arrow { +namespace matlab { +namespace feather { namespace internal { // Returns the arrow::DataType that corresponds to the input type string @@ -278,7 +280,8 @@ Status FeatherWriter::Open(const std::string& filename, *feather_writer = std::shared_ptr(new FeatherWriter()); // Open a FileOutputStream corresponding to the provided filename. - ARROW_ASSIGN_OR_RAISE((*feather_writer)->file_output_stream_, + ARROW_ASSIGN_OR_RAISE( + (*feather_writer)->file_output_stream_, io::FileOutputStream::Open(filename, &((*feather_writer)->file_output_stream_))); return Status::OK(); } @@ -330,15 +333,15 @@ Status FeatherWriter::WriteVariables(const mxArray* variables, const mxArray* me auto datatype = internal::ConvertMatlabTypeStringToArrowDataType(type_str); auto field = std::make_shared(name_str, datatype); - ARROW_ASSIGN_OR_RAISE(std::shared_ptr validity_bitmap, + ARROW_ASSIGN_OR_RAISE( + std::shared_ptr validity_bitmap, arrow::AllocateResizableBuffer(internal::BitPackedLength(num_rows_))); // Populate bit-packed arrow::Buffer using validity data in the mxArray*. internal::BitPackBuffer(valid, validity_bitmap); // Wrap mxArray data in an arrow::Array of the equivalent type. - auto array = - internal::WriteVariableData(data, type_str, validity_bitmap); + auto array = internal::WriteVariableData(data, type_str, validity_bitmap); // Verify that the arrow::Array has the right number of elements. internal::ValidateNumRows(array->length(), num_rows_); @@ -361,4 +364,6 @@ Status FeatherWriter::WriteVariables(const mxArray* variables, const mxArray* me return ipc::feather::WriteTable(*table, file_output_stream_.get(), write_props); } -} // namespace arrow::matlab::feather +} // namespace feather +} // namespace matlab +} // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_writer.h b/matlab/src/cpp/arrow/matlab/feather/feather_writer.h index e314ab00bcb..0042d4ac694 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_writer.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_writer.h @@ -17,15 +17,17 @@ #pragma once -#include -#include - #include #include #include #include -namespace arrow::matlab::feather { +#include +#include + +namespace arrow { +namespace matlab { +namespace feather { class FeatherWriter { public: @@ -63,4 +65,6 @@ class FeatherWriter { std::shared_ptr file_output_stream_; }; -} // namespace arrow::matlab::feather +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/matlab_traits.h b/matlab/src/cpp/arrow/matlab/feather/matlab_traits.h index a76539fa7a9..8ef5aea933a 100644 --- a/matlab/src/cpp/arrow/matlab/feather/matlab_traits.h +++ b/matlab/src/cpp/arrow/matlab/feather/matlab_traits.h @@ -18,7 +18,6 @@ #pragma once #include - #include namespace arrow { @@ -100,4 +99,3 @@ struct MatlabTraits { } // namespace matlab } // namespace arrow - diff --git a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc index f1c3b7f2598..86d6c1c79cf 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc +++ b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.cc @@ -16,11 +16,11 @@ // under the License. #include - #include namespace arrow { namespace matlab { +namespace feather { namespace util { void HandleStatus(const Status& status) { @@ -86,6 +86,8 @@ void HandleStatus(const Status& status) { } } } + } // namespace util +} // namespace feather } // namespace matlab } // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h index 7649bd62f82..3025b3b2080 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h +++ b/matlab/src/cpp/arrow/matlab/feather/util/handle_status.h @@ -19,13 +19,17 @@ #include -namespace arrow::matlab::feather::util { +namespace arrow { +namespace matlab { +namespace feather { +namespace util { // Terminates execution and returns to the MATLAB prompt, // displaying an error message if the given status // indicates that an error has occurred. void HandleStatus(const Status& status); -} // namespace arrow::matlab::feather::util - - +} // namespace util +} // namespace feather +} // namespace matlab +} // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc index bd139f2ed16..623943d0ca6 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc +++ b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.cc @@ -15,19 +15,22 @@ // specific language governing permissions and limitations // under the License. -#include /* for std::wstring_convert */ -#include /* for std::codecvt_utf8_utf16 */ - #include "unicode_conversion.h" -namespace namespace arrow::matlab::feather::util { +#include /* for std::codecvt_utf8_utf16 */ +#include /* for std::wstring_convert */ + +namespace arrow { +namespace matlab { +namespace feather { +namespace util { mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string) { // Get pointers to the start and end of the std::string data. const char* string_start = utf8_string.c_str(); const char* string_end = string_start + utf8_string.length(); - // Due to this issue on MSVC: https://stackoverflow.com/q/32055357 we cannot + // Due to this issue on MSVC: https://stackoverflow.com/q/32055357 we cannot // directly use a destination type of char16_t. #if _MSC_VER >= 1900 using CharType = int16_t; @@ -41,7 +44,7 @@ mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string) { try { utf16_string = code_converter.from_bytes(string_start, string_end); } catch (...) { - // In the case that any error occurs, just try returning a string in the + // In the case that any error occurs, just try returning a string in the // user's current locale instead. return mxCreateString(string_start); } @@ -50,10 +53,13 @@ mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string) { const mwSize dimensions[2] = {1, utf16_string.size()}; mxArray* character_matrix = mxCreateCharArray(2, dimensions); mxChar* character_matrix_pointer = mxGetChars(character_matrix); - std::copy(utf16_string.data(), utf16_string.data() + utf16_string.size(), - character_matrix_pointer); + std::copy(utf16_string.data(), utf16_string.data() + utf16_string.size(), + character_matrix_pointer); return character_matrix; } -} // namespace arrow::matlab::feather::util +} // namespace util +} // namespace feather +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h index 6805cb71906..8a41f08605a 100644 --- a/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h +++ b/matlab/src/cpp/arrow/matlab/feather/util/unicode_conversion.h @@ -17,14 +17,20 @@ #pragma once -#include #include -namespace arrow::matlab::feather::util { +#include + +namespace arrow { +namespace matlab { +namespace feather { +namespace util { // Converts a UTF-8 encoded std::string to a heap-allocated UTF-16 encoded // mxCharArray. mxArray* ConvertUTF8StringToUTF16CharMatrix(const std::string& utf8_string); -} // namespace arrow::matlab::feather::util - +} // namespace util +} // namespace feather +} // namespace matlab +} // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index 8ca4bcddbf0..b4fc4269585 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -15,55 +15,51 @@ // specific language governing permissions and limitations // under the License. -#include +#include + #include +#include #include -#include - #include "../feather/feather_functions.h" -using mex_fcn_t = std::function; +using mex_fcn_t = + std::function; -static const std::unordered_map FUNCTION_MAP = -{ +static const std::unordered_map FUNCTION_MAP = { {"featherread", arrow::matlab::feather::featherread}, - {"featherwrite", arrow::matlab::feather::featherwrite} -}; + {"featherwrite", arrow::matlab::feather::featherwrite}}; -std::string get_function_name(const mxArray* input) -{ - std::string opname; - if (!mxIsChar(input)) { - mexErrMsgIdAndTxt("MATLAB:arrow:FunctionNameDataType", - "The first input argument to 'mexfcn' must be a character vector."); - } - const char* c_str = mxArrayToUTF8String(input); - return std::string{c_str}; +std::string get_function_name(const mxArray* input) { + std::string opname; + if (!mxIsChar(input)) { + mexErrMsgIdAndTxt("MATLAB:arrow:FunctionNameDataType", + "The first input argument to 'mexfcn' must be a character vector."); + } + const char* c_str = mxArrayToUTF8String(input); + return std::string{c_str}; } -void checkNumArgs(int nrhs) -{ - if (nrhs < 1) { - mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", - "'mexfcn' requires at least one input argument, which must be the name of the C++ MEX to invoke."); - } +void checkNumArgs(int nrhs) { + if (nrhs < 1) { + mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", + "'mexfcn' requires at least one input argument, which must be the " + "name of the C++ MEX to invoke."); + } } -mex_fcn_t lookup_function(const std::string& function_name) -{ - auto kv_pair = FUNCTION_MAP.find(function_name); - if (kv_pair == FUNCTION_MAP.end()) { - mexErrMsgIdAndTxt("MATLAB:arrow:UnknownMEXFunction", - "Unrecognized MEX function '%s'", function_name.c_str()); - } - return kv_pair->second; +mex_fcn_t lookup_function(const std::string& function_name) { + auto kv_pair = FUNCTION_MAP.find(function_name); + if (kv_pair == FUNCTION_MAP.end()) { + mexErrMsgIdAndTxt("MATLAB:arrow:UnknownMEXFunction", "Unrecognized MEX function '%s'", + function_name.c_str()); + } + return kv_pair->second; } // MEX gateway function. -void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) -{ - checkNumArgs(nrhs); - auto fcn = lookup_function(get_function_name(prhs[0])); - fcn(nlhs, plhs, nrhs - 1, ++prhs); +void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { + checkNumArgs(nrhs); + auto fcn = lookup_function(get_function_name(prhs[0])); + fcn(nlhs, plhs, nrhs - 1, ++prhs); } From ffae35ceba3f1f8a174076c770c84242d17cc870 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Mon, 13 Sep 2021 10:41:18 -0400 Subject: [PATCH 17/33] Fix CMakeLists.txt format --- matlab/CMakeLists.txt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 43aaeb2e02c..4b97fd9bf70 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -229,9 +229,13 @@ endif() find_package(Matlab REQUIRED) # Construct the absolute path to mexfcn's souce files -set(mexfcn_sources mex/mexfcn.cc feather/feather_reader.cc feather/feather_writer.cc - feather/feather_functions.cc feather/util/handle_status.cc - feather/util/unicode_conversion.cc) +set(mexfcn_sources + mex/mexfcn.cc + feather/feather_reader.cc + feather/feather_writer.cc + feather/feather_functions.cc + feather/util/handle_status.cc + feather/util/unicode_conversion.cc) list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/) # Build mexDispatcher MEX binary From 7be104b424eafebb87413620e7b0b101a5867e37 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Mon, 13 Sep 2021 14:15:31 -0400 Subject: [PATCH 18/33] Add namespace arrow::matlab::mex Co-authored-by: Kevin Gurney --- matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index b4fc4269585..6700b7cdeec 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -23,12 +23,17 @@ #include "../feather/feather_functions.h" +namespace arrow { +namespace matlab { +namespace mex { + +using namespace feather; + using mex_fcn_t = std::function; static const std::unordered_map FUNCTION_MAP = { - {"featherread", arrow::matlab::feather::featherread}, - {"featherwrite", arrow::matlab::feather::featherwrite}}; + {"featherread", featherread}, {"featherwrite", featherwrite}}; std::string get_function_name(const mxArray* input) { std::string opname; @@ -57,8 +62,14 @@ mex_fcn_t lookup_function(const std::string& function_name) { return kv_pair->second; } +} // namespace mex +} // namespace matlab +} // namespace arrow + // MEX gateway function. void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { + using namespace arrow::matlab::mex; + checkNumArgs(nrhs); auto fcn = lookup_function(get_function_name(prhs[0])); fcn(nlhs, plhs, nrhs - 1, ++prhs); From 47e240480957b65c1372e669bf84d8f36ab8159b Mon Sep 17 00:00:00 2001 From: sgilmore Date: Mon, 13 Sep 2021 14:47:51 -0400 Subject: [PATCH 19/33] Split out mex function utilities Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 8 ++- .../src/cpp/arrow/matlab/mex/mex_functions.h | 40 ++++++++++++++ matlab/src/cpp/arrow/matlab/mex/mex_util.cc | 53 +++++++++++++++++++ matlab/src/cpp/arrow/matlab/mex/mex_util.h | 38 +++++++++++++ matlab/src/cpp/arrow/matlab/mex/mexfcn.cc | 50 +---------------- 5 files changed, 139 insertions(+), 50 deletions(-) create mode 100644 matlab/src/cpp/arrow/matlab/mex/mex_functions.h create mode 100644 matlab/src/cpp/arrow/matlab/mex/mex_util.cc create mode 100644 matlab/src/cpp/arrow/matlab/mex/mex_util.h diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 4b97fd9bf70..c1be0f7abe6 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -228,15 +228,19 @@ endif() # MATLAB is Required find_package(Matlab REQUIRED) +set(CPP_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp) +set(MATLAB_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab) + # Construct the absolute path to mexfcn's souce files set(mexfcn_sources mex/mexfcn.cc + mex/mex_util.cc feather/feather_reader.cc feather/feather_writer.cc feather/feather_functions.cc feather/util/handle_status.cc feather/util/unicode_conversion.cc) -list(TRANSFORM mexfcn_sources PREPEND ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab/) +list(TRANSFORM mexfcn_sources PREPEND ${MATLAB_SOURCE_DIR}/) # Build mexDispatcher MEX binary matlab_add_mex(R2018a @@ -244,6 +248,8 @@ matlab_add_mex(R2018a SRC ${mexfcn_sources} LINK_TO arrow_shared) +target_include_directories(mexfcn PRIVATE ${CPP_SOURCE_DIR}) + # ############################################################################## # C++ Tests # ############################################################################## diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_functions.h b/matlab/src/cpp/arrow/matlab/mex/mex_functions.h new file mode 100644 index 00000000000..aa09fc863b2 --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/mex/mex_functions.h @@ -0,0 +1,40 @@ +// 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. + +#include +#include +#include + +#include + +#include "arrow/matlab/feather/feather_functions.h" + +namespace arrow { +namespace matlab { +namespace mex { + +using namespace arrow::matlab::feather; + +using mex_fcn_t = + std::function; + +static const std::unordered_map FUNCTION_MAP = { + {"featherread", featherread}, {"featherwrite", featherwrite}}; + +} // namespace mex +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_util.cc b/matlab/src/cpp/arrow/matlab/mex/mex_util.cc new file mode 100644 index 00000000000..ba6e87c483e --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/mex/mex_util.cc @@ -0,0 +1,53 @@ +// 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. + +#include "mex_util.h" + +namespace arrow { +namespace matlab { +namespace mex { + +void checkNumArgs(int nrhs) { + if (nrhs < 1) { + mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", + "'mexfcn' requires at least one input argument, which must be the " + "name of the C++ MEX to invoke."); + } +} + +std::string get_function_name(const mxArray* input) { + std::string opname; + if (!mxIsChar(input)) { + mexErrMsgIdAndTxt("MATLAB:arrow:FunctionNameDataType", + "The first input argument to 'mexfcn' must be a character vector."); + } + const char* c_str = mxArrayToUTF8String(input); + return std::string{c_str}; +} + +mex_fcn_t lookup_function(const std::string& function_name) { + auto kv_pair = FUNCTION_MAP.find(function_name); + if (kv_pair == FUNCTION_MAP.end()) { + mexErrMsgIdAndTxt("MATLAB:arrow:UnknownMEXFunction", "Unrecognized MEX function '%s'", + function_name.c_str()); + } + return kv_pair->second; +} + +} // namespace mex +} // namespace matlab +} // namespace arrow \ No newline at end of file diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_util.h b/matlab/src/cpp/arrow/matlab/mex/mex_util.h new file mode 100644 index 00000000000..e17673a4e82 --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/mex/mex_util.h @@ -0,0 +1,38 @@ +// 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. + +#include +#include +#include + +#include + +#include "mex_functions.h" + +namespace arrow { +namespace matlab { +namespace mex { + +void checkNumArgs(int nrhs); + +std::string get_function_name(const mxArray* input); + +mex_fcn_t lookup_function(const std::string& function_name); + +} // namespace mex +} // namespace matlab +} // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc index 6700b7cdeec..c1205b071c1 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mexfcn.cc @@ -17,56 +17,8 @@ #include -#include -#include -#include +#include "mex_util.h" -#include "../feather/feather_functions.h" - -namespace arrow { -namespace matlab { -namespace mex { - -using namespace feather; - -using mex_fcn_t = - std::function; - -static const std::unordered_map FUNCTION_MAP = { - {"featherread", featherread}, {"featherwrite", featherwrite}}; - -std::string get_function_name(const mxArray* input) { - std::string opname; - if (!mxIsChar(input)) { - mexErrMsgIdAndTxt("MATLAB:arrow:FunctionNameDataType", - "The first input argument to 'mexfcn' must be a character vector."); - } - const char* c_str = mxArrayToUTF8String(input); - return std::string{c_str}; -} - -void checkNumArgs(int nrhs) { - if (nrhs < 1) { - mexErrMsgIdAndTxt("MATLAB:arrow:minrhs", - "'mexfcn' requires at least one input argument, which must be the " - "name of the C++ MEX to invoke."); - } -} - -mex_fcn_t lookup_function(const std::string& function_name) { - auto kv_pair = FUNCTION_MAP.find(function_name); - if (kv_pair == FUNCTION_MAP.end()) { - mexErrMsgIdAndTxt("MATLAB:arrow:UnknownMEXFunction", "Unrecognized MEX function '%s'", - function_name.c_str()); - } - return kv_pair->second; -} - -} // namespace mex -} // namespace matlab -} // namespace arrow - -// MEX gateway function. void mexFunction(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]) { using namespace arrow::matlab::mex; From 4ac3c4afbf5eb1e193547617deecf3f6eabc5577 Mon Sep 17 00:00:00 2001 From: sgilmore Date: Mon, 13 Sep 2021 15:21:27 -0400 Subject: [PATCH 20/33] Add skeleton for mex_util_test.cc --- matlab/CMakeLists.txt | 5 +++ .../src/cpp/arrow/matlab/mex/mex_util_test.cc | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index c1be0f7abe6..44775cfa297 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -260,10 +260,15 @@ if(MATLAB_BUILD_TESTS) # Define a test executable target. TODO: Remove the placeholder test. This is # just for testing GoogleTest integration. add_executable(placeholder_test ${CMAKE_SOURCE_DIR}/src/placeholder_test.cc) + add_executable(mex_util_test ${CPP_SOURCE_DIR}/arrow/matlab/mex/mex_util_test.cc) + # Declare a dependency on the GTest::gtest and GTest::gtest_main IMPORTED # targets. target_link_libraries(placeholder_test GTest::gtest GTest::gtest_main) + target_link_libraries(mex_util_test GTest::gtest GTest::gtest_main) # Add a test target. add_test(PlaceholderTestTarget placeholder_test) + add_test(CheckNumArgsTestTarget mex_util_test) + endif() diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc b/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc new file mode 100644 index 00000000000..a4cc985fb61 --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc @@ -0,0 +1,32 @@ +// 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. + +#include + +#include "mex_util.h" + +namespace arrow { +namespace matlab { +namespace mex { +// TODO: Remove this placeholder test. +TEST(CheckNumArgsTests, TooFewArgsError) { + mxArray* input = nullptr; + EXPECT_THROW(checkNumArgs(input), std::exception); +} +} // namespace mex +} // namespace matlab +} // namespace arrow From aa94d2545817c2c2b5693abf879b8831e2d4f895 Mon Sep 17 00:00:00 2001 From: Kevin Gurney Date: Mon, 13 Sep 2021 17:23:43 -0400 Subject: [PATCH 21/33] Factor out C++ code into a separate arrow_matlab shared library. Co-authored-by: Sarah Gilmore --- matlab/CMakeLists.txt | 33 +++++++++++++++---- matlab/src/cpp/arrow/matlab/api/visibility.h | 28 ++++++++++++++++ .../arrow/matlab/feather/feather_functions.h | 10 ++++-- matlab/src/cpp/arrow/matlab/mex/mex_util.h | 10 ++++-- .../src/cpp/arrow/matlab/mex/mex_util_test.cc | 4 +-- 5 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 matlab/src/cpp/arrow/matlab/api/visibility.h diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 44775cfa297..11e9dde406a 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -231,22 +231,34 @@ find_package(Matlab REQUIRED) set(CPP_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp) set(MATLAB_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab) -# Construct the absolute path to mexfcn's souce files -set(mexfcn_sources - mex/mexfcn.cc +set(arrow_matlab_sources mex/mex_util.cc feather/feather_reader.cc feather/feather_writer.cc feather/feather_functions.cc feather/util/handle_status.cc feather/util/unicode_conversion.cc) -list(TRANSFORM mexfcn_sources PREPEND ${MATLAB_SOURCE_DIR}/) +list(TRANSFORM arrow_matlab_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) + +add_library(arrow_matlab SHARED ${arrow_matlab_sources}) +# Declare a dependency on the MEX shared library (libmex.so). +target_link_libraries(arrow_matlab ${Matlab_MEX_LIBRARY}) +target_link_libraries(arrow_matlab arrow_shared) +# Include the MATLAB MEX headers. +target_include_directories(arrow_matlab PRIVATE ${Matlab_INCLUDE_DIRS}) +target_include_directories(arrow_matlab PRIVATE ${CPP_SOURCE_DIR}) +target_include_directories(arrow_matlab PRIVATE ${ARROW_INCLUDE_DIR}) +target_compile_definitions(arrow_matlab PRIVATE ARROW_MATLAB_EXPORTING) + +set(mexfcn_sources + mex/mexfcn.cc) +list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) -# Build mexDispatcher MEX binary +# Build mexfcn MEX binary matlab_add_mex(R2018a NAME mexfcn SRC ${mexfcn_sources} - LINK_TO arrow_shared) + LINK_TO arrow_matlab) target_include_directories(mexfcn PRIVATE ${CPP_SOURCE_DIR}) @@ -265,7 +277,16 @@ if(MATLAB_BUILD_TESTS) # Declare a dependency on the GTest::gtest and GTest::gtest_main IMPORTED # targets. target_link_libraries(placeholder_test GTest::gtest GTest::gtest_main) + + # Declare a dependency on the GTest::gtest and GTest::gtest_main IMPORTED + # targets. target_link_libraries(mex_util_test GTest::gtest GTest::gtest_main) + target_link_libraries(mex_util_test arrow_matlab) + + # Include the MATLAB MEX headers. + target_include_directories(mex_util_test PRIVATE ${Matlab_INCLUDE_DIRS}) + # Include the C++ source headers. + target_include_directories(mex_util_test PRIVATE ${CPP_SOURCE_DIR}) # Add a test target. add_test(PlaceholderTestTarget placeholder_test) diff --git a/matlab/src/cpp/arrow/matlab/api/visibility.h b/matlab/src/cpp/arrow/matlab/api/visibility.h new file mode 100644 index 00000000000..51efac972ee --- /dev/null +++ b/matlab/src/cpp/arrow/matlab/api/visibility.h @@ -0,0 +1,28 @@ +// 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. + +#pragma once + +#if defined(_WIN32) || defined(__CYGWIN__) +#ifdef ARROW_MATLAB_EXPORTING +#define ARROW_MATLAB_EXPORT __declspec(dllexport) +#else +#define ARROW_MATLAB_EXPORT __declspec(dllimport) +#endif +#else // Not Windows +#define ARROW_MATLAB_EXPORT __attribute__((visibility("default"))) +#endif diff --git a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h index 4e47243b3ae..c055e9582c0 100644 --- a/matlab/src/cpp/arrow/matlab/feather/feather_functions.h +++ b/matlab/src/cpp/arrow/matlab/feather/feather_functions.h @@ -15,16 +15,20 @@ // specific language governing permissions and limitations // under the License. +#pragma once + #include +#include "arrow/matlab/api/visibility.h" + namespace arrow { namespace matlab { namespace feather { -void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); +ARROW_MATLAB_EXPORT void featherwrite(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); -void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); +ARROW_MATLAB_EXPORT void featherread(int nlhs, mxArray* plhs[], int nrhs, const mxArray* prhs[]); } // namespace feather } // namespace matlab -} // namespace arrow \ No newline at end of file +} // namespace arrow diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_util.h b/matlab/src/cpp/arrow/matlab/mex/mex_util.h index e17673a4e82..8e02084a288 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mex_util.h +++ b/matlab/src/cpp/arrow/matlab/mex/mex_util.h @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +#pragma once + #include #include #include @@ -23,15 +25,17 @@ #include "mex_functions.h" +#include "arrow/matlab/api/visibility.h" + namespace arrow { namespace matlab { namespace mex { -void checkNumArgs(int nrhs); +ARROW_MATLAB_EXPORT void checkNumArgs(int nrhs); -std::string get_function_name(const mxArray* input); +ARROW_MATLAB_EXPORT std::string get_function_name(const mxArray* input); -mex_fcn_t lookup_function(const std::string& function_name); +ARROW_MATLAB_EXPORT mex_fcn_t lookup_function(const std::string& function_name); } // namespace mex } // namespace matlab diff --git a/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc b/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc index a4cc985fb61..8f2c65c75f3 100644 --- a/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc +++ b/matlab/src/cpp/arrow/matlab/mex/mex_util_test.cc @@ -22,10 +22,8 @@ namespace arrow { namespace matlab { namespace mex { -// TODO: Remove this placeholder test. TEST(CheckNumArgsTests, TooFewArgsError) { - mxArray* input = nullptr; - EXPECT_THROW(checkNumArgs(input), std::exception); + EXPECT_THROW(checkNumArgs(0), std::exception); } } // namespace mex } // namespace matlab From 846afebe4632d56606f51e169d18b7e9de19f9cf Mon Sep 17 00:00:00 2001 From: sgilmore Date: Tue, 14 Sep 2021 16:42:41 -0400 Subject: [PATCH 22/33] Fix CMakeLists.txt to build on mac Store the Configuration Type value for multi-configuration generators in a txt file for use during testing Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 11e9dde406a..b5e46958340 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -228,6 +228,9 @@ endif() # MATLAB is Required find_package(Matlab REQUIRED) +message(STATUS "Mex Library: ${Matlab_MEX_LIBRARY}") +message(STATUS "Mex Include Folder: ${Matlab_INCLUDE_DIRS}") + set(CPP_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp) set(MATLAB_SOURCE_DIR ${CMAKE_SOURCE_DIR}/src/cpp/arrow/matlab) @@ -242,6 +245,7 @@ list(TRANSFORM arrow_matlab_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) add_library(arrow_matlab SHARED ${arrow_matlab_sources}) # Declare a dependency on the MEX shared library (libmex.so). +target_link_libraries(arrow_matlab ${Matlab_MX_LIBRARY}) target_link_libraries(arrow_matlab ${Matlab_MEX_LIBRARY}) target_link_libraries(arrow_matlab arrow_shared) # Include the MATLAB MEX headers. @@ -254,6 +258,15 @@ set(mexfcn_sources mex/mexfcn.cc) list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) +# Store the CMake Configuration Type for multi-configuration generators so +# that we can reference it later to automatically add the correct build +# folder to the MATLAB Search Path +get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) +if(is_multi_config) + add_custom_command(TARGET arrow_matlab POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E echo $ > ${CMAKE_BINARY_DIR}/is_multi_config.txt) +endif() + # Build mexfcn MEX binary matlab_add_mex(R2018a NAME mexfcn From 1cc2eebe3c221c76a3f295ae55adba4de03684c6 Mon Sep 17 00:00:00 2001 From: Fiona La Date: Tue, 16 Nov 2021 16:40:56 -0500 Subject: [PATCH 23/33] On Windows, copy gtest.dll, gtest_main.dll, and arrow.dll to the MATLAB test folder. Add directory that contains libmx.dll and libmex.dll to the %PATH% for running CheckNumArgsTestTarget. Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 226 ++++++++++++++++++++++++++++-------------- 1 file changed, 150 insertions(+), 76 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index b5e46958340..8e8ef5ac47d 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -31,23 +31,30 @@ function(build_arrow) message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}") endif() + set(ARROW_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-prefix") + if(WIN32) - set(ARROW_IMPORTED_TYPE IMPORTED_IMPLIB) - set(ARROW_LIBRARY_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) + # On Windows, the link time suffix (.lib) and runtime library suffixes (.dll) are different. + set(ARROW_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) + # The shared library is located in the "bin" directory. + set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/bin") else() - set(ARROW_IMPORTED_TYPE IMPORTED_LOCATION) - set(ARROW_LIBRARY_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) + # On Linux and Mac, the link time suffix and runtime suffixes are consistent, (.so and .dylib). + set(ARROW_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) + # The shared library is located in the "lib" directory. + set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/lib") endif() - set(ARROW_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-prefix") - set(ARROW_INCLUDE_DIR "${ARROW_PREFIX}/include") - set(ARROW_LIBRARY_DIR "${ARROW_PREFIX}/lib") - set(ARROW_SHARED_LIB - "${ARROW_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}arrow${ARROW_LIBRARY_SUFFIX}") + set(ARROW_LINK_LIB_FILENAME "${CMAKE_IMPORT_LIBRARY_PREFIX}arrow${ARROW_LINK_SUFFIX}") + set(ARROW_LINK_LIB "${ARROW_PREFIX}/lib/${ARROW_LINK_LIB_FILENAME}") + + set(ARROW_SHARED_LIB_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}arrow${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(ARROW_SHARED_LIB "${ARROW_SHARED_LIBRARY_DIR}/${ARROW_SHARED_LIB_FILENAME}") + set(ARROW_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-build") set(ARROW_CMAKE_ARGS "-DCMAKE_INSTALL_PREFIX=${ARROW_PREFIX}" "-DCMAKE_INSTALL_LIBDIR=lib" "-DARROW_BUILD_STATIC=OFF") - set(ARROW_BUILD_BYPRODUCTS "${ARROW_SHARED_LIB}") + set(ARROW_INCLUDE_DIR "${ARROW_PREFIX}/include") # Building the Arrow C++ libraries and bundled GoogleTest binaries requires ExternalProject. include(ExternalProject) @@ -60,13 +67,13 @@ function(build_arrow) SOURCE_DIR "${CMAKE_SOURCE_DIR}/../cpp" BINARY_DIR "${ARROW_BINARY_DIR}" CMAKE_ARGS ${ARROW_CMAKE_ARGS} - BUILD_BYPRODUCTS ${ARROW_BUILD_BYPRODUCTS}) + BUILD_BYPRODUCTS ${ARROW_SHARED_LIB}) set(ARROW_LIBRARY_TARGET arrow_shared) # If find_package has already found a valid Arrow installation, then # we don't want to link against the newly built arrow_shared library. - # However, we still need create a library target to trigger building + # However, we still need to create a library target to trigger building # of the arrow_ep target, which will ultimately build the bundled # GoogleTest binaries. if(Arrow_FOUND) @@ -76,8 +83,14 @@ function(build_arrow) file(MAKE_DIRECTORY "${ARROW_INCLUDE_DIR}") add_library(${ARROW_LIBRARY_TARGET} SHARED IMPORTED) set_target_properties(${ARROW_LIBRARY_TARGET} - PROPERTIES ${ARROW_IMPORTED_TYPE} ${ARROW_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES ${ARROW_INCLUDE_DIR}) + PROPERTIES + INTERFACE_INCLUDE_DIRECTORIES ${ARROW_INCLUDE_DIR} + IMPORTED_LOCATION ${ARROW_SHARED_LIB}) + if(WIN32) + set_target_properties(${ARROW_LIBRARY_TARGET} + PROPERTIES + IMPORTED_IMPLIB ${ARROW_LINK_LIB}) + endif() add_dependencies(${ARROW_LIBRARY_TARGET} arrow_ep) @@ -88,32 +101,37 @@ function(build_arrow) endfunction() macro(enable_gtest) - if(WIN32) - set(ARROW_GTEST_IMPORTED_TYPE IMPORTED_IMPLIB) - set(ARROW_GTEST_MAIN_IMPORTED_TYPE IMPORTED_IMPLIB) + set(ARROW_GTEST_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") + set(ARROW_GTEST_MAIN_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") - set(ARROW_GTEST_LIBRARY_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) - set(ARROW_GTEST_MAIN_LIBRARY_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) + if(WIN32) + set(ARROW_GTEST_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) + set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/bin") + set(ARROW_GTEST_MAIN_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) + set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/bin") else() - set(ARROW_GTEST_IMPORTED_TYPE IMPORTED_LOCATION) - set(ARROW_GTEST_MAIN_IMPORTED_TYPE IMPORTED_LOCATION) - - set(ARROW_GTEST_LIBRARY_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) - set(ARROW_GTEST_MAIN_LIBRARY_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) + set(ARROW_GTEST_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) + set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") + set(ARROW_GTEST_MAIN_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) + set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") endif() - set(ARROW_GTEST_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_PREFIX}/include") - set(ARROW_GTEST_LIBRARY_DIR "${ARROW_GTEST_PREFIX}/lib") + set(ARROW_GTEST_LINK_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") + set(ARROW_GTEST_LINK_LIB + "${ARROW_GTEST_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest${ARROW_GTEST_LINK_SUFFIX}" + ) set(ARROW_GTEST_SHARED_LIB - "${ARROW_GTEST_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${ARROW_GTEST_LIBRARY_SUFFIX}" + "${ARROW_GTEST_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" ) - set(ARROW_GTEST_MAIN_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") set(ARROW_GTEST_MAIN_INCLUDE_DIR "${ARROW_GTEST_MAIN_PREFIX}/include") - set(ARROW_GTEST_MAIN_LIBRARY_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") + set(ARROW_GTEST_MAIN_LINK_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") + set(ARROW_GTEST_MAIN_LINK_LIB + "${ARROW_GTEST_MAIN_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest_main${ARROW_GTEST_MAIN_LINK_SUFFIX}" + ) set(ARROW_GTEST_MAIN_SHARED_LIB - "${ARROW_GTEST_MAIN_LIBRARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${ARROW_GTEST_MAIN_LIBRARY_SUFFIX}" + "${ARROW_GTEST_MAIN_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" ) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") @@ -123,61 +141,32 @@ endmacro() # Build the GoogleTest binaries that are bundled with the Arrow C++ libraries. macro(build_gtest) - set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_PREFIX}/include") - set(ARROW_GTEST_MAIN_INCLUDE_DIR "${ARROW_GTEST_MAIN_PREFIX}/include") - file(MAKE_DIRECTORY "${ARROW_GTEST_INCLUDE_DIR}") + add_library(GTest::gtest SHARED IMPORTED) + set_target_properties(GTest::gtest + PROPERTIES + IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} + INTERFACE_INCLUDE_DIRECTORIES ${ARROW_GTEST_INCLUDE_DIR}) if(WIN32) - set(ARROW_GTEST_RUNTIME_DIR "${ARROW_GTEST_PREFIX}/bin") - set(ARROW_GTEST_MAIN_RUNTIME_DIR "${ARROW_GTEST_MAIN_PREFIX}/bin") - set(ARROW_GTEST_RUNTIME_SUFFIX "${CMAKE_SHARED_LIBRARY_SUFFIX}") - set(ARROW_GTEST_MAIN_RUNTIME_SUFFIX "${CMAKE_SHARED_LIBRARY_SUFFIX}") - set(ARROW_GTEST_RUNTIME_LIB - "${ARROW_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${ARROW_GTEST_RUNTIME_SUFFIX}" - ) - set(ARROW_GTEST_MAIN_RUNTIME_LIB - "${ARROW_GTEST_MAIN_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${ARROW_GTEST_MAIN_RUNTIME_SUFFIX}" - ) - - # Multi-Configuration generators (e.g. Visual Studio or XCode) place their build artifacts - # in a subdirectory named ${CMAKE_BUILD_TYPE} by default, where ${CMAKE_BUILD_TYPE} varies - # depending on the chosen build configuration (e.g. Release or Debug). - get_property(GENERATOR_IS_MULTI_CONFIG_VALUE GLOBAL - PROPERTY GENERATOR_IS_MULTI_CONFIG) - if(GENERATOR_IS_MULTI_CONFIG_VALUE) - set(MATLAB_TESTS_DIR "${CMAKE_BINARY_DIR}/$") - else() - set(MATLAB_TESTS_DIR "${CMAKE_BINARY_DIR}") - endif() - - # We need to copy the gtest and gtest_main runtime DLLs into the directory where the - # MATLAB C++ tests reside, since Windows requires that runtime DLLs are in the same - # directory as the executables that depend on them (or on the %PATH%). - externalproject_add_step(arrow_ep copy - COMMAND ${CMAKE_COMMAND} -E make_directory - ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy ${ARROW_GTEST_RUNTIME_LIB} - ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy - ${ARROW_GTEST_MAIN_RUNTIME_LIB} ${MATLAB_TESTS_DIR} - DEPENDEES install) + set_target_properties(GTest::gtest + PROPERTIES + IMPORTED_IMPLIB ${ARROW_GTEST_LINK_LIB}) endif() - add_library(GTest::gtest SHARED IMPORTED) - set_target_properties(GTest::gtest - PROPERTIES ${ARROW_GTEST_IMPORTED_TYPE} ${ARROW_GTEST_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES - ${ARROW_GTEST_INCLUDE_DIR}) + add_dependencies(GTest::gtest arrow_ep) add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main - PROPERTIES ${ARROW_GTEST_MAIN_IMPORTED_TYPE} - ${ARROW_GTEST_MAIN_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES - ${ARROW_GTEST_MAIN_INCLUDE_DIR}) + PROPERTIES + IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB} + INTERFACE_INCLUDE_DIRECTORIES ${ARROW_GTEST_MAIN_INCLUDE_DIR}) + if(WIN32) + set_target_properties(GTest::gtest_main + PROPERTIES + IMPORTED_IMPLIB ${ARROW_GTEST_MAIN_LINK_LIB}) + endif() - add_dependencies(GTest::gtest arrow_ep) add_dependencies(GTest::gtest_main arrow_ep) endmacro() @@ -198,10 +187,22 @@ endif() set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR}/cmake_modules) +# Multi-Configuration generators (e.g. Visual Studio or XCode) place their build artifacts +# in a subdirectory named ${CMAKE_BUILD_TYPE} by default, where ${CMAKE_BUILD_TYPE} varies +# depending on the chosen build configuration (e.g. Release or Debug). +get_property(GENERATOR_IS_MULTI_CONFIG_VALUE GLOBAL + PROPERTY GENERATOR_IS_MULTI_CONFIG) +if(GENERATOR_IS_MULTI_CONFIG_VALUE) + set(MATLAB_BUILD_OUTPUT_DIR "${CMAKE_BINARY_DIR}/$") +else() + set(MATLAB_BUILD_OUTPUT_DIR "${CMAKE_BINARY_DIR}") +endif() + # Only build the MATLAB interface C++ tests if MATLAB_BUILD_TESTS=ON. if(MATLAB_BUILD_TESTS) # find_package(GTest) supports custom GTEST_ROOT as well as package managers. find_package(GTest) + if(NOT GTest_FOUND) # find_package(Arrow) supports custom ARROW_HOME as well as package # managers. @@ -212,12 +213,60 @@ if(MATLAB_BUILD_TESTS) # C++ libraries that are built from source. build_arrow(BUILD_GTEST) else() + # GTest is found, on Windows, IMPORTED_LOCATION needs to be set to indicate where the shared + # libraries live + if(WIN32) + set(GTEST_SHARED_LIB_DIR "${GTEST_ROOT}/bin") + set(GTEST_SHARED_LIBRARY_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(GTEST_SHARED_LIBRARY_LIB + "${GTEST_SHARED_LIB_DIR}/${GTEST_SHARED_LIBRARY_FILENAME}" + ) + + set(GTEST_MAIN_SHARED_LIB_DIR "${GTEST_ROOT}/bin") + set(GTEST_MAIN_SHARED_LIBRARY_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(GTEST_MAIN_SHARED_LIBRARY_LIB + "${GTEST_MAIN_SHARED_LIB_DIR}/${GTEST_MAIN_SHARED_LIBRARY_FILENAME}" + ) + + set_target_properties(GTest::gtest PROPERTIES + IMPORTED_LOCATION "${GTEST_SHARED_LIBRARY_LIB}") + + set_target_properties(GTest::gtest_main PROPERTIES + IMPORTED_LOCATION "${GTEST_MAIN_SHARED_LIBRARY_LIB}") + endif() + find_package(Arrow) if(NOT Arrow_FOUND) # Trigger an automatic build of the Arrow C++ libraries. build_arrow() endif() endif() + + # On Windows: copy the gtest and gtest_main runtime DLLs into the directory where the + # MATLAB C++ tests reside, since Windows require runtime DLLs that are depended on by + # executables to be in the same directory (or on the %PATH%). + if(WIN32) + set(MATLAB_TESTS_DIR "${MATLAB_BUILD_OUTPUT_DIR}") + + get_property(GTEST_SHARED_LIB + TARGET GTest::gtest + PROPERTY IMPORTED_LOCATION) + + + get_property(GTEST_MAIN_SHARED_LIB + TARGET GTest::gtest_main + PROPERTY IMPORTED_LOCATION) + + add_custom_target(copy_gtest_to_tests_dir + ALL + COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_TESTS_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_SHARED_LIB} ${MATLAB_TESTS_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_MAIN_SHARED_LIB} ${MATLAB_TESTS_DIR} + COMMENT "Start copying gtest dlls from ${GTEST_SHARED_LIB} to ${MATLAB_TESTS_DIR} and ${GTEST_MAIN_SHARED_LIB} to ${MATLAB_TESTS_DIR}") + add_dependencies(copy_gtest_to_tests_dir GTest::gtest) + add_dependencies(copy_gtest_to_tests_dir GTest::gtest_main) + endif() + else() find_package(Arrow) if(NOT Arrow_FOUND) @@ -225,6 +274,22 @@ else() endif() endif() +# On Windows: copy arrow.dll into the directory where the MATLAB C++ tests reside, +# since Windows require runtime DLLs that are depended on by executables to be in +# the same directory (or on the %PATH%). +if(WIN32) + get_property(ARROW_SHARED_LIB + TARGET arrow_shared + PROPERTY IMPORTED_LOCATION) + + add_custom_target(copy_arrow_to_build_output_dir + ALL + COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_BUILD_OUTPUT_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${ARROW_SHARED_LIB} ${MATLAB_BUILD_OUTPUT_DIR} + COMMENT "Start copying arrow.dll from ${ARROW_SHARED_LIB} to ${MATLAB_BUILD_OUTPUT_DIR}") + add_dependencies(copy_arrow_to_build_output_dir arrow_shared) +endif() + # MATLAB is Required find_package(Matlab REQUIRED) @@ -305,4 +370,13 @@ if(MATLAB_BUILD_TESTS) add_test(PlaceholderTestTarget placeholder_test) add_test(CheckNumArgsTestTarget mex_util_test) + # For Windows: add the directory of libmx.dll and libmex.dll to the %PATH% for + # running CheckNumArgsTestTarget + # Note: When appending to the path using set_test_properties' ENVIRONNENT property, + # make sure that we escape ';' to avoid cmake from interpreting the input as + # a list of strings. + if(WIN32) + set(MATLAB_DLL_DEPENDENCIES_DIR "${Matlab_ROOT_DIR}/bin/win64") + set_tests_properties(CheckNumArgsTestTarget PROPERTIES ENVIRONMENT "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;$ENV{PATH}") + endif() endif() From af7db1d2cf561fdcdb4ac06d31cb0ceefa882372 Mon Sep 17 00:00:00 2001 From: Fiona La Date: Tue, 7 Dec 2021 11:26:47 -0500 Subject: [PATCH 24/33] On macOS, add the location that contains the output library of arrow_shared as a target_link_option for arrow_matlab, as the rpath is not automatically added when linking imported libraries. Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 8e8ef5ac47d..d9d3b0cda61 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -39,7 +39,7 @@ function(build_arrow) # The shared library is located in the "bin" directory. set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/bin") else() - # On Linux and Mac, the link time suffix and runtime suffixes are consistent, (.so and .dylib). + # On Linux and macOS, the link time suffix and runtime suffixes are consistent, (.so and .dylib). set(ARROW_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) # The shared library is located in the "lib" directory. set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/lib") @@ -309,10 +309,23 @@ set(arrow_matlab_sources list(TRANSFORM arrow_matlab_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) add_library(arrow_matlab SHARED ${arrow_matlab_sources}) -# Declare a dependency on the MEX shared library (libmex.so). + +# Declare a dependency on arrow_shared (libarrow.so/dylib/dll). +target_link_libraries(arrow_matlab arrow_shared) + +# On macOS, rpaths for linking imported targets are not automatically added to linked libraries +if(APPLE) + get_property(ARROW_SHARED_IMPORTED_LOCATION + TARGET arrow_shared + PROPERTY IMPORTED_LOCATION) + get_filename_component(ARROW_SHARED_LIBRARY_DIR ${ARROW_SHARED_IMPORTED_LOCATION} DIRECTORY) + target_link_options(arrow_matlab PUBLIC -Wl,-rpath,${ARROW_SHARED_LIBRARY_DIR}) +endif() + +# Declare a dependency on the MEX shared library (libmex.so/dylib/dll). target_link_libraries(arrow_matlab ${Matlab_MX_LIBRARY}) target_link_libraries(arrow_matlab ${Matlab_MEX_LIBRARY}) -target_link_libraries(arrow_matlab arrow_shared) + # Include the MATLAB MEX headers. target_include_directories(arrow_matlab PRIVATE ${Matlab_INCLUDE_DIRS}) target_include_directories(arrow_matlab PRIVATE ${CPP_SOURCE_DIR}) From fdd3484ef7aaee5cfa81fd159003cd6da9ea01ec Mon Sep 17 00:00:00 2001 From: Fiona La Date: Wed, 15 Dec 2021 14:59:42 -0500 Subject: [PATCH 25/33] On Windows, add a global flag to set the MSVC runtime library to the release version and add the link libraries to arrow_ep on Windows as BUILD_BYPRODUCTS. Co-authored-by: Kevin Gurney --- matlab/CMakeLists.txt | 161 ++++++++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 67 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index d9d3b0cda61..a88bd9ea2d9 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -32,11 +32,11 @@ function(build_arrow) endif() set(ARROW_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-prefix") - + if(WIN32) # On Windows, the link time suffix (.lib) and runtime library suffixes (.dll) are different. set(ARROW_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) - # The shared library is located in the "bin" directory. + # The shared library is located in the "bin" directory. set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/bin") else() # On Linux and macOS, the link time suffix and runtime suffixes are consistent, (.so and .dylib). @@ -48,7 +48,8 @@ function(build_arrow) set(ARROW_LINK_LIB_FILENAME "${CMAKE_IMPORT_LIBRARY_PREFIX}arrow${ARROW_LINK_SUFFIX}") set(ARROW_LINK_LIB "${ARROW_PREFIX}/lib/${ARROW_LINK_LIB_FILENAME}") - set(ARROW_SHARED_LIB_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}arrow${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(ARROW_SHARED_LIB_FILENAME + "${CMAKE_SHARED_LIBRARY_PREFIX}arrow${CMAKE_SHARED_LIBRARY_SUFFIX}") set(ARROW_SHARED_LIB "${ARROW_SHARED_LIBRARY_DIR}/${ARROW_SHARED_LIB_FILENAME}") set(ARROW_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-build") @@ -56,6 +57,16 @@ function(build_arrow) "-DCMAKE_INSTALL_LIBDIR=lib" "-DARROW_BUILD_STATIC=OFF") set(ARROW_INCLUDE_DIR "${ARROW_PREFIX}/include") + # On Windows, add the Arrow link library as a BUILD_BYPRODUCTS for arrow_ep. + # On Linux and macOS, add the Arrow shared library as a BUILD_BYPRODUCTS for arrow_ep. + # When building with Ninja, the link/shared libraries need to be guaranteed to be available for linking the test + # executables. + if(WIN32) + set(ARROW_BUILD_BYPRODUCTS "${ARROW_LINK_LIB}") + else() + set(ARROW_BUILD_BYPRODUCTS "${ARROW_SHARED_LIB}") + endif() + # Building the Arrow C++ libraries and bundled GoogleTest binaries requires ExternalProject. include(ExternalProject) @@ -66,8 +77,8 @@ function(build_arrow) externalproject_add(arrow_ep SOURCE_DIR "${CMAKE_SOURCE_DIR}/../cpp" BINARY_DIR "${ARROW_BINARY_DIR}" - CMAKE_ARGS ${ARROW_CMAKE_ARGS} - BUILD_BYPRODUCTS ${ARROW_SHARED_LIB}) + CMAKE_ARGS "${ARROW_CMAKE_ARGS}" + BUILD_BYPRODUCTS "${ARROW_BUILD_BYPRODUCTS}") set(ARROW_LIBRARY_TARGET arrow_shared) @@ -83,13 +94,11 @@ function(build_arrow) file(MAKE_DIRECTORY "${ARROW_INCLUDE_DIR}") add_library(${ARROW_LIBRARY_TARGET} SHARED IMPORTED) set_target_properties(${ARROW_LIBRARY_TARGET} - PROPERTIES - INTERFACE_INCLUDE_DIRECTORIES ${ARROW_INCLUDE_DIR} - IMPORTED_LOCATION ${ARROW_SHARED_LIB}) + PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${ARROW_INCLUDE_DIR} + IMPORTED_LOCATION ${ARROW_SHARED_LIB}) if(WIN32) - set_target_properties(${ARROW_LIBRARY_TARGET} - PROPERTIES - IMPORTED_IMPLIB ${ARROW_LINK_LIB}) + set_target_properties(${ARROW_LIBRARY_TARGET} PROPERTIES IMPORTED_IMPLIB + ${ARROW_LINK_LIB}) endif() add_dependencies(${ARROW_LIBRARY_TARGET} arrow_ep) @@ -97,7 +106,6 @@ function(build_arrow) if(ARG_BUILD_GTEST) build_gtest() endif() - endfunction() macro(enable_gtest) @@ -131,12 +139,22 @@ macro(enable_gtest) "${ARROW_GTEST_MAIN_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest_main${ARROW_GTEST_MAIN_LINK_SUFFIX}" ) set(ARROW_GTEST_MAIN_SHARED_LIB - "${ARROW_GTEST_MAIN_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" + "${ARROW_GTEST_MAIN_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" ) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") - list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_SHARED_LIB}" - "${ARROW_GTEST_MAIN_SHARED_LIB}") + + # On Windows, add the gtest link libraries as BUILD_BYPRODUCTS for arrow_ep. + # On Linux and macOS, add the gtest shared libraries as BUILD_BYPRODUCTS for arrow_ep. + # When building with Ninja, The link/shared libraries need to be guaranteed to be available for linking the test + # executables. + if(WIN32) + list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_LINK_LIB}" + "${ARROW_GTEST_MAIN_LINK_LIB}") + else() + list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_SHARED_LIB}" + "${ARROW_GTEST_MAIN_SHARED_LIB}") + endif() endmacro() # Build the GoogleTest binaries that are bundled with the Arrow C++ libraries. @@ -145,26 +163,23 @@ macro(build_gtest) add_library(GTest::gtest SHARED IMPORTED) set_target_properties(GTest::gtest - PROPERTIES - IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES ${ARROW_GTEST_INCLUDE_DIR}) + PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} + INTERFACE_INCLUDE_DIRECTORIES + ${ARROW_GTEST_INCLUDE_DIR}) if(WIN32) - set_target_properties(GTest::gtest - PROPERTIES - IMPORTED_IMPLIB ${ARROW_GTEST_LINK_LIB}) + set_target_properties(GTest::gtest PROPERTIES IMPORTED_IMPLIB ${ARROW_GTEST_LINK_LIB}) endif() add_dependencies(GTest::gtest arrow_ep) add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main - PROPERTIES - IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB} - INTERFACE_INCLUDE_DIRECTORIES ${ARROW_GTEST_MAIN_INCLUDE_DIR}) + PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB} + INTERFACE_INCLUDE_DIRECTORIES + ${ARROW_GTEST_MAIN_INCLUDE_DIR}) if(WIN32) - set_target_properties(GTest::gtest_main - PROPERTIES - IMPORTED_IMPLIB ${ARROW_GTEST_MAIN_LINK_LIB}) + set_target_properties(GTest::gtest_main PROPERTIES IMPORTED_IMPLIB + ${ARROW_GTEST_MAIN_LINK_LIB}) endif() add_dependencies(GTest::gtest_main arrow_ep) @@ -177,6 +192,12 @@ string(REGEX MATCH "^[0-9]+\\.[0-9]+\\.[0-9]+" MLARROW_BASE_VERSION "${MLARROW_V project(mlarrow VERSION "${MLARROW_BASE_VERSION}") +# On Windows, set the global variable that determines the MSVC runtime library that is +# used by targets created in this CMakeLists.txt file. +if(WIN32) + set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL") +endif() + option(MATLAB_BUILD_TESTS "Build the C++ tests for the MATLAB interface" OFF) # Grab CMAKE Modules from the CPP interface @@ -190,8 +211,7 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_SOURCE_DIR}/cmake_modules) # Multi-Configuration generators (e.g. Visual Studio or XCode) place their build artifacts # in a subdirectory named ${CMAKE_BUILD_TYPE} by default, where ${CMAKE_BUILD_TYPE} varies # depending on the chosen build configuration (e.g. Release or Debug). -get_property(GENERATOR_IS_MULTI_CONFIG_VALUE GLOBAL - PROPERTY GENERATOR_IS_MULTI_CONFIG) +get_property(GENERATOR_IS_MULTI_CONFIG_VALUE GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) if(GENERATOR_IS_MULTI_CONFIG_VALUE) set(MATLAB_BUILD_OUTPUT_DIR "${CMAKE_BINARY_DIR}/$") else() @@ -217,24 +237,25 @@ if(MATLAB_BUILD_TESTS) # libraries live if(WIN32) set(GTEST_SHARED_LIB_DIR "${GTEST_ROOT}/bin") - set(GTEST_SHARED_LIBRARY_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(GTEST_SHARED_LIBRARY_FILENAME + "${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}") set(GTEST_SHARED_LIBRARY_LIB - "${GTEST_SHARED_LIB_DIR}/${GTEST_SHARED_LIBRARY_FILENAME}" - ) + "${GTEST_SHARED_LIB_DIR}/${GTEST_SHARED_LIBRARY_FILENAME}") set(GTEST_MAIN_SHARED_LIB_DIR "${GTEST_ROOT}/bin") - set(GTEST_MAIN_SHARED_LIBRARY_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}") + set(GTEST_MAIN_SHARED_LIBRARY_FILENAME + "${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}") set(GTEST_MAIN_SHARED_LIBRARY_LIB - "${GTEST_MAIN_SHARED_LIB_DIR}/${GTEST_MAIN_SHARED_LIBRARY_FILENAME}" - ) + "${GTEST_MAIN_SHARED_LIB_DIR}/${GTEST_MAIN_SHARED_LIBRARY_FILENAME}") - set_target_properties(GTest::gtest PROPERTIES - IMPORTED_LOCATION "${GTEST_SHARED_LIBRARY_LIB}") + set_target_properties(GTest::gtest PROPERTIES IMPORTED_LOCATION + "${GTEST_SHARED_LIBRARY_LIB}") - set_target_properties(GTest::gtest_main PROPERTIES - IMPORTED_LOCATION "${GTEST_MAIN_SHARED_LIBRARY_LIB}") + set_target_properties(GTest::gtest_main + PROPERTIES IMPORTED_LOCATION + "${GTEST_MAIN_SHARED_LIBRARY_LIB}") endif() - + find_package(Arrow) if(NOT Arrow_FOUND) # Trigger an automatic build of the Arrow C++ libraries. @@ -249,20 +270,21 @@ if(MATLAB_BUILD_TESTS) set(MATLAB_TESTS_DIR "${MATLAB_BUILD_OUTPUT_DIR}") get_property(GTEST_SHARED_LIB - TARGET GTest::gtest + TARGET GTest::gtest PROPERTY IMPORTED_LOCATION) - get_property(GTEST_MAIN_SHARED_LIB - TARGET GTest::gtest_main + TARGET GTest::gtest_main PROPERTY IMPORTED_LOCATION) - add_custom_target(copy_gtest_to_tests_dir - ALL - COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_SHARED_LIB} ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_MAIN_SHARED_LIB} ${MATLAB_TESTS_DIR} - COMMENT "Start copying gtest dlls from ${GTEST_SHARED_LIB} to ${MATLAB_TESTS_DIR} and ${GTEST_MAIN_SHARED_LIB} to ${MATLAB_TESTS_DIR}") + add_custom_target(copy_gtest_to_tests_dir ALL + COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_TESTS_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_SHARED_LIB} + ${MATLAB_TESTS_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different + ${GTEST_MAIN_SHARED_LIB} ${MATLAB_TESTS_DIR} + COMMENT "Start copying gtest dlls from ${GTEST_SHARED_LIB} to ${MATLAB_TESTS_DIR} and ${GTEST_MAIN_SHARED_LIB} to ${MATLAB_TESTS_DIR}" + ) add_dependencies(copy_gtest_to_tests_dir GTest::gtest) add_dependencies(copy_gtest_to_tests_dir GTest::gtest_main) endif() @@ -279,14 +301,15 @@ endif() # the same directory (or on the %PATH%). if(WIN32) get_property(ARROW_SHARED_LIB - TARGET arrow_shared - PROPERTY IMPORTED_LOCATION) - - add_custom_target(copy_arrow_to_build_output_dir - ALL - COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_BUILD_OUTPUT_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${ARROW_SHARED_LIB} ${MATLAB_BUILD_OUTPUT_DIR} - COMMENT "Start copying arrow.dll from ${ARROW_SHARED_LIB} to ${MATLAB_BUILD_OUTPUT_DIR}") + TARGET arrow_shared + PROPERTY IMPORTED_LOCATION) + + add_custom_target(copy_arrow_to_build_output_dir ALL + COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_BUILD_OUTPUT_DIR} + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${ARROW_SHARED_LIB} + ${MATLAB_BUILD_OUTPUT_DIR} + COMMENT "Start copying arrow.dll from ${ARROW_SHARED_LIB} to ${MATLAB_BUILD_OUTPUT_DIR}" + ) add_dependencies(copy_arrow_to_build_output_dir arrow_shared) endif() @@ -315,10 +338,11 @@ target_link_libraries(arrow_matlab arrow_shared) # On macOS, rpaths for linking imported targets are not automatically added to linked libraries if(APPLE) - get_property(ARROW_SHARED_IMPORTED_LOCATION - TARGET arrow_shared - PROPERTY IMPORTED_LOCATION) - get_filename_component(ARROW_SHARED_LIBRARY_DIR ${ARROW_SHARED_IMPORTED_LOCATION} DIRECTORY) + get_property(ARROW_SHARED_IMPORTED_LOCATION + TARGET arrow_shared + PROPERTY IMPORTED_LOCATION) + get_filename_component(ARROW_SHARED_LIBRARY_DIR ${ARROW_SHARED_IMPORTED_LOCATION} + DIRECTORY) target_link_options(arrow_matlab PUBLIC -Wl,-rpath,${ARROW_SHARED_LIBRARY_DIR}) endif() @@ -332,8 +356,7 @@ target_include_directories(arrow_matlab PRIVATE ${CPP_SOURCE_DIR}) target_include_directories(arrow_matlab PRIVATE ${ARROW_INCLUDE_DIR}) target_compile_definitions(arrow_matlab PRIVATE ARROW_MATLAB_EXPORTING) -set(mexfcn_sources - mex/mexfcn.cc) +set(mexfcn_sources mex/mexfcn.cc) list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) # Store the CMake Configuration Type for multi-configuration generators so @@ -341,8 +364,10 @@ list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) # folder to the MATLAB Search Path get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) if(is_multi_config) - add_custom_command(TARGET arrow_matlab POST_BUILD - COMMAND "${CMAKE_COMMAND}" -E echo $ > ${CMAKE_BINARY_DIR}/is_multi_config.txt) + add_custom_command(TARGET arrow_matlab + POST_BUILD + COMMAND "${CMAKE_COMMAND}" -E echo $ > + ${CMAKE_BINARY_DIR}/is_multi_config.txt) endif() # Build mexfcn MEX binary @@ -386,10 +411,12 @@ if(MATLAB_BUILD_TESTS) # For Windows: add the directory of libmx.dll and libmex.dll to the %PATH% for # running CheckNumArgsTestTarget # Note: When appending to the path using set_test_properties' ENVIRONNENT property, - # make sure that we escape ';' to avoid cmake from interpreting the input as + # make sure that we escape ';' to avoid cmake from interpreting the input as # a list of strings. if(WIN32) set(MATLAB_DLL_DEPENDENCIES_DIR "${Matlab_ROOT_DIR}/bin/win64") - set_tests_properties(CheckNumArgsTestTarget PROPERTIES ENVIRONMENT "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;$ENV{PATH}") + set_tests_properties(CheckNumArgsTestTarget + PROPERTIES ENVIRONMENT + "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;$ENV{PATH}") endif() endif() From d845d59de2a339a8edc576430abf7a77840bcbee Mon Sep 17 00:00:00 2001 From: Fiona La Date: Tue, 21 Dec 2021 18:35:13 -0500 Subject: [PATCH 26/33] Remove code that creates a config file for multi-config generators, include in a separate pull request --- matlab/CMakeLists.txt | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index a88bd9ea2d9..4a587e982ed 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -359,17 +359,6 @@ target_compile_definitions(arrow_matlab PRIVATE ARROW_MATLAB_EXPORTING) set(mexfcn_sources mex/mexfcn.cc) list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) -# Store the CMake Configuration Type for multi-configuration generators so -# that we can reference it later to automatically add the correct build -# folder to the MATLAB Search Path -get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG) -if(is_multi_config) - add_custom_command(TARGET arrow_matlab - POST_BUILD - COMMAND "${CMAKE_COMMAND}" -E echo $ > - ${CMAKE_BINARY_DIR}/is_multi_config.txt) -endif() - # Build mexfcn MEX binary matlab_add_mex(R2018a NAME mexfcn From 0359a1f49002cfffa7033d41c69af2edc4d76334 Mon Sep 17 00:00:00 2001 From: lafiona Date: Wed, 22 Dec 2021 12:32:01 -0800 Subject: [PATCH 27/33] On Windows, add path to GTest dlls to environment path for tests instead of copying the dlls to the test directory --- matlab/CMakeLists.txt | 52 +++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 4a587e982ed..732dc8ebe52 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -263,32 +263,6 @@ if(MATLAB_BUILD_TESTS) endif() endif() - # On Windows: copy the gtest and gtest_main runtime DLLs into the directory where the - # MATLAB C++ tests reside, since Windows require runtime DLLs that are depended on by - # executables to be in the same directory (or on the %PATH%). - if(WIN32) - set(MATLAB_TESTS_DIR "${MATLAB_BUILD_OUTPUT_DIR}") - - get_property(GTEST_SHARED_LIB - TARGET GTest::gtest - PROPERTY IMPORTED_LOCATION) - - get_property(GTEST_MAIN_SHARED_LIB - TARGET GTest::gtest_main - PROPERTY IMPORTED_LOCATION) - - add_custom_target(copy_gtest_to_tests_dir ALL - COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${GTEST_SHARED_LIB} - ${MATLAB_TESTS_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different - ${GTEST_MAIN_SHARED_LIB} ${MATLAB_TESTS_DIR} - COMMENT "Start copying gtest dlls from ${GTEST_SHARED_LIB} to ${MATLAB_TESTS_DIR} and ${GTEST_MAIN_SHARED_LIB} to ${MATLAB_TESTS_DIR}" - ) - add_dependencies(copy_gtest_to_tests_dir GTest::gtest) - add_dependencies(copy_gtest_to_tests_dir GTest::gtest_main) - endif() - else() find_package(Arrow) if(NOT Arrow_FOUND) @@ -397,15 +371,35 @@ if(MATLAB_BUILD_TESTS) add_test(PlaceholderTestTarget placeholder_test) add_test(CheckNumArgsTestTarget mex_util_test) - # For Windows: add the directory of libmx.dll and libmex.dll to the %PATH% for - # running CheckNumArgsTestTarget + # For Windows: + # Add the directory of gtest.dll and gtest_main.dll to the %PATH% for running + # all tests. + # Add the directory of libmx.dll and libmex.dll to the %PATH% for running + # CheckNumArgsTestTarget. # Note: When appending to the path using set_test_properties' ENVIRONNENT property, # make sure that we escape ';' to avoid cmake from interpreting the input as # a list of strings. if(WIN32) + get_property(GTEST_SHARED_LIB + TARGET GTest::gtest + PROPERTY IMPORTED_LOCATION) + get_filename_component(GTEST_SHARED_LIB_DIR ${GTEST_SHARED_LIB} DIRECTORY) + + get_property(GTEST_MAIN_SHARED_LIB + TARGET GTest::gtest_main + PROPERTY IMPORTED_LOCATION) + get_filename_component(GTEST_MAIN_SHARED_LIB_DIR ${GTEST_MAIN_SHARED_LIB} DIRECTORY) + + set_tests_properties(PlaceholderTestTarget + PROPERTIES ENVIRONMENT + "PATH=${GTEST_SHARED_LIB_DIR}\;${GTEST_MAIN_SHARED_LIB_DIR}\;$ENV{PATH}" + ) + set(MATLAB_DLL_DEPENDENCIES_DIR "${Matlab_ROOT_DIR}/bin/win64") + set_tests_properties(CheckNumArgsTestTarget PROPERTIES ENVIRONMENT - "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;$ENV{PATH}") + "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;${GTEST_SHARED_LIB_DIR}\;${GTEST_MAIN_SHARED_LIB_DIR}\;$ENV{PATH}" + ) endif() endif() From e8b7bcd7e64b303ddc3250d68e13f07f2ae2aa8a Mon Sep 17 00:00:00 2001 From: lafiona Date: Thu, 6 Jan 2022 13:20:04 -0800 Subject: [PATCH 28/33] Add support for installing to a user specified directory and adding the directory to the MATLAB Search Path. Co-authored-by: Kevin Gurney --- ci/scripts/matlab_build.sh | 5 +- matlab/CMakeLists.txt | 206 ++++++++++++++++------ matlab/test/tfeather.m | 4 - matlab/test/tfeathermex.m | 4 - matlab/test/tmexfcn.m | 4 - matlab/tools/UpdateMatlabSearchPath.cmake | 42 +++++ matlab/tools/addInstallDirToSearchPath.m | 31 ++++ 7 files changed, 232 insertions(+), 64 deletions(-) create mode 100644 matlab/tools/UpdateMatlabSearchPath.cmake create mode 100644 matlab/tools/addInstallDirToSearchPath.m diff --git a/ci/scripts/matlab_build.sh b/ci/scripts/matlab_build.sh index 5e9bdd2a91a..1e8b7447c96 100755 --- a/ci/scripts/matlab_build.sh +++ b/ci/scripts/matlab_build.sh @@ -23,7 +23,8 @@ set -ex base_dir=${1} source_dir=${base_dir}/matlab build_dir=${base_dir}/matlab/build +install_dir=${base_dir}/matlab/install -cmake -S ${source_dir} -B ${build_dir} -G Ninja -D MATLAB_BUILD_TESTS=ON -cmake --build ${build_dir} --config Release +cmake -S ${source_dir} -B ${build_dir} -G Ninja -D MATLAB_BUILD_TESTS=ON -D CMAKE_INSTALL_PREFIX=${install_dir} +cmake --build ${build_dir} --config Release --target install ctest --test-dir ${build_dir} diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 732dc8ebe52..863583fca0a 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -22,6 +22,7 @@ function(build_arrow) set(options BUILD_GTEST) set(one_value_args) set(multi_value_args) + cmake_parse_arguments(ARG "${options}" "${one_value_args}" @@ -31,6 +32,7 @@ function(build_arrow) message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}") endif() + # If Arrow needs to be built, the default location will be within the build tree. set(ARROW_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-prefix") if(WIN32) @@ -57,13 +59,13 @@ function(build_arrow) "-DCMAKE_INSTALL_LIBDIR=lib" "-DARROW_BUILD_STATIC=OFF") set(ARROW_INCLUDE_DIR "${ARROW_PREFIX}/include") - # On Windows, add the Arrow link library as a BUILD_BYPRODUCTS for arrow_ep. - # On Linux and macOS, add the Arrow shared library as a BUILD_BYPRODUCTS for arrow_ep. - # When building with Ninja, the link/shared libraries need to be guaranteed to be available for linking the test + # The output libraries need to be guaranteed to be available for linking the test # executables. if(WIN32) + # On Windows, add the Arrow link library as a BUILD_BYPRODUCTS for arrow_ep. set(ARROW_BUILD_BYPRODUCTS "${ARROW_LINK_LIB}") else() + # On Linux and macOS, add the Arrow shared library as a BUILD_BYPRODUCTS for arrow_ep. set(ARROW_BUILD_BYPRODUCTS "${ARROW_SHARED_LIB}") endif() @@ -144,14 +146,14 @@ macro(enable_gtest) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") - # On Windows, add the gtest link libraries as BUILD_BYPRODUCTS for arrow_ep. - # On Linux and macOS, add the gtest shared libraries as BUILD_BYPRODUCTS for arrow_ep. - # When building with Ninja, The link/shared libraries need to be guaranteed to be available for linking the test + # The appropriate libraries need to be guaranteed to be available for linking the test # executables. if(WIN32) + # On Windows, add the gtest link libraries as BUILD_BYPRODUCTS for arrow_ep. list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_LINK_LIB}" "${ARROW_GTEST_MAIN_LINK_LIB}") else() + # On Linux and macOS, add the gtest shared libraries as BUILD_BYPRODUCTS for arrow_ep. list(APPEND ARROW_BUILD_BYPRODUCTS "${ARROW_GTEST_SHARED_LIB}" "${ARROW_GTEST_MAIN_SHARED_LIB}") endif() @@ -161,6 +163,7 @@ endmacro() macro(build_gtest) file(MAKE_DIRECTORY "${ARROW_GTEST_INCLUDE_DIR}") + # Create target GTest::gtest add_library(GTest::gtest SHARED IMPORTED) set_target_properties(GTest::gtest PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_SHARED_LIB} @@ -172,6 +175,7 @@ macro(build_gtest) add_dependencies(GTest::gtest arrow_ep) + # Create target GTest::gtest_main add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main PROPERTIES IMPORTED_LOCATION ${ARROW_GTEST_MAIN_SHARED_LIB} @@ -200,7 +204,7 @@ endif() option(MATLAB_BUILD_TESTS "Build the C++ tests for the MATLAB interface" OFF) -# Grab CMAKE Modules from the CPP interface +# Grab CMAKE Modules from the CPP interface. set(CPP_CMAKE_MODULES "${CMAKE_SOURCE_DIR}/../cpp/cmake_modules") if(EXISTS "${CPP_CMAKE_MODULES}") set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CPP_CMAKE_MODULES}) @@ -233,8 +237,8 @@ if(MATLAB_BUILD_TESTS) # C++ libraries that are built from source. build_arrow(BUILD_GTEST) else() - # GTest is found, on Windows, IMPORTED_LOCATION needs to be set to indicate where the shared - # libraries live + # On Windows, IMPORTED_LOCATION needs to be set to indicate where the shared + # libraries live when GTest is found. if(WIN32) set(GTEST_SHARED_LIB_DIR "${GTEST_ROOT}/bin") set(GTEST_SHARED_LIBRARY_FILENAME @@ -270,25 +274,8 @@ else() endif() endif() -# On Windows: copy arrow.dll into the directory where the MATLAB C++ tests reside, -# since Windows require runtime DLLs that are depended on by executables to be in -# the same directory (or on the %PATH%). -if(WIN32) - get_property(ARROW_SHARED_LIB - TARGET arrow_shared - PROPERTY IMPORTED_LOCATION) - - add_custom_target(copy_arrow_to_build_output_dir ALL - COMMAND ${CMAKE_COMMAND} -E make_directory ${MATLAB_BUILD_OUTPUT_DIR} - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${ARROW_SHARED_LIB} - ${MATLAB_BUILD_OUTPUT_DIR} - COMMENT "Start copying arrow.dll from ${ARROW_SHARED_LIB} to ${MATLAB_BUILD_OUTPUT_DIR}" - ) - add_dependencies(copy_arrow_to_build_output_dir arrow_shared) -endif() - # MATLAB is Required -find_package(Matlab REQUIRED) +find_package(Matlab REQUIRED COMPONENTS MAIN_PROGRAM) message(STATUS "Mex Library: ${Matlab_MEX_LIBRARY}") message(STATUS "Mex Include Folder: ${Matlab_INCLUDE_DIRS}") @@ -310,16 +297,6 @@ add_library(arrow_matlab SHARED ${arrow_matlab_sources}) # Declare a dependency on arrow_shared (libarrow.so/dylib/dll). target_link_libraries(arrow_matlab arrow_shared) -# On macOS, rpaths for linking imported targets are not automatically added to linked libraries -if(APPLE) - get_property(ARROW_SHARED_IMPORTED_LOCATION - TARGET arrow_shared - PROPERTY IMPORTED_LOCATION) - get_filename_component(ARROW_SHARED_LIBRARY_DIR ${ARROW_SHARED_IMPORTED_LOCATION} - DIRECTORY) - target_link_options(arrow_matlab PUBLIC -Wl,-rpath,${ARROW_SHARED_LIBRARY_DIR}) -endif() - # Declare a dependency on the MEX shared library (libmex.so/dylib/dll). target_link_libraries(arrow_matlab ${Matlab_MX_LIBRARY}) target_link_libraries(arrow_matlab ${Matlab_MEX_LIBRARY}) @@ -333,7 +310,7 @@ target_compile_definitions(arrow_matlab PRIVATE ARROW_MATLAB_EXPORTING) set(mexfcn_sources mex/mexfcn.cc) list(TRANSFORM mexfcn_sources PREPEND ${CPP_SOURCE_DIR}/arrow/matlab/) -# Build mexfcn MEX binary +# Build mexfcn MEX binary. matlab_add_mex(R2018a NAME mexfcn SRC ${mexfcn_sources} @@ -367,27 +344,34 @@ if(MATLAB_BUILD_TESTS) # Include the C++ source headers. target_include_directories(mex_util_test PRIVATE ${CPP_SOURCE_DIR}) - # Add a test target. + # Add test targets for C++ tests. add_test(PlaceholderTestTarget placeholder_test) add_test(CheckNumArgsTestTarget mex_util_test) - # For Windows: + # On macOS, add the directory of libarrow.dylib to the $DYLD_LIBRARY_PATH for + # running CheckNumArgsTestTarget. + if(APPLE) + get_target_property(ARROW_SHARED_LIB arrow_shared IMPORTED_LOCATION) + get_filename_component(ARROW_SHARED_LIB_DIR ${ARROW_SHARED_LIB} DIRECTORY) + + set_tests_properties(CheckNumArgsTestTarget + PROPERTIES ENVIRONMENT + "DYLD_LIBRARY_PATH=${ARROW_SHARED_LIB_DIR}") + endif() + + # On Windows: # Add the directory of gtest.dll and gtest_main.dll to the %PATH% for running # all tests. - # Add the directory of libmx.dll and libmex.dll to the %PATH% for running + # Add the directory of libmx.dll, libmex.dll, and libarrow.dll to the %PATH% for running # CheckNumArgsTestTarget. - # Note: When appending to the path using set_test_properties' ENVIRONNENT property, - # make sure that we escape ';' to avoid cmake from interpreting the input as + # Note: When appending to the path using set_test_properties' ENVIRONMENT property, + # make sure that we escape ';' to prevent CMake from interpreting the input as # a list of strings. if(WIN32) - get_property(GTEST_SHARED_LIB - TARGET GTest::gtest - PROPERTY IMPORTED_LOCATION) + get_target_property(GTEST_SHARED_LIB GTest::gtest IMPORTED_LOCATION) get_filename_component(GTEST_SHARED_LIB_DIR ${GTEST_SHARED_LIB} DIRECTORY) - get_property(GTEST_MAIN_SHARED_LIB - TARGET GTest::gtest_main - PROPERTY IMPORTED_LOCATION) + get_target_property(GTEST_MAIN_SHARED_LIB GTest::gtest_main IMPORTED_LOCATION) get_filename_component(GTEST_MAIN_SHARED_LIB_DIR ${GTEST_MAIN_SHARED_LIB} DIRECTORY) set_tests_properties(PlaceholderTestTarget @@ -395,11 +379,133 @@ if(MATLAB_BUILD_TESTS) "PATH=${GTEST_SHARED_LIB_DIR}\;${GTEST_MAIN_SHARED_LIB_DIR}\;$ENV{PATH}" ) + get_target_property(ARROW_SHARED_LIB arrow_shared IMPORTED_LOCATION) + get_filename_component(ARROW_SHARED_LIB_DIR ${ARROW_SHARED_LIB} DIRECTORY) + set(MATLAB_DLL_DEPENDENCIES_DIR "${Matlab_ROOT_DIR}/bin/win64") set_tests_properties(CheckNumArgsTestTarget PROPERTIES ENVIRONMENT - "PATH=${MATLAB_DLL_DEPENDENCIES_DIR}\;${GTEST_SHARED_LIB_DIR}\;${GTEST_MAIN_SHARED_LIB_DIR}\;$ENV{PATH}" + "PATH=${ARROW_SHARED_LIB_DIR}\;${MATLAB_DLL_DEPENDENCIES_DIR}\;${GTEST_SHARED_LIB_DIR}\;${GTEST_MAIN_SHARED_LIB_DIR}\;$ENV{PATH}" ) endif() endif() + +# ############################################################################## +# Install +# ############################################################################## +# Create a subdirectory at CMAKE_INSTALL_PREFIX to install the interface. +set(CMAKE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/arrow_matlab") + +# Install MATLAB source files. +if(APPLE) + # On macOS, exclude '.DS_Store' files in the source tree from installation. + install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" + DESTINATION ${CMAKE_INSTALL_DIR} + PATTERN ".DS_Store" EXCLUDE) +else() + install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" DESTINATION ${CMAKE_INSTALL_DIR}) +endif() + +# Install arrow_matlab and mexfcn. +if(WIN32) + install(TARGETS arrow_matlab mexfcn RUNTIME DESTINATION ${CMAKE_INSTALL_DIR}) +else() + # On Linux install arrow_matlab.so and on macOS install arrow_matlab.dylib + # using the LIBRARY keyword. + install(TARGETS arrow_matlab mexfcn LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) +endif() + +get_target_property(ARROW_SHARED_LIB arrow_shared IMPORTED_LOCATION) +get_filename_component(ARROW_SHARED_LIB_DIR ${ARROW_SHARED_LIB} DIRECTORY) +get_filename_component(ARROW_SHARED_LIB_FILENAME ${ARROW_SHARED_LIB} NAME_WE) + +if(WIN32) + # On Windows, arrow.dll must be installed to to CMAKE_INSTALL_DIR regardless of whether + # Arrow_FOUND is true or false. + install(FILES ${ARROW_SHARED_LIB} DESTINATION "${CMAKE_INSTALL_DIR}") +endif() + +# On macOS, use the RPATH values below for runtime dependency resolution. This enables +# relocation of the installation directory. +if(APPLE) + # Setting INSTALL_RPATH_USE_LINK_PATH to true will add the paths to external dependencies + # to the RPATH of arrow_matlab and mexfcn, including the MATLAB dependencies. + # If Arrow_FOUND is true, this also includes the path to arrow_shared. + set_target_properties(arrow_matlab mexfcn PROPERTIES INSTALL_RPATH_USE_LINK_PATH TRUE) + + # Add @loader_path to the RPATH of mexfcn so that libarrow_matlab.dylib can be found + # at runtime. + set_target_properties(mexfcn PROPERTIES INSTALL_RPATH "@loader_path") + + if(NOT Arrow_FOUND) + # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs + # to be copied to CMAKE_INSTALL_DIR. The DIRECTORY install command is used to + # install libarrow.dylib (symlink) and the real files it points to. + # + # The subfolders cmake and pkgconfig are excluded as they will be empty. + # Note: The following CMake Issue suggests enabling an option to exclude all + # folders that would be empty after installation: + # https://gitlab.kitware.com/cmake/cmake/-/issues/17122 + install(DIRECTORY "${ARROW_SHARED_LIB_DIR}/" + DESTINATION ${CMAKE_INSTALL_DIR} + FILES_MATCHING + REGEX "${ARROW_SHARED_LIB_FILENAME}\..*dylib" + PATTERN "cmake" EXCLUDE + PATTERN "pkgconfig" EXCLUDE) + + # Add @loader_path to the RPATH of arrow_matlab so that libarrow.dylib can be found + # at runtime. + set_target_properties(arrow_matlab PROPERTIES INSTALL_RPATH "@loader_path") + endif() +endif() + +# On Linux, use the RUNPATH values below for runtime dependency resolution. This enables +# relocation of the installation directory. +if(UNIX + AND NOT APPLE + AND NOT CYGWIN) + # Setting INSTALL_RPATH_USE_LINK_PATH to true will add the paths to external dependencies + # to the RUNPATH of arrow_matlab and mexfcn, including the MATLAB dependencies. + # If Arrow_FOUND is true, this also includes the path to arrow_shared. + set_target_properties(arrow_matlab mexfcn PROPERTIES INSTALL_RPATH_USE_LINK_PATH TRUE) + + # Add $ORIGIN to the RUNPATH of mexfcn so that libarrow_matlab.so can be found + # at runtime. + set_target_properties(mexfcn PROPERTIES INSTALL_RPATH $ORIGIN) + + if(NOT Arrow_FOUND) + # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs + # to be copied to CMAKE_INSTALL_DIR. The DIRECTORY install command is used to + # install libarrow.so (symlink) and the real files it points to. + # + # The subfolders cmake and pkgconfig are excluded as they will be empty. + # Note: The following CMake Issue suggests enabling an option to exclude all + # folders that would be empty after installation: + # https://gitlab.kitware.com/cmake/cmake/-/issues/17122 + install(DIRECTORY "${ARROW_SHARED_LIB_DIR}/" + DESTINATION ${CMAKE_INSTALL_DIR} + FILES_MATCHING + REGEX "${ARROW_SHARED_LIB_FILENAME}\.so.*" + PATTERN "cmake" EXCLUDE + PATTERN "pkgconfig" EXCLUDE) + + # Add $ORIGIN to the RUNPATH of arrow_matlab so that libarrow.so can be found + # at runtime. + set_target_properties(arrow_matlab PROPERTIES INSTALL_RPATH $ORIGIN) + endif() +endif() + +# As part of the installation process, add the installation directory to +# the MATLAB Search Path. +set(TOOLS_DIR "${CMAKE_SOURCE_DIR}/tools") + +# Pass Matlab_MAIN_PROGRAM, TOOLS_DIR, and INSTALL_DIR to the install step +# code/script execution scope. +install(CODE "set(Matlab_MAIN_PROGRAM \"${Matlab_MAIN_PROGRAM}\")") +install(CODE "set(TOOLS_DIR \"${TOOLS_DIR}\")") +install(CODE "set(INSTALL_DIR \"${CMAKE_INSTALL_DIR}\")") + +# Call the CMake script that runs the MATLAB function to add the install directory +# to the MATLAB Search Path. +install(SCRIPT "${TOOLS_DIR}/UpdateMatlabSearchPath.cmake") diff --git a/matlab/test/tfeather.m b/matlab/test/tfeather.m index e21f3f3b4fa..2f3ade59c79 100755 --- a/matlab/test/tfeather.m +++ b/matlab/test/tfeather.m @@ -22,10 +22,6 @@ function addFeatherFunctionsToMATLABPath(testCase) import matlab.unittest.fixtures.PathFixture % Add Feather test utilities to the MATLAB path. testCase.applyFixture(PathFixture('util')); - % Add featherread and featherwrite to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); - % Add mexfcn to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'build'))); % mexfcn must be on the MATLAB path. testCase.assertTrue(~isempty(which('mexfcn')), ... '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); diff --git a/matlab/test/tfeathermex.m b/matlab/test/tfeathermex.m index 62139a8d472..f502b05c949 100644 --- a/matlab/test/tfeathermex.m +++ b/matlab/test/tfeathermex.m @@ -22,10 +22,6 @@ function addFeatherFunctionsToMATLABPath(testCase) import matlab.unittest.fixtures.PathFixture % Add Feather test utilities to the MATLAB path. testCase.applyFixture(PathFixture('util')); - % Add featherread and featherwrite to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); - % Add mexfcn to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'build'))); % mexfcn must be on the MATLAB path. testCase.assertTrue(~isempty(which('mexfcn')), ... '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); diff --git a/matlab/test/tmexfcn.m b/matlab/test/tmexfcn.m index ef0ca686969..ebaec650e1d 100644 --- a/matlab/test/tmexfcn.m +++ b/matlab/test/tmexfcn.m @@ -21,10 +21,6 @@ function addFeatherFunctionsToMATLABPath(testCase) import matlab.unittest.fixtures.PathFixture % Add Feather test utilities to the MATLAB path. testCase.applyFixture(PathFixture('util')); - % Add featherread and featherwrite to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'src', 'matlab'))); - % Add mexfcn to the MATLAB path. - testCase.applyFixture(PathFixture(fullfile('..', 'build'))); % mexfcn must be on the MATLAB path. testCase.assertTrue(~isempty(which('mexfcn')), ... '''mexfcn'' must be on the MATLAB path. Use ''addpath'' to add folders to the MATLAB path.'); diff --git a/matlab/tools/UpdateMatlabSearchPath.cmake b/matlab/tools/UpdateMatlabSearchPath.cmake new file mode 100644 index 00000000000..a065ed5bf99 --- /dev/null +++ b/matlab/tools/UpdateMatlabSearchPath.cmake @@ -0,0 +1,42 @@ +# 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. + +# Add ${CMAKE_INSTALL_PREFIX}/arrow_matlab to the MATLAB Search Path so that MATLAB can find +# the installed libraries. + +# Convert Matlab_MAIN_PROGRAM, INSTALL_DIR and TOOLS_DIR to use OS native path notation. +file(TO_NATIVE_PATH ${Matlab_MAIN_PROGRAM} NATIVE_MATLAB_MAIN_PROGRAM) +file(TO_NATIVE_PATH ${INSTALL_DIR} NATIVE_INSTALL_DIR) +file(TO_NATIVE_PATH ${TOOLS_DIR} NATIVE_TOOLS_DIR) + +# Initialize an instance of MATLAB and call the MATLAB function, addInstallDirToSearchPath, +# defined in ${NATIVE_TOOLS_DIR}. +# Flags to pass to MATLAB: +# -sd: startup directory for the MATLAB +# -batch: non-interactive script execution +execute_process(COMMAND "${NATIVE_MATLAB_MAIN_PROGRAM}" -sd "${NATIVE_TOOLS_DIR}" -batch + "addInstallDirToSearchPath('${INSTALL_DIR}')" + RESULT_VARIABLE MATLAB_EXIT_CODE) + +if(MATLAB_EXIT_CODE EQUAL "1") + # Get path to MATLAB pathdef.m file. This is the location of the default MATLAB Search Path + set(MATLAB_PATHDEF_FILE "${Matlab_MAIN_PROGRAM}/toolbox/local/pathdef.m") + file(TO_NATIVE_PATH ${MATLAB_PATHDEF_FILE} NATIVE_MATLAB_PATHDEF_FILE) + + message(FATAL_ERROR "Failed to add the installation directory, ${NATIVE_INSTALL_DIR}, to the MATLAB Search Path. This may be due to the current user lacking the necessary filesystem permissions to modify ${NATIVE_MATLAB_PATHDEF_FILE}. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + ) +endif() diff --git a/matlab/tools/addInstallDirToSearchPath.m b/matlab/tools/addInstallDirToSearchPath.m new file mode 100644 index 00000000000..6c80bd590bb --- /dev/null +++ b/matlab/tools/addInstallDirToSearchPath.m @@ -0,0 +1,31 @@ +function addInstallDirToSearchPath(installDirPath) + % addInstallDirToSearchPath Add the input path, installDirPath, to the + % MATLAB Search Path and save. + % + % 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. + + addpath(installDirPath); + status = savepath(fullfile(matlabroot, "toolbox", "local", "pathdef.m")); + + % Return exit code 1 to indicate failure and 0 to indicate the path has + % been saved successfully. + if status == 0 + disp("Sucessfully added directory to the MATLAB Search Path: " + installDirPath); + quit(0); + else + quit(1); + end +end From c37cd71be817303fa875439136f55d5b77bd13bd Mon Sep 17 00:00:00 2001 From: Fiona La Date: Tue, 18 Jan 2022 23:43:33 -0500 Subject: [PATCH 29/33] Add two flags that control if/how the installation directory is added to the MATLAB Search Path. In Github Actions, the installation directory will be made available to the MATLAB tests by setting the MATLABPATH environment variable. Co-authored-by: Kevin Gurney --- .github/workflows/matlab.yml | 4 ++ ci/scripts/matlab_build.sh | 2 +- matlab/CMakeLists.txt | 50 +++++++++++++++++------ matlab/test/tfeather.m | 1 - matlab/tools/UpdateMatlabSearchPath.cmake | 20 +++++++-- matlab/tools/addInstallDirToSearchPath.m | 41 +++++++++++++++---- 6 files changed, 90 insertions(+), 28 deletions(-) diff --git a/.github/workflows/matlab.yml b/.github/workflows/matlab.yml index b403fbc3366..1bf273df1ce 100644 --- a/.github/workflows/matlab.yml +++ b/.github/workflows/matlab.yml @@ -60,6 +60,10 @@ jobs: # errors will occur. To work around this issue, we can explicitly # force MATLAB to use the system libstdc++.so via LD_PRELOAD. LD_PRELOAD: /usr/lib/x86_64-linux-gnu/libstdc++.so.6 + + # Add the installation directory to the MATLAB Search Path by + # setting the MATLABPATH environment variable. + MATLABPATH: matlab/install/arrow_matlab uses: matlab-actions/run-tests@v1 with: select-by-folder: matlab/test diff --git a/ci/scripts/matlab_build.sh b/ci/scripts/matlab_build.sh index 1e8b7447c96..656805275ed 100755 --- a/ci/scripts/matlab_build.sh +++ b/ci/scripts/matlab_build.sh @@ -25,6 +25,6 @@ source_dir=${base_dir}/matlab build_dir=${base_dir}/matlab/build install_dir=${base_dir}/matlab/install -cmake -S ${source_dir} -B ${build_dir} -G Ninja -D MATLAB_BUILD_TESTS=ON -D CMAKE_INSTALL_PREFIX=${install_dir} +cmake -S ${source_dir} -B ${build_dir} -G Ninja -D MATLAB_BUILD_TESTS=ON -D CMAKE_INSTALL_PREFIX=${install_dir} -D MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH=OFF cmake --build ${build_dir} --config Release --target install ctest --test-dir ${build_dir} diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index 863583fca0a..a0d44f230b1 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -496,16 +496,40 @@ if(UNIX endif() endif() -# As part of the installation process, add the installation directory to -# the MATLAB Search Path. -set(TOOLS_DIR "${CMAKE_SOURCE_DIR}/tools") - -# Pass Matlab_MAIN_PROGRAM, TOOLS_DIR, and INSTALL_DIR to the install step -# code/script execution scope. -install(CODE "set(Matlab_MAIN_PROGRAM \"${Matlab_MAIN_PROGRAM}\")") -install(CODE "set(TOOLS_DIR \"${TOOLS_DIR}\")") -install(CODE "set(INSTALL_DIR \"${CMAKE_INSTALL_DIR}\")") - -# Call the CMake script that runs the MATLAB function to add the install directory -# to the MATLAB Search Path. -install(SCRIPT "${TOOLS_DIR}/UpdateMatlabSearchPath.cmake") +# MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE sets whether the path to the install directory should +# be added to the startup.m file located at the MATLAB userpath. +option(MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE + "Sets whether the path to the install directory should be added to the startup.m file located at the MATLAB userpath" + OFF) + +# If MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE is specified ON and MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH is +# not specified, then set MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH=OFF. +if(MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE AND NOT DEFINED + MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH) + set(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH OFF) +endif() + +# MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH sets whether the path to the install directory should +# be directly added to the MATLAB Search Path. +option(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH + "Sets whether the path to the install directory should be directly added to the MATLAB Search Path" + ON) + +if(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH OR MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE) + set(TOOLS_DIR "${CMAKE_SOURCE_DIR}/tools") + + # Pass Matlab_MAIN_PROGRAM, TOOLS_DIR, and INSTALL_DIR to the install step + # code/script execution scope. + install(CODE "set(Matlab_MAIN_PROGRAM \"${Matlab_MAIN_PROGRAM}\")") + install(CODE "set(TOOLS_DIR \"${TOOLS_DIR}\")") + install(CODE "set(INSTALL_DIR \"${CMAKE_INSTALL_DIR}\")") + install(CODE "set(MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE \"${MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE}\")" + ) + install(CODE "set(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH \"${MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH}\")" + ) + + # Call the CMake script that runs the MATLAB function to add the install directory + # to the MATLAB Search Path or add a command to the MATLAB startup file to add the + # install directory to the MATLAB Search Path. + install(SCRIPT "${TOOLS_DIR}/UpdateMatlabSearchPath.cmake") +endif() diff --git a/matlab/test/tfeather.m b/matlab/test/tfeather.m index 2f3ade59c79..7d871762583 100755 --- a/matlab/test/tfeather.m +++ b/matlab/test/tfeather.m @@ -17,7 +17,6 @@ % permissions and limitations under the License. methods(TestClassSetup) - function addFeatherFunctionsToMATLABPath(testCase) import matlab.unittest.fixtures.PathFixture % Add Feather test utilities to the MATLAB path. diff --git a/matlab/tools/UpdateMatlabSearchPath.cmake b/matlab/tools/UpdateMatlabSearchPath.cmake index a065ed5bf99..50265e0a05c 100644 --- a/matlab/tools/UpdateMatlabSearchPath.cmake +++ b/matlab/tools/UpdateMatlabSearchPath.cmake @@ -15,9 +15,6 @@ # specific language governing permissions and limitations # under the License. -# Add ${CMAKE_INSTALL_PREFIX}/arrow_matlab to the MATLAB Search Path so that MATLAB can find -# the installed libraries. - # Convert Matlab_MAIN_PROGRAM, INSTALL_DIR and TOOLS_DIR to use OS native path notation. file(TO_NATIVE_PATH ${Matlab_MAIN_PROGRAM} NATIVE_MATLAB_MAIN_PROGRAM) file(TO_NATIVE_PATH ${INSTALL_DIR} NATIVE_INSTALL_DIR) @@ -29,7 +26,7 @@ file(TO_NATIVE_PATH ${TOOLS_DIR} NATIVE_TOOLS_DIR) # -sd: startup directory for the MATLAB # -batch: non-interactive script execution execute_process(COMMAND "${NATIVE_MATLAB_MAIN_PROGRAM}" -sd "${NATIVE_TOOLS_DIR}" -batch - "addInstallDirToSearchPath('${INSTALL_DIR}')" + "addInstallDirToSearchPath('${INSTALL_DIR}', '${MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH}', '${MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE}')" RESULT_VARIABLE MATLAB_EXIT_CODE) if(MATLAB_EXIT_CODE EQUAL "1") @@ -40,3 +37,18 @@ if(MATLAB_EXIT_CODE EQUAL "1") message(FATAL_ERROR "Failed to add the installation directory, ${NATIVE_INSTALL_DIR}, to the MATLAB Search Path. This may be due to the current user lacking the necessary filesystem permissions to modify ${NATIVE_MATLAB_PATHDEF_FILE}. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." ) endif() + +if(MATLAB_EXIT_CODE EQUAL "2") + message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fopen() failed to open file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + ) +endif() + +if(MATLAB_EXIT_CODE EQUAL "3") + message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fwrite() failed to write to the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + ) +endif() + +if(MATLAB_EXIT_CODE EQUAL "4") + message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fclose() failed to close the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + ) +endif() diff --git a/matlab/tools/addInstallDirToSearchPath.m b/matlab/tools/addInstallDirToSearchPath.m index 6c80bd590bb..4f5d73c3be5 100644 --- a/matlab/tools/addInstallDirToSearchPath.m +++ b/matlab/tools/addInstallDirToSearchPath.m @@ -1,4 +1,4 @@ -function addInstallDirToSearchPath(installDirPath) +function addInstallDirToSearchPath(installDirPath, addInstallDirToSearchPath, addInstallDirToStartupFile) % addInstallDirToSearchPath Add the input path, installDirPath, to the % MATLAB Search Path and save. % @@ -17,15 +17,38 @@ function addInstallDirToSearchPath(installDirPath) % implied. See the License for the specific language governing % permissions and limitations under the License. - addpath(installDirPath); - status = savepath(fullfile(matlabroot, "toolbox", "local", "pathdef.m")); + if addInstallDirToSearchPath == "ON" + addpath(installDirPath); + status = savepath(fullfile(matlabroot, "toolbox", "local", "pathdef.m")); - % Return exit code 1 to indicate failure and 0 to indicate the path has - % been saved successfully. - if status == 0 - disp("Sucessfully added directory to the MATLAB Search Path: " + installDirPath); + % Return exit code 1 to indicate savepath failure and 0 to indicate the path has + % been saved successfully. + if status == 0 + disp("Sucessfully added installation directory to the MATLAB Search Path: " + installDirPath); + quit(0); + else + quit(1); + end + end + + if addInstallDirToStartupFile == "ON" + fid = fopen(fullfile(userpath, "startup.m"), "a"); + if fid > 2 + count = fwrite(fid, "addpath(""" + installDirPath + """);"); + if count == 0 + % fwrite failed. + quit(3); + end + status = fclose(fid); + if status ~= 0 + % fclose failed. + quit(4); + end + else + % fopen failed. + quit(2); + end + disp("Sucessfully appended an addpath command to the MATLAB startup.m file located at the userpath to add installation directory to the MATLAB Search Path: " + installDirPath); quit(0); - else - quit(1); end end From b6f263a663fe95a9fe8c2ccac731e920f7c2c4d6 Mon Sep 17 00:00:00 2001 From: Fiona La Date: Thu, 20 Jan 2022 15:35:56 -0500 Subject: [PATCH 30/33] Update comments and error messages --- matlab/CMakeLists.txt | 11 ++++++----- matlab/tools/UpdateMatlabSearchPath.cmake | 8 ++++---- matlab/tools/addInstallDirToSearchPath.m | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index a0d44f230b1..c80746ef453 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -496,20 +496,21 @@ if(UNIX endif() endif() -# MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE sets whether the path to the install directory should -# be added to the startup.m file located at the MATLAB userpath. +# MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE toggles whether an addpath command to add the install +# directory path to the MATLAB Search Path is added to the startup.m file located in the MATLAB +# userpath directory. option(MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE "Sets whether the path to the install directory should be added to the startup.m file located at the MATLAB userpath" OFF) -# If MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE is specified ON and MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH is -# not specified, then set MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH=OFF. +# If MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE is specified ON and MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH +# is not specified, then set MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH=OFF. if(MATLAB_ADD_INSTALL_DIR_TO_STARTUP_FILE AND NOT DEFINED MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH) set(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH OFF) endif() -# MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH sets whether the path to the install directory should +# MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH toggles whether the path to the install directory should # be directly added to the MATLAB Search Path. option(MATLAB_ADD_INSTALL_DIR_TO_SEARCH_PATH "Sets whether the path to the install directory should be directly added to the MATLAB Search Path" diff --git a/matlab/tools/UpdateMatlabSearchPath.cmake b/matlab/tools/UpdateMatlabSearchPath.cmake index 50265e0a05c..178bb48ab5d 100644 --- a/matlab/tools/UpdateMatlabSearchPath.cmake +++ b/matlab/tools/UpdateMatlabSearchPath.cmake @@ -30,7 +30,7 @@ execute_process(COMMAND "${NATIVE_MATLAB_MAIN_PROGRAM}" -sd "${NATIVE_TOOLS_DIR} RESULT_VARIABLE MATLAB_EXIT_CODE) if(MATLAB_EXIT_CODE EQUAL "1") - # Get path to MATLAB pathdef.m file. This is the location of the default MATLAB Search Path + # Get path to the default MATLAB pathdef.m file. set(MATLAB_PATHDEF_FILE "${Matlab_MAIN_PROGRAM}/toolbox/local/pathdef.m") file(TO_NATIVE_PATH ${MATLAB_PATHDEF_FILE} NATIVE_MATLAB_PATHDEF_FILE) @@ -39,16 +39,16 @@ if(MATLAB_EXIT_CODE EQUAL "1") endif() if(MATLAB_EXIT_CODE EQUAL "2") - message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fopen() failed to open file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + message(FATAL_ERROR "Failed to append to the MATLAB startup.m file located at the MATLAB userpath directory. fopen() failed to open the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands." ) endif() if(MATLAB_EXIT_CODE EQUAL "3") - message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fwrite() failed to write to the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + message(FATAL_ERROR "Failed to append to the MATLAB startup.m file located at the MATLAB userpath directory. fwrite() failed to write to the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands." ) endif() if(MATLAB_EXIT_CODE EQUAL "4") - message(FATAL_ERROR "Failed to add the addpath command to the MATLAB startup.m file located at the MATLAB userpath directory: ${NATIVE_INSTALL_DIR}. fclose() failed to close the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands or by resolving the permissions issues and re-running the CMake install target." + message(FATAL_ERROR "Failed to append to the MATLAB startup.m file located at the MATLAB userpath directory. fclose() failed to close the file. In order to complete the installation process, ${NATIVE_INSTALL_DIR} must be added to the MATLAB Search Path using the \"addpath\" and \"savepath\" MATLAB commands." ) endif() diff --git a/matlab/tools/addInstallDirToSearchPath.m b/matlab/tools/addInstallDirToSearchPath.m index 4f5d73c3be5..67253439549 100644 --- a/matlab/tools/addInstallDirToSearchPath.m +++ b/matlab/tools/addInstallDirToSearchPath.m @@ -1,5 +1,5 @@ function addInstallDirToSearchPath(installDirPath, addInstallDirToSearchPath, addInstallDirToStartupFile) - % addInstallDirToSearchPath Add the input path, installDirPath, to the + % addInstallDirToSearchPath Add the input path, INSTALLDIRPATH, to the % MATLAB Search Path and save. % % Licensed to the Apache Software Foundation (ASF) under one or more From b453deea0f5af980d4efae8764437e36b5519318 Mon Sep 17 00:00:00 2001 From: lafiona Date: Mon, 31 Jan 2022 11:48:14 -0800 Subject: [PATCH 31/33] Exclude ".DS_Store" files on all platforms when installing the interface source files. Co-authored-by: Sutou Kouhei --- matlab/CMakeLists.txt | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index c80746ef453..e5b0bcab5d3 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -398,14 +398,10 @@ endif() set(CMAKE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/arrow_matlab") # Install MATLAB source files. -if(APPLE) - # On macOS, exclude '.DS_Store' files in the source tree from installation. - install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" - DESTINATION ${CMAKE_INSTALL_DIR} - PATTERN ".DS_Store" EXCLUDE) -else() - install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" DESTINATION ${CMAKE_INSTALL_DIR}) -endif() +# On macOS, exclude '.DS_Store' files in the source tree from installation. +install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" + DESTINATION ${CMAKE_INSTALL_DIR} + PATTERN ".DS_Store" EXCLUDE) # Install arrow_matlab and mexfcn. if(WIN32) From 2c0804f698a6af61ac4440722fdc8883c2eb104b Mon Sep 17 00:00:00 2001 From: lafiona Date: Mon, 31 Jan 2022 11:48:56 -0800 Subject: [PATCH 32/33] Simplify the install command for arrow_matlab and mexfcn. Co-authored-by: Sutou Kouhei --- matlab/CMakeLists.txt | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index e5b0bcab5d3..5ab78587f44 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -404,13 +404,9 @@ install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" PATTERN ".DS_Store" EXCLUDE) # Install arrow_matlab and mexfcn. -if(WIN32) - install(TARGETS arrow_matlab mexfcn RUNTIME DESTINATION ${CMAKE_INSTALL_DIR}) -else() - # On Linux install arrow_matlab.so and on macOS install arrow_matlab.dylib - # using the LIBRARY keyword. - install(TARGETS arrow_matlab mexfcn LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) -endif() +install(TARGETS arrow_matlab mexfcn + RUNTIME DESTINATION ${CMAKE_INSTALL_DIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) get_target_property(ARROW_SHARED_LIB arrow_shared IMPORTED_LOCATION) get_filename_component(ARROW_SHARED_LIB_DIR ${ARROW_SHARED_LIB} DIRECTORY) From 564c3071010d3cd389fc05e551868db88b962c51 Mon Sep 17 00:00:00 2001 From: Fiona La Date: Mon, 31 Jan 2022 14:50:50 -0500 Subject: [PATCH 33/33] Simplify logic relating to setting imported library paths for Windows, fix regex for installing on macOS --- matlab/CMakeLists.txt | 79 +++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt index c80746ef453..ec808f0f7ca 100644 --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -36,20 +36,18 @@ function(build_arrow) set(ARROW_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/arrow_ep-prefix") if(WIN32) - # On Windows, the link time suffix (.lib) and runtime library suffixes (.dll) are different. - set(ARROW_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) # The shared library is located in the "bin" directory. set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/bin") + + # Imported libraries are used + set(ARROW_IMPORT_LIB_FILENAME + "${CMAKE_IMPORT_LIBRARY_PREFIX}arrow${CMAKE_IMPORT_LIBRARY_SUFFIX}") + set(ARROW_IMPORT_LIB "${ARROW_PREFIX}/lib/${ARROW_IMPORT_LIB_FILENAME}") else() - # On Linux and macOS, the link time suffix and runtime suffixes are consistent, (.so and .dylib). - set(ARROW_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) # The shared library is located in the "lib" directory. set(ARROW_SHARED_LIBRARY_DIR "${ARROW_PREFIX}/lib") endif() - set(ARROW_LINK_LIB_FILENAME "${CMAKE_IMPORT_LIBRARY_PREFIX}arrow${ARROW_LINK_SUFFIX}") - set(ARROW_LINK_LIB "${ARROW_PREFIX}/lib/${ARROW_LINK_LIB_FILENAME}") - set(ARROW_SHARED_LIB_FILENAME "${CMAKE_SHARED_LIBRARY_PREFIX}arrow${CMAKE_SHARED_LIBRARY_SUFFIX}") set(ARROW_SHARED_LIB "${ARROW_SHARED_LIBRARY_DIR}/${ARROW_SHARED_LIB_FILENAME}") @@ -63,7 +61,7 @@ function(build_arrow) # executables. if(WIN32) # On Windows, add the Arrow link library as a BUILD_BYPRODUCTS for arrow_ep. - set(ARROW_BUILD_BYPRODUCTS "${ARROW_LINK_LIB}") + set(ARROW_BUILD_BYPRODUCTS "${ARROW_IMPORT_LIB}") else() # On Linux and macOS, add the Arrow shared library as a BUILD_BYPRODUCTS for arrow_ep. set(ARROW_BUILD_BYPRODUCTS "${ARROW_SHARED_LIB}") @@ -99,8 +97,10 @@ function(build_arrow) PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${ARROW_INCLUDE_DIR} IMPORTED_LOCATION ${ARROW_SHARED_LIB}) if(WIN32) + # On Windows, IMPORTED_IMPLIB is set to the location of arrow.lib, which is + # for linking arrow_matlab against the Arrow C++ library. set_target_properties(${ARROW_LIBRARY_TARGET} PROPERTIES IMPORTED_IMPLIB - ${ARROW_LINK_LIB}) + ${ARROW_IMPORT_LIB}) endif() add_dependencies(${ARROW_LIBRARY_TARGET} arrow_ep) @@ -115,38 +115,36 @@ macro(enable_gtest) set(ARROW_GTEST_MAIN_PREFIX "${ARROW_BINARY_DIR}/googletest_ep-prefix") if(WIN32) - set(ARROW_GTEST_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/bin") - set(ARROW_GTEST_MAIN_LINK_SUFFIX ${CMAKE_IMPORT_LIBRARY_SUFFIX}) set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/bin") + + set(ARROW_GTEST_LINK_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") + set(ARROW_GTEST_LINK_LIB + "${ARROW_GTEST_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest${CMAKE_IMPORT_LIBRARY_SUFFIX}" + ) + + set(ARROW_GTEST_MAIN_LINK_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") + set(ARROW_GTEST_MAIN_LINK_LIB + "${ARROW_GTEST_MAIN_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest_main${CMAKE_IMPORT_LIBRARY_SUFFIX}" + ) else() - set(ARROW_GTEST_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) set(ARROW_GTEST_SHARED_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") - set(ARROW_GTEST_MAIN_LINK_SUFFIX ${CMAKE_SHARED_LIBRARY_SUFFIX}) set(ARROW_GTEST_MAIN_SHARED_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") endif() set(ARROW_GTEST_INCLUDE_DIR "${ARROW_GTEST_PREFIX}/include") - set(ARROW_GTEST_LINK_LIB_DIR "${ARROW_GTEST_PREFIX}/lib") - set(ARROW_GTEST_LINK_LIB - "${ARROW_GTEST_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest${ARROW_GTEST_LINK_SUFFIX}" - ) set(ARROW_GTEST_SHARED_LIB "${ARROW_GTEST_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${CMAKE_SHARED_LIBRARY_SUFFIX}" ) set(ARROW_GTEST_MAIN_INCLUDE_DIR "${ARROW_GTEST_MAIN_PREFIX}/include") - set(ARROW_GTEST_MAIN_LINK_LIB_DIR "${ARROW_GTEST_MAIN_PREFIX}/lib") - set(ARROW_GTEST_MAIN_LINK_LIB - "${ARROW_GTEST_MAIN_LINK_LIB_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}gtest_main${ARROW_GTEST_MAIN_LINK_SUFFIX}" - ) set(ARROW_GTEST_MAIN_SHARED_LIB "${ARROW_GTEST_MAIN_SHARED_LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${CMAKE_SHARED_LIBRARY_SUFFIX}" ) list(APPEND ARROW_CMAKE_ARGS "-DARROW_BUILD_TESTS=ON") - # The appropriate libraries need to be guaranteed to be available for linking the test + # The appropriate libraries need to be guaranteed to be available when linking the test # executables. if(WIN32) # On Windows, add the gtest link libraries as BUILD_BYPRODUCTS for arrow_ep. @@ -398,23 +396,22 @@ endif() set(CMAKE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/arrow_matlab") # Install MATLAB source files. -if(APPLE) - # On macOS, exclude '.DS_Store' files in the source tree from installation. - install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" - DESTINATION ${CMAKE_INSTALL_DIR} - PATTERN ".DS_Store" EXCLUDE) -else() - install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" DESTINATION ${CMAKE_INSTALL_DIR}) -endif() - -# Install arrow_matlab and mexfcn. -if(WIN32) - install(TARGETS arrow_matlab mexfcn RUNTIME DESTINATION ${CMAKE_INSTALL_DIR}) -else() - # On Linux install arrow_matlab.so and on macOS install arrow_matlab.dylib - # using the LIBRARY keyword. - install(TARGETS arrow_matlab mexfcn LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) -endif() +# On macOS, exclude '.DS_Store' files in the source tree from installation. +install(DIRECTORY "${CMAKE_SOURCE_DIR}/src/matlab/" + DESTINATION ${CMAKE_INSTALL_DIR} + PATTERN ".DS_Store" EXCLUDE) + +# # Install arrow_matlab and mexfcn. +# if(WIN32) +# install(TARGETS arrow_matlab mexfcn RUNTIME DESTINATION ${CMAKE_INSTALL_DIR}) +# else() +# # On Linux install arrow_matlab.so and on macOS install arrow_matlab.dylib +# # using the LIBRARY keyword. +# install(TARGETS arrow_matlab mexfcn LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) +# endif() +install(TARGETS arrow_matlab mexfcn + RUNTIME DESTINATION ${CMAKE_INSTALL_DIR} + LIBRARY DESTINATION ${CMAKE_INSTALL_DIR}) get_target_property(ARROW_SHARED_LIB arrow_shared IMPORTED_LOCATION) get_filename_component(ARROW_SHARED_LIB_DIR ${ARROW_SHARED_LIB} DIRECTORY) @@ -450,7 +447,7 @@ if(APPLE) install(DIRECTORY "${ARROW_SHARED_LIB_DIR}/" DESTINATION ${CMAKE_INSTALL_DIR} FILES_MATCHING - REGEX "${ARROW_SHARED_LIB_FILENAME}\..*dylib" + REGEX "${ARROW_SHARED_LIB_FILENAME}\\..*dylib" PATTERN "cmake" EXCLUDE PATTERN "pkgconfig" EXCLUDE) @@ -486,7 +483,7 @@ if(UNIX install(DIRECTORY "${ARROW_SHARED_LIB_DIR}/" DESTINATION ${CMAKE_INSTALL_DIR} FILES_MATCHING - REGEX "${ARROW_SHARED_LIB_FILENAME}\.so.*" + REGEX "${ARROW_SHARED_LIB_FILENAME}\\.so.*" PATTERN "cmake" EXCLUDE PATTERN "pkgconfig" EXCLUDE)