You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "leihou6116 (via GitHub)" <gi...@apache.org> on 2023/05/26 15:03:40 UTC
[GitHub] [arrow] leihou6116 opened a new pull request, #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
leihou6116 opened a new pull request, #35792:
URL: https://github.com/apache/arrow/pull/35792
### Rationale for this change
Currently GitHub Actions builds arrow/matlab on Linux and Mac but not on Windows. This change is to enable the build on Windows such that we have qualification coverage on all popular platforms.
### What changes are included in this PR?
The build on windows uses Ninja and Visual Studio to build. This submission is aimed to enable the CI on Windows. In the future, we'll try to build using Ninja and MinGW.
### Are these changes tested?
Yes. When I submitted the change, I verified that the Windows CI was enabled that tests were run successfully.
### Are there any user-facing changes?
No.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 closed pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 closed pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
URL: https://github.com/apache/arrow/pull/35792
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1239257315
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
Review Comment:
```suggestion
# On macOS, we need to match these files: libarrow.dylib
```
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
+ AND NOT CYGWIN)
Review Comment:
Do we want to support Cygwin? If we don't, how about removing this condition?
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
Review Comment:
Can we simplify this?
```suggestion
set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}([.][0-9]+)*${CMAKE_SHARED_LIBRARY_SUFFIX}")
```
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
Review Comment:
This is redundant.
```suggestion
```
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
+ AND NOT CYGWIN)
+ # Install libarrow.so (symlink) and the real files it points to.
+ # On Linux, we need to match these files: libarrow.so
+ # libarrow.so.1200
+ # libarrow.so.1200.0.0
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}${CMAKE_SHARED_LIBRARY_SUFFIX}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)")
Review Comment:
Can we simplify this?
```suggestion
set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}${CMAKE_SHARED_LIBRARY_SUFFIX}([.][0-9]+)*")
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1611489671
Hi Kou, about whether to keep the check of Cygwin or not, I asked my team. We are trying to be more specific in the if statement. Regarding the if branch for Linux, we only want Linux to go through it but not Linux-like. CYGWIN is Linux-like. So we decided to exclude it explicitly.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1207595461
##########
matlab/CMakeLists.txt:
##########
@@ -542,6 +542,23 @@ if(UNIX
endif()
endif()
+if(WIN32)
Review Comment:
It seems that we have similar code for `Apple`.
Can we unify them by using `CMAKE_SHARED_LIBRARY_PATH` variable?
https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LIBRARY_SUFFIX.html
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242223209
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
Review Comment:
Hi Kou, thank you for your review feedback. In CmakeLists, the regular expression doesn't support {}. So I cannot use regular expression like this ([.][0-9]+)){0,3} to match numbers up to 3 times. I have to literally list all of them as (([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?).
=== Cmake regular expression document. ===
https://cmake.org/cmake/help/latest/command/string.html#regex-specification
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242993282
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
+ AND NOT CYGWIN)
Review Comment:
I see.
FYI: Apache Arrow C++ works on MSYS2 but doesn't support Cygwin. (It may work on Cygwin but we don't test it.) If Apache Arrow MATLAB supports Cygwin, at first, we need to work on Apache Arrow C++.
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
Review Comment:
I see.
FYI: If we want to strict pattern, we should use `(([.][0-9]+)(([.][0-9]+)([.][0-9]+))?)?`, escape `.` in `CMAKE_SHARED_LIBRARY_SUFFIX` and so on. But I think that it's over-engineering because it will not cause a problem. Because file names will not be changed in general and we can control Apache Arrow C++ (it exists in the same repository).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1565196557
For build time reduction using ccache as in the cpp workflows would probably be best as most of the build time comes from arrow.
There is also an issue with the ` ilammy/msvc-dev-cmd@v1` action as it is not on the asf infra white list. Ideally we would just get rid of the action and do it manually, we might even have a script that does this already in dev/ or ci/ :D
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1570451874
> For build time reduction using ccache as in the cpp workflows would probably be best as most of the build time comes from arrow.
>
> There is also an issue with the ` ilammy/msvc-dev-cmd@v1` action as it is not on the asf infra allow list. Ideally we would just get rid of the action and do it manually, we might even have a script that does this already in dev/ or ci/ :D
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242991861
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
Review Comment:
I see.
FYI: If we want to use strict pattern, we should use `(([.][0-9]+)(([.][0-9]+)([.][0-9]+))?)?`, escape `.` in `CMAKE_SHARED_LIBRARY_SUFFIX` and so on. But I think that it's over-engineering because it will not cause a problem. Because file names will not be changed in general and we can control Apache Arrow C++ (it exists in the same repository).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1612370243
Could you fix the "Dev / Lint C++, Python, R, Docker, RAT" job by `archery lint --cmake-format --fix`?
https://github.com/apache/arrow/actions/runs/5380716556/jobs/9772450410?pr=35792#step:5:873
```text
ERROR __main__.py:618: Check failed: /arrow/matlab/CMakeLists.txt
```
See also:
* https://arrow.apache.org/docs/dev/developers/continuous_integration/archery.html
* https://arrow.apache.org/docs/dev/developers/cpp/development.html#code-style-linting-and-ci
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1622860439
@kevingurney @sgilmore10 Could you help @leihou6116?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1570453826
Closed accidentally.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1219932479
##########
.github/workflows/matlab.yml:
##########
@@ -94,3 +130,48 @@ jobs:
uses: matlab-actions/run-tests@v1
with:
select-by-folder: matlab/test
+ windows:
+ name: AMD64 Windows 2022 MATLAB
+ runs-on: windows-2022
+ if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+ steps:
+ - name: Check out repository
+ uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+ - name: Install MATLAB
+ uses: matlab-actions/setup-matlab@v1
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - name: Install ccache
+ shell: bash
+ run: ci/scripts/install_ccache.sh 4.6.3 /usr
+ - name: Setup ccache
+ shell: bash
+ run: ci/scripts/ccache_setup.sh
+ - name: Get Date
+ id: get-date
+ shell: bash
+ run: |
+ echo "date=$(/bin/date -u "+%Y%m%d")" >> $GITHUB_OUTPUT
+ - name: Cache ccache
+ uses: actions/cache@v3
+ with:
+ path: |
+ cpp/**
+ key: cpp-ccache-for-matlab-windows-${{ steps.get-date.outputs.date }}-${{ hashFiles('cpp/**') }}
Review Comment:
Hi Jacob, that is a good question. I added the date such that we could track the history about when the latest cache was generated. The restore-key doesn't match the date. So it should find the latest cache even though the creation date in the key name is not the current date. My current version doesn't reduce build time. I need to figure out why, either the changing-date causing the problem or the value of "path" is not right. If you know the right value for files under cpp/**, please teach me. I really appreciate it. In the meantime, I'll do exercise by trying different values. Sorry for so many trial submissions.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1225022588
##########
.github/workflows/matlab.yml:
##########
@@ -94,3 +130,48 @@ jobs:
uses: matlab-actions/run-tests@v1
with:
select-by-folder: matlab/test
+ windows:
+ name: AMD64 Windows 2022 MATLAB
+ runs-on: windows-2022
+ if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+ steps:
+ - name: Check out repository
+ uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+ - name: Install MATLAB
+ uses: matlab-actions/setup-matlab@v1
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - name: Install ccache
+ shell: bash
+ run: ci/scripts/install_ccache.sh 4.6.3 /usr
+ - name: Setup ccache
+ shell: bash
+ run: ci/scripts/ccache_setup.sh
+ - name: Get Date
+ id: get-date
+ shell: bash
+ run: |
+ echo "date=$(/bin/date -u "+%Y%m%d")" >> $GITHUB_OUTPUT
+ - name: Cache ccache
+ uses: actions/cache@v3
+ with:
+ path: |
+ cpp/**
+ key: cpp-ccache-for-matlab-windows-${{ steps.get-date.outputs.date }}-${{ hashFiles('cpp/**') }}
Review Comment:
Ah I see, makes sense :)
It looks like it is working now: https://github.com/mathworks/arrow/actions/runs/5204445685/jobs/9388659172#step:9:73 :tada:
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1624009044
It looks like the MATLAB macOS 11 build failed due to the sporadic linker error we've seen on and off. I think if we re-run that job, it should pass.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1574616150
Thanks this inspired me to open #35896 :D
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1603505823
I addressed all review feedbacks:
- Unified the installation code in CMakeLists when ARROW is not found.
- Used ccache to reduce build time. It saves 20-30 min on each platform.
- Removed the dependency on ilammy/msvc-dev-cmd@v1 when using Microsoft Visual Studio as compiler.
Please help review the latest change. Thanks for reviewing it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou merged pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #35792:
URL: https://github.com/apache/arrow/pull/35792
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1611525856
The failed and cancelled checks are unrelated to my change. The action job for only my change passed, see https://github.com/mathworks/arrow/actions/runs/5335009120
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1564530207
* Closes: #20047
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1624013137
> It looks like the MATLAB macOS 11 build failed due to the sporadic linker error we've seen on and off. I think if we re-run that job, it should pass.
>
> We're still looking into that issue. It's unfortunately difficult to reliably reproduce.
The lastest submission - fix linting error - passed on all three platforms.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1622598313
I'm working on the rebasing issue. This is my first time working on GitHub. I'm sorry that I messed up the rebase history. I'm working on it and try to come up with a good solution.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] conbench-apache-arrow[bot] commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1635342999
Conbench analyzed the 6 benchmark runs on commit `3c9b4573`.
There was 1 benchmark result indicating a performance regression:
- Commit Run on `arm64-m6g-linux-compute` at [2023-07-07 04:02:05Z](http://conbench.ursa.dev/compare/runs/e59ffd78496c44a49ad6a993959334b5...2cb268fbc1cb402691d61ad3991f13fe/)
- [params=<STATIC_VECTOR(std::string)>, source=cpp-micro, suite=arrow-small-vector-benchmark](http://conbench.ursa.dev/compare/benchmarks/064a77b44bc9719c800029f62615b0e6...064a78e8198c75958000a327629a198f)
The [full Conbench report](https://github.com/apache/arrow/runs/15036697761) has more details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242224291
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
Review Comment:
Good catch. I'll remove this. Thank you.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242369097
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
Review Comment:
Hi Kou, we only want to match libarrow.so and libarrow.so.<number>.<number>.<number>. We don't want to match other files. That is why we chose to not use "*".
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1242374906
##########
matlab/CMakeLists.txt:
##########
@@ -520,28 +503,48 @@ if(UNIX
# at runtime.
set_target_properties(mexcall 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_PACKAGED_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_PACKAGED_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)
+ # 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()
+
+if(NOT Arrow_FOUND)
+ # If Arrow_FOUND is false, Arrow is built by the arrow_shared target and needs
+ # to be copied to CMAKE_PACKAGED_INSTALL_DIR.
+
+ if(APPLE)
+ # Install libarrow.dylib (symlink) and the real files it points to.
+ # on MacOS, we need to match these files: libarrow.dylib
+ # libarrow.1300.dylib
+ # libarrow.1300.0.0.dylib
+ # where the version number might change.
+ set(SHARED_LIBRARY_VERSION_REGEX "${ARROW_SHARED_LIB_FILENAME}(([.][0-9]+)?([.][0-9]+)?([.][0-9]+)?)${CMAKE_SHARED_LIBRARY_SUFFIX}")
+ elseif(UNIX
+ AND NOT APPLE
+ AND NOT CYGWIN)
Review Comment:
In the future, we might want to allocate resource to support Cygwin. It would be great that matlab/arrow supports as many platforms as possible in the long run.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1621769647
@leihou6116 Can you answer @kou 's request above? Also, can you merge or rebase from git main?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] sgilmore10 commented on pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "sgilmore10 (via GitHub)" <gi...@apache.org>.
sgilmore10 commented on PR #35792:
URL: https://github.com/apache/arrow/pull/35792#issuecomment-1623751404
> @kevingurney @sgilmore10 Could you help @leihou6116?
@leihou6116 and I rebased on main. I think everything is good to go.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] leihou6116 commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "leihou6116 (via GitHub)" <gi...@apache.org>.
leihou6116 commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1210503085
##########
matlab/CMakeLists.txt:
##########
@@ -542,6 +542,23 @@ if(UNIX
endif()
endif()
+if(WIN32)
Review Comment:
Thank you for your suggestion. It would be great to unify the code. I'll try and get update soon.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on a diff in pull request #35792: GH-20047: [MATLAB] Enable GitHub Actions CI for MATLAB Interface on Windows
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #35792:
URL: https://github.com/apache/arrow/pull/35792#discussion_r1218840719
##########
.github/workflows/matlab.yml:
##########
@@ -94,3 +130,48 @@ jobs:
uses: matlab-actions/run-tests@v1
with:
select-by-folder: matlab/test
+ windows:
+ name: AMD64 Windows 2022 MATLAB
+ runs-on: windows-2022
+ if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
+ steps:
+ - name: Check out repository
+ uses: actions/checkout@v3
+ with:
+ fetch-depth: 0
+ - name: Install MATLAB
+ uses: matlab-actions/setup-matlab@v1
+ - name: Download Timezone Database
+ shell: bash
+ run: ci/scripts/download_tz_database.sh
+ - name: Install ccache
+ shell: bash
+ run: ci/scripts/install_ccache.sh 4.6.3 /usr
+ - name: Setup ccache
+ shell: bash
+ run: ci/scripts/ccache_setup.sh
+ - name: Get Date
+ id: get-date
+ shell: bash
+ run: |
+ echo "date=$(/bin/date -u "+%Y%m%d")" >> $GITHUB_OUTPUT
+ - name: Cache ccache
+ uses: actions/cache@v3
+ with:
+ path: |
+ cpp/**
+ key: cpp-ccache-for-matlab-windows-${{ steps.get-date.outputs.date }}-${{ hashFiles('cpp/**') }}
Review Comment:
If you hash the cpp files why also add the date? Why is a cache from yesterday for state x not valid today?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org