You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@quickstep.apache.org by hbdeshmukh <gi...@git.apache.org> on 2016/06/10 16:00:31 UTC

[GitHub] incubator-quickstep pull request #24: Configure libtcmalloc_minimal based on...

GitHub user hbdeshmukh opened a pull request:

    https://github.com/apache/incubator-quickstep/pull/24

    Configure libtcmalloc_minimal based on shared libs setting

    - Added a switch to change the way we build libtcmalloc_minimal i.e. either a static library or shared library.
    - The build works successfully when tcmalloc is enabled (which is the default setting).
    
    Thanks @navsan for debugging the issue. 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/incubator-quickstep tcmalloc-sharelib-fix

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-quickstep/pull/24.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #24
    
----
commit dcb2e2e19de54a3100f0d38637dd755ef0ef925a
Author: Harshad Deshmukh <hb...@apache.org>
Date:   2016-06-10T15:55:49Z

    Configure libtcmalloc_minimal based on shared libs setting
    
    - Added a switch to change the way we build libtcmalloc_minimal i.e.
      either a static library or shared library.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #24: Configure libtcmalloc_minimal based on...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/24#discussion_r66644724
  
    --- Diff: CMakeLists.txt ---
    @@ -500,9 +500,13 @@ if(USE_TCMALLOC)
         #   as some generators, e.g. Ninja, may need it to build properly
         # BUILD_BYPRODUCTS <INSTALL_DIR>/lib/libtcmalloc_minimal.a
       )
    -  # Static libtcmalloc_minimal.a
    -  add_library(libtcmalloc_minimal STATIC IMPORTED)
    -  set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    +  if (BUILD_SHARED_LIBS)
    +    add_library(libtcmalloc_minimal SHARED IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.so)
    +  else()
    +    add_library(libtcmalloc_minimal STATIC IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    --- End diff --
    
    It seems that we could move this line out of the `if-else` statement.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #24: Configure libtcmalloc_minimal based on...

Posted by navsan <gi...@git.apache.org>.
Github user navsan commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/24#discussion_r66645002
  
    --- Diff: CMakeLists.txt ---
    @@ -500,9 +500,13 @@ if(USE_TCMALLOC)
         #   as some generators, e.g. Ninja, may need it to build properly
         # BUILD_BYPRODUCTS <INSTALL_DIR>/lib/libtcmalloc_minimal.a
       )
    -  # Static libtcmalloc_minimal.a
    -  add_library(libtcmalloc_minimal STATIC IMPORTED)
    -  set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    +  if (BUILD_SHARED_LIBS)
    +    add_library(libtcmalloc_minimal SHARED IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.so)
    +  else()
    +    add_library(libtcmalloc_minimal STATIC IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    --- End diff --
    
    Not really, the file name is different based on whether it's a shared or static library.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #24: Configure libtcmalloc_minimal based on shared...

Posted by hbdeshmukh <gi...@git.apache.org>.
Github user hbdeshmukh commented on the issue:

    https://github.com/apache/incubator-quickstep/pull/24
  
    You mean close the PR right? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #24: Configure libtcmalloc_minimal based on...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-quickstep/pull/24


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #24: Configure libtcmalloc_minimal based on...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on a diff in the pull request:

    https://github.com/apache/incubator-quickstep/pull/24#discussion_r66648781
  
    --- Diff: CMakeLists.txt ---
    @@ -500,9 +500,13 @@ if(USE_TCMALLOC)
         #   as some generators, e.g. Ninja, may need it to build properly
         # BUILD_BYPRODUCTS <INSTALL_DIR>/lib/libtcmalloc_minimal.a
       )
    -  # Static libtcmalloc_minimal.a
    -  add_library(libtcmalloc_minimal STATIC IMPORTED)
    -  set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    +  if (BUILD_SHARED_LIBS)
    +    add_library(libtcmalloc_minimal SHARED IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.so)
    +  else()
    +    add_library(libtcmalloc_minimal STATIC IMPORTED)
    +    set_property(TARGET libtcmalloc_minimal PROPERTY IMPORTED_LOCATION ${CMAKE_CURRENT_BINARY_DIR}/third_party/gperftools/lib/libtcmalloc_minimal.a)
    --- End diff --
    
    LGTM!
    
    Oh, I missed the last part: `.so` vs `.a`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #24: Configure libtcmalloc_minimal based on shared...

Posted by zuyu <gi...@git.apache.org>.
Github user zuyu commented on the issue:

    https://github.com/apache/incubator-quickstep/pull/24
  
    @hbdeshmukh I've already merged it to master, but for some reasons, it does not close by asfgit.
    
    Could you please close it? Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---