You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "git-hulk (via GitHub)" <gi...@apache.org> on 2023/02/07 13:04:49 UTC
[GitHub] [incubator-kvrocks] git-hulk opened a new pull request, #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
git-hulk opened a new pull request, #1250:
URL: https://github.com/apache/incubator-kvrocks/pull/1250
Related issue: #776
We now can automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version like below:
```
-- CMAKE_OSX_DEPLOYMENT_TARGET was set to: 12.5
```
This change can make developers who are using macOS happy.
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
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 this `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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1250:
URL: https://github.com/apache/incubator-kvrocks/pull/1250#discussion_r1098708444
##########
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:
Good advice, will update
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1250:
URL: https://github.com/apache/incubator-kvrocks/pull/1250#issuecomment-1421846303
Thanks all, mergeing...
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk merged pull request #1250: Automatically set the env MACOSX_DEPLOYMENT_TARGET to the current macOS version unless the env is already set
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #1250:
URL: https://github.com/apache/incubator-kvrocks/pull/1250
--
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: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org