kou commented on code in PR #38660:
URL: https://github.com/apache/arrow/pull/38660#discussion_r1524408214


##########
LICENSE.txt:
##########
@@ -2252,3 +2252,13 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, 
DAMAGES OR OTHER
 LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 SOFTWARE.
+
+--------------------------------------------------------------------------------
+
+3rdparty dependency mathworks/libmexclass is redistributed as a dynamically
+linked shared library in certain binary distributions, like the MATLAB
+distribution.
+
+Copyright: 2022-2024 The MathWorks, Inc. All rights reserved.
+Homepage: https://github.com/mathworks/libmexclass
+License: 3-clause BSD

Review Comment:
   We don't need this because we don't bundle mathworks/libmexclass (our source 
archive, apache-arrow-X.Y.Z.tar.gz, doesn't include mathworks/libmexclass.). 



##########
NOTICE.txt:
##########
@@ -82,3 +82,8 @@ its NOTICE file:
 
   This product includes software developed by Hewlett-Packard:
   (c) Copyright [2014-2015] Hewlett-Packard Development Company, L.P
+
+---------------------------------------------------------------------------------
+
+This product includes software from The MathWorks, Inc. (Apache 2.0)
+  * Copyright (C) 2024 The MathWorks, Inc.

Review Comment:
   We need this only for `NOTICE.txt` in `.mltbx` because our source archive 
doesn't include software from The MathWorks, Inc. See also: 
https://infra.apache.org/licensing-howto.html#binary
   
   Can we add this in `/dev/tasks/matlab/github.yml` dynamically instead of 
adding this to `/NOTICE.txt` statially? We use `/NOTICE.txt` for source archive 
too.



##########
dev/tasks/matlab/github.yml:
##########
@@ -0,0 +1,162 @@
+# 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.
+
+{% import 'macros.jinja' as macros with context %}
+
+{{ macros.github_header() }}
+
+jobs:
+
+  ubuntu:
+    name: AMD64 Ubuntu 20.04 MATLAB
+    runs-on: ubuntu-20.04
+    steps:
+      {{ macros.github_checkout_arrow()|indent }}
+      - name: Install ninja-build
+        run: sudo apt-get update && sudo apt-get install ninja-build
+      - name: Install MATLAB
+        uses: matlab-actions/setup-matlab@v1
+        with:
+          release: R2023a
+      - name: Build MATLAB Interface
+        env:
+        {{ macros.github_set_sccache_envvars()|indent(8) }}
+        run: arrow/ci/scripts/matlab_build.sh $(pwd)/arrow
+      - name: Change shared library dependency name
+        # MATLAB's programmatic packaging interface does not properly 
+        # include symbolic link files in the package MLTBX - this is a
+        # bug. As a temporary workaround, change the expected name of the 
+        # Arrow C++ library which libarrowproxy.so depends on. For example,
+        # change libarrow.so.1500 to libarrow.so.1500.0.0.
+        run: |
+          pushd arrow/matlab/install/arrow_matlab/+libmexclass/+proxy/ 
+          SYMLINK_ARROW_LIB="$(find . -name 'libarrow.so.*' -type l | xargs 
basename)"
+          REGULAR_ARROW_LIB="$(echo libarrow.so.*.*)"
+          echo "SYMLINK_ARROW_LIB = ${SYMLINK_ARROW_LIB}"
+          echo "REGULAR_ARROW_LIB = ${REGULAR_ARROW_LIB}"
+          patchelf --replace-needed $SYMLINK_ARROW_LIB $REGULAR_ARROW_LIB 
libarrowproxy.so
+          popd
+      - name: Compress into single artifact
+        run: tar -cvzf matlab-arrow-ubuntu.tar.gz 
arrow/matlab/install/arrow_matlab
+      - name: Upload artifacts
+        uses: actions/upload-artifact@v2

Review Comment:
   Could you use the latest version?
   See also: https://github.com/actions/upload-artifact
   
   ```suggestion
           uses: actions/upload-artifact@v4
   ```



