Re: [PATCH 1/2] KernelShark: Inherit libdir from Makefile

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 17.02.20 г. 16:11 ч., Yordan Karadzhov (VMware) wrote:
include(${KS_DIR}/build/FindTraceCmd.cmake)
  include(${KS_DIR}/build/FindJSONC.cmake)
@@ -34,8 +38,8 @@ if (Qt5Widgets_FOUND)
  endif (Qt5Widgets_FOUND)
-set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/lib")
-set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/bin")
+set(LIBRARY_OUTPUT_PATH    "${KS_DIR}/${_LIBDIR}")
+set(EXECUTABLE_OUTPUT_PATH "${KS_DIR}/${_INSTALL_PREFIX}/bin")

Not sure what is your idea here, but this looks wrong to me. When you are building from source (just typing "make") you don't expect the object files and the executables to be placed outside the trunk of the repository(${KS_DIR}). Do not confuse building the source with installing (when typing "make install").

In the beginning this was meant to make sure the debuginfo package is generated within corresponding lib64 directory. However I just compiled again and find it's not needed now.


  set(CMAKE_C_FLAGS   "${CMAKE_C_FLAGS} -Wall -pthread -fPIC")
  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -std=c++11 -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}/")

Please stick to lower-case characters with all CMake commands.


Ah sorry, this is definitely a typo when converting to capital letters in bulks.

Thanks for the review. As I don't know the background beforehand, if you think part of this patch still makes sense, I can make v2 and drop the bits you don't need yet.


Hi Ziqian,

I had a conversation with Steven about the problem and he suggested another solution how to make sure that the users know the libraries are not ready to be used directly yet. Just send new version of the patch with the small changes I asked for. Do not change the README file. I will implement what Steven suggested on top of your patch.

Thanks!
Yordan



[Index of Archives]     [Linux USB Development]     [Linux USB Development]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux