You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/06/03 08:04:10 UTC

[GitHub] [incubator-kvrocks] PragmaTwice opened a new pull request, #618: Build static lib of glog in CMake

PragmaTwice opened a new pull request, #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618

   Fix #615.


-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888847179


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Static libraries allow users to not worry about library versioning issues and linking issues, and are more portable (just like go / rust, statically link almost everything), which I think is probably secondary to the increase in size of these binaries.
   And `BUILD_SHARED_LIBS` only controls the default behavior, if we want to build shared libraries then we can still specify it manually.



-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on pull request #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#issuecomment-1146508587

   Merging...
   
   FWIW a discussion (#615) cannot be resolved. You may create an issue from the discussion the next time.


-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#issuecomment-1146524618

   > Merging...
   > 
   > FWIW a discussion (#615) cannot be resolved. You may create an issue from the discussion the next time.
   
   My mistake, will open an issue next time :)


-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888847179


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Static libraries allow users to not worry about library versioning issues and linking issues, and are more portable (just like go / rust, statically link almost everything), which I think is probably secondary to the increase in size of these binaries.



-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888847179


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Static libraries allow users to not worry about library versioning issues and linking issues, and are more portable (just like go / rust, statically link almost everything), which I think is probably secondary to the increase in size of these binaries.
   And `BUILD_SHARED_LIBS` only controls the default behavior, if we want to build shared libraries then we can still specify `SHARED` manually.



-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888847179


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Static libraries allow users to not worry about library versioning issues and linking issues, and are more portable (just like go / rust, statically link almost everything), which I think is probably secondary to the increase in size of these binaries.
   And `BUILD_SHARED_LIBS` only controls the default behavior, if we want to build dynamic libraries then we can still specify it manually.



-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r889449551


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Thanks for your explanation. Sounds good to me.



-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888836596


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   Why do we set this flag on root CMakeLists also?
   
   BTW, the comment seems magic without background (why forbid shared lib?).



-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888844042


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   I think we just want to build static libraries of dependences and link them statically, so I added it to CMakeLists to avoid some dependencies set this cache variable to true in their cmake to pollute this configuration. (cache variables can only be assigned once, and the next assignment needs to specify `FORCE`, otherwise it is invalid)



-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun merged pull request #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618


-- 
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: dev-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 #618: Build static lib of glog in CMake

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #618:
URL: https://github.com/apache/incubator-kvrocks/pull/618#discussion_r888844042


##########
CMakeLists.txt:
##########
@@ -45,6 +45,8 @@ if(NOT DISABLE_JEMALLOC)
     list(APPEND EXTERNAL_LIBS PRIVATE jemalloc)
 endif()
 
+set(BUILD_SHARED_LIBS OFF CACHE BOOL "do not build shared libs by default")

Review Comment:
   I think we just want to build static libraries of dependences and link them statically, so I added it to CMakeLists to avoid some dependencies set this cache variable to true in their cmake to pollute the global configuration. (cache variables can only be assigned once, and the next assignment needs to specify `FORCE`, otherwise it is invalid)



-- 
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: dev-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org