##########
dev/tasks/matlab/github.yml:
##########
@@ -0,0 +1,162 @@
+# 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.
+
+{% import 'macros.jinja' as macros with context %}
+
+{{ macros.github_header() }}
+
+jobs:
+
+  ubuntu:
+    name: AMD64 Ubuntu 20.04 MATLAB
+    runs-on: ubuntu-20.04
+    steps:
+      {{ macros.github_checkout_arrow()|indent }}
+      - name: Install ninja-build
+        run: sudo apt-get update && sudo apt-get install ninja-build
+      - name: Install MATLAB
+        uses: matlab-actions/setup-matlab@v1
+        with:
+          release: R2023a
+      - name: Build MATLAB Interface
+        env:
+        {{ macros.github_set_sccache_envvars()|indent(8) }}
+        run: arrow/ci/scripts/matlab_build.sh $(pwd)/arrow
+      - name: Change shared library dependency name
+        # MATLAB's programmatic packaging interface does not properly 
+        # include symbolic link files in the package MLTBX - this is a
+        # bug. As a temporary workaround, change the expected name of the 
+        # Arrow C++ library which libarrowproxy.so depends on. For example,
+        # change libarrow.so.1500 to libarrow.so.1500.0.0.
+        run: |
+          pushd arrow/matlab/install/arrow_matlab/+libmexclass/+proxy/ 
+          SYMLINK_ARROW_LIB="$(find . -name 'libarrow.so.*' -type l | xargs 
basename)"
+          REGULAR_ARROW_LIB="$(echo libarrow.so.*.*)"
+          echo "SYMLINK_ARROW_LIB = ${SYMLINK_ARROW_LIB}"
+          echo "REGULAR_ARROW_LIB = ${REGULAR_ARROW_LIB}"
+          patchelf --replace-needed $SYMLINK_ARROW_LIB $REGULAR_ARROW_LIB 
libarrowproxy.so
+          popd
+      - name: Compress into single artifact
+        run: tar -cvzf matlab-arrow-ubuntu.tar.gz 
arrow/matlab/install/arrow_matlab
+      - name: Upload artifacts
+        uses: actions/upload-artifact@v2
+        with:
+          name: matlab-arrow-ubuntu.tar.gz
+          path: matlab-arrow-ubuntu.tar.gz
+
+  macos:
+    name: AMD64 macOS 11 MATLAB

Review Comment:
   `macos-latest` is 12 now. :-)
   
   ```suggestion
       name: AMD64 macOS 12 MATLAB
   ```



##########
matlab/tools/packageMatlabInterface.m:
##########
@@ -0,0 +1,60 @@
+% 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.
+
+toolboxFolder = string(getenv("ARROW_MATLAB_TOOLBOX_FOLDER"));
+outputFolder = string(getenv("ARROW_MATLAB_TOOLBOX_OUTPUT_FOLDER"));
+toolboxVersionRaw = string(getenv("ARROW_MATLAB_TOOLBOX_VERSION"));
+
+% Output folder must exist.
+mkdir(outputFolder);
+
+disp("Toolbox Folder: " + toolboxFolder);
+disp("Output Folder: " + outputFolder);
+disp("Toolbox Version Raw: " + toolboxVersionRaw);
+
+% Note: This string processing heuristic may not be robust to future
+% changes in the Arrow versioning scheme.
+dotIdx = strfind(toolboxVersionRaw, ".");
+numDots = numel(dotIdx);
+if numDots >= 3
+    toolboxVersion = extractBefore(toolboxVersionRaw, dotIdx(3));
+else
+    toolboxVersion = toolboxVersionRaw;
+end
+
+disp("Toolbox Version:" + toolboxVersion);
+
+identifier = "ad1d0fe6-22d1-4969-9e6f-0ab5d0f12ce3";
+opts = matlab.addons.toolbox.ToolboxOptions(toolboxFolder, identifier);
+opts.ToolboxName = "MATLAB Arrow Interface";
+opts.ToolboxVersion = toolboxVersion;
+opts.AuthorName = "Apache Arrow";

Review Comment:
   Could you use ASF?
   
   ```suggestion
   opts.AuthorName = "The Apache Software Foundation";
   ```



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to