You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/06/08 04:39:00 UTC
[GitHub] [arrow] kou commented on a change in pull request #10477: ARROW-13002: [C++] Add a check for the utf8proc's version in CMake
kou commented on a change in pull request #10477:
URL: https://github.com/apache/arrow/pull/10477#discussion_r647103927
##########
File path: cpp/cmake_modules/Findutf8proc.cmake
##########
@@ -15,6 +15,31 @@
# specific language governing permissions and limitations
# under the License.
+function(EXTRACT_UTF8PROC_VERSION)
+ if(utf8proc_INCLUDE_DIR)
+ file(READ "${utf8proc_INCLUDE_DIR}/utf8proc.h" UTF8PROC_H_CONTENT)
+
+ string(REGEX MATCH "#define UTF8PROC_VERSION_MAJOR [0-9]+" UTF8PROC_MAJOR_VERSION_DEFINITION
+ "${UTF8PROC_H_CONTENT}")
+ string(REGEX MATCH "#define UTF8PROC_VERSION_MINOR [0-9]+" UTF8PROC_MINOR_VERSION_DEFINITION
+ "${UTF8PROC_H_CONTENT}")
+ string(REGEX MATCH "#define UTF8PROC_VERSION_PATCH [0-9]+" UTF8PROC_PATCH_VERSION_DEFINITION
+ "${UTF8PROC_H_CONTENT}")
+
+ string(REGEX MATCH " [0-9.]+" UTF8PROC_MAJOR_VERSION "${UTF8PROC_MAJOR_VERSION_DEFINITION}")
+ string(REGEX MATCH " [0-9.]+" UTF8PROC_MINOR_VERSION "${UTF8PROC_MINOR_VERSION_DEFINITION}")
+ string(REGEX MATCH " [0-9.]+" UTF8PROC_PATCH_VERSION "${UTF8PROC_PATCH_VERSION_DEFINITION}")
+
+ string(STRIP "${UTF8PROC_MAJOR_VERSION}" UTF8PROC_MAJOR_VERSION)
+ string(STRIP "${UTF8PROC_MINOR_VERSION}" UTF8PROC_MINOR_VERSION)
+ string(STRIP "${UTF8PROC_PATCH_VERSION}" UTF8PROC_PATCH_VERSION)
+
+ set(UTF8PROC_VERSION "${UTF8PROC_MAJOR_VERSION}.${UTF8PROC_MINOR_VERSION}.${UTF8PROC_PATCH_VERSION}" PARENT_SCOPE)
Review comment:
Could you use `utf8proc_` prefix for variable that is exported because this package's name is `utf8proc`?
```suggestion
set(utf8proc_VERSION "${UTF8PROC_MAJOR_VERSION}.${UTF8PROC_MINOR_VERSION}.${UTF8PROC_PATCH_VERSION}" PARENT_SCOPE)
```
##########
File path: cpp/cmake_modules/Findutf8proc.cmake
##########
@@ -15,6 +15,31 @@
# specific language governing permissions and limitations
# under the License.
+function(EXTRACT_UTF8PROC_VERSION)
Review comment:
Could you use lower case because we use lower case for function call?
```suggestion
function(extract_utf8proc_version)
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org