PragmaTwice commented on code in PR #1250:
URL:
https://github.com/apache/incubator-kvrocks/pull/1250#discussion_r1098701344
##########
cmake/luajit.cmake:
##########
@@ -19,27 +19,42 @@ include_guard()
include(cmake/utils.cmake)
-if((${CMAKE_SYSTEM_NAME} MATCHES "Darwin") AND (NOT
CMAKE_OSX_DEPLOYMENT_TARGET))
+if ((${CMAKE_SYSTEM_NAME} MATCHES "Darwin") AND (NOT
CMAKE_OSX_DEPLOYMENT_TARGET))
+ set(sw_cmd "sw_vers")
+ set(sw_arg "-productVersion")
+ execute_process(COMMAND ${sw_cmd} ${sw_arg}
+ WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+ OUTPUT_VARIABLE OUTPUT
+ RESULT_VARIABLE STATUS
+ OUTPUT_STRIP_TRAILING_WHITESPACE)
+ if (STATUS EQUAL 0)
+ string(REGEX MATCH "[0-9]+.[0-9]+"
+ DEPLOYMENT_TARGET ${OUTPUT})
+ set(CMAKE_OSX_DEPLOYMENT_TARGET "${DEPLOYMENT_TARGET}")
+ message(STATUS "CMAKE_OSX_DEPLOYMENT_TARGET was set to:
${DEPLOYMENT_TARGET}")
+ endif ()
Review Comment:
CMake variables has no regular scope (in `if` or `include`), so I prefer
more concrete names, e.g. `SW_CMD_OUTPUT` rather than `OUTPUT`, `SW_CMD_STATUS`
rather than `STATUS`.
Otherwise we can access `OUTPUT` variable anywhere after the code above is
executed.
--
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]