Re: [PATCH v2] KernelShark: Inherit libdir from Makefile

[Zamir SUN <sztsian@gmail.com>]

On 2/21/20 9:12 PM, Yordan Karadzhov (VMware) wrote:
> Hi Zamir,
> 
> On 21.02.20 г. 14:42 ч., sztsian@gmail.com wrote:
>> From: "Ziqian SUN (Zamir)" <sztsian@gmail.com>
>>
>> The trace-cmd makefile supports install lib into a different name like
>> lib64. Now this patch implemented the same in kernel-shark.
>>
>> Change since v1: Drop the change of EXECUTABLE_OUTPUT_PATH and fix set
>> command to lower case.
>>
>> Signed-off-by: Ziqian SUN (Zamir) <sztsian@gmail.com>
>> ---
>>   Makefile                        |  2 +-
>>   kernel-shark/CMakeLists.txt     | 11 ++++++++---
>>   kernel-shark/src/CMakeLists.txt |  6 +++---
>>   3 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index d75f143..1aca807 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -295,7 +295,7 @@ CMAKE_COMMAND = /usr/bin/cmake
>>   BUILD_TYPE ?= RelWithDebInfo
>>   $(kshark-dir)/build/Makefile: $(kshark-dir)/CMakeLists.txt
>> -    $(Q) cd $(kshark-dir)/build && $(CMAKE_COMMAND) 
>> -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -D_INSTALL_PREFIX=$(prefix) ..
>> +    $(Q) cd $(kshark-dir)/build && $(CMAKE_COMMAND) 
>> -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -D_INSTALL_PREFIX=$(prefix) 
>> -D_LIBDIR=$(libdir) ..
>>   gui: force $(CMD_TARGETS) $(kshark-dir)/build/Makefile
>>       $(Q)$(MAKE) $(S) -C $(kshark-dir)/build
>> diff --git a/kernel-shark/CMakeLists.txt b/kernel-shark/CMakeLists.txt
>> index 8786b83..c60d29b 100644
>> --- a/kernel-shark/CMakeLists.txt
>> +++ b/kernel-shark/CMakeLists.txt
>> @@ -17,6 +17,10 @@ if (NOT _INSTALL_PREFIX)
>>       set(_INSTALL_PREFIX "/usr/local")
>>   endif (NOT _INSTALL_PREFIX)
>> +if (NOT _LIBDIR)
>> +    set(_LIBDIR "${_INSTALL_PREFIX}/lib")
>> +endif (NOT _LIBDIR)
>> +
>>   include(${KS_DIR}/build/FindTraceCmd.cmake)
>>   include(${KS_DIR}/build/FindJSONC.cmake)
>> @@ -34,7 +38,7 @@ if (Qt5Widgets_FOUND)
>>   endif (Qt5Widgets_FOUND)
>> -set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/lib")
>> +set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/${_LIBDIR}")
> 
> Again, I am not sure what is your idea here.
> As I already mentioned in my previous review, LIBRARY_OUTPUT_PATH (and 
> EXECUTABLE_OUTPUT_PATH) has nothing to do with the installation path of 
> the libraries. See
> https://cmake.org/cmake/help/v3.0/prop_tgt/LIBRARY_OUTPUT_DIRECTORY.html
> 
> This specifies the directory into which library target files are built.
> Not the directory into which the library files will be installed.
> 

Hi Yordan,

I was thinking you only have objections to my modification of the 
EXECUTABLE_OUTPUT_PATH. I just compiled and confirm without this line 
packaging still works as expected. I'll send out the updated one shortly.

Really sorry for make this a bit messy.

> Thanks!
> Yordan
> 
>>   set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/bin")
>>   set(CMAKE_C_FLAGS   "${CMAKE_C_FLAGS} -Wall -pthread -fPIC")
>> @@ -54,14 +58,15 @@ if (NOT CMAKE_CXX_FLAGS_PACKAGE)
>>       set(CMAKE_CXX_FLAGS_PACKAGE "-O3")
>>   endif (NOT CMAKE_CXX_FLAGS_PACKAGE)
>> -set(KS_PLUGIN_INSTALL_PREFIX 
>> ${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/plugins/)
>> +set(KS_PLUGIN_INSTALL_PREFIX ${_LIBDIR}/${KS_APP_NAME}/plugins/)
>>   set(KS_ICON        KS_icon_shark.svg)
>>   set(KS_ICON_FIN    KS_icon_fin.svg)
>>   set(KS_LOGO        KS_logo_symbol.svg)
>>   set(KS_LOGO_LABEL  KS_logo_horizontal.svg)
>> -set(CMAKE_INSTALL_RPATH "${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/")
>> +set(CMAKE_INSTALL_RPATH "${_LIBDIR}/${KS_APP_NAME}/")
>> +
>>   if (CMAKE_BUILD_TYPE MATCHES Package)
>> diff --git a/kernel-shark/src/CMakeLists.txt 
>> b/kernel-shark/src/CMakeLists.txt
>> index 33b5db8..9666b18 100644
>> --- a/kernel-shark/src/CMakeLists.txt
>> +++ b/kernel-shark/src/CMakeLists.txt
>> @@ -15,7 +15,7 @@ target_link_libraries(kshark ${TRACEEVENT_LIBRARY}
>>   set_target_properties(kshark  PROPERTIES SUFFIX    
>> ".so.${KS_VERSION_STRING}")
>> -install(TARGETS kshark LIBRARY DESTINATION 
>> ${_INSTALL_PREFIX}/lib/${KS_APP_NAME})
>> +install(TARGETS kshark LIBRARY DESTINATION ${_LIBDIR}/${KS_APP_NAME})
>>   if (OPENGL_FOUND AND GLUT_FOUND)
>> @@ -29,7 +29,7 @@ if (OPENGL_FOUND AND GLUT_FOUND)
>>       set_target_properties(kshark-plot PROPERTIES  SUFFIX 
>> ".so.${KS_VERSION_STRING}")
>> -    install(TARGETS kshark-plot LIBRARY DESTINATION 
>> ${_INSTALL_PREFIX}/lib/${KS_APP_NAME})
>> +    install(TARGETS kshark-plot LIBRARY DESTINATION 
>> ${_LIBDIR}/${KS_APP_NAME})
>>   endif (OPENGL_FOUND AND GLUT_FOUND)
>> @@ -85,7 +85,7 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND)
>>       install(TARGETS ${KS_APP_NAME} kshark-record kshark-gui
>>               RUNTIME DESTINATION ${_INSTALL_PREFIX}/bin/
>> -            LIBRARY DESTINATION ${_INSTALL_PREFIX}/lib/${KS_APP_NAME}/)
>> +            LIBRARY DESTINATION ${_LIBDIR}/${KS_APP_NAME}/)
>>       install(FILES "${KS_DIR}/${KS_APP_NAME}.desktop"
>>               DESTINATION ${_INSTALL_PREFIX}/share/applications/)
>>

-- 
Zamir SUN
Fedora user
GPG : 1D86 6D4A 49CE 4BBD 72CF FCF5 D856 6E11 F2A0 525E