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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
