[PATCH 0/4] kernel-shark: CMake changes

[PATCH 0/4] kernel-shark: CMake changes

[Michal Sojka]

This patch series is a follow up to my previous patch for the older
kernel-shark repo. The first two patches will make life easier for
distributions like NixOS, the last two patches replace my previous
patch.

Note that I didn't add kshark-su-record to a separate component, as
suggested by Yordan, because it is not necessary. The generated polkit
action will refer to the correct path and I think it is preferable to
have all kernelshark binaries in the same directory.

Michal Sojka (4):
  kernel-shark: Allow specifying PKG_CONFIG_DIR on cmake command line
  kernel-shark: Allow specifying TT_FONT_FILE on cmake command line
  kernel-shark: Allow installing polkit policy separately
  kernel-shark: Do not use sudo in install_gui.sh + update README

 CMakeLists.txt       | 12 ++++++++++--
 README               |  2 +-
 build/install_gui.sh | 10 +++++++++-
 src/CMakeLists.txt   | 10 ++++++----
 4 files changed, 26 insertions(+), 8 deletions(-)

-- 
2.30.1

[PATCH 1/4] kernel-shark: Allow specifying PKG_CONFIG_DIR on cmake command line

[Michal Sojka]

On some distributions (e.g. NixOS), it is not possible to write files
to directories owned by different packages. For this reason, we cannot
always install libkshark.pc file to the directory reported by
pkg-config.

This commit allows to specify where to install the .pc file on cmake
commandline via -DPKG_CONGIG_DIR=... When specified, automatic
detection of .pc install directory is skipped.

Signed-off-by: Michal Sojka <michal.sojka@cvut.cz>
---
 src/CMakeLists.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 1e86e9c..b81d7d9 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -134,8 +134,10 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND AND TT_FONT_FILE)
             DESTINATION ${_INSTALL_PREFIX}/bin/
                 COMPONENT                 kernelshark)
 
-    execute_process(COMMAND  bash "-c" "pkg-config --variable pc_path pkg-config | cut -f 1 -d: -z"
-                    OUTPUT_VARIABLE PKG_CONGIG_DIR)
+    if (NOT PKG_CONGIG_DIR)
+        execute_process(COMMAND  bash "-c" "pkg-config --variable pc_path pkg-config | cut -f 1 -d: -z"
+          OUTPUT_VARIABLE PKG_CONGIG_DIR)
+    endif (NOT PKG_CONGIG_DIR)
     install(FILES "${KS_DIR}/libkshark.pc"
             DESTINATION ${PKG_CONGIG_DIR}
                 COMPONENT                 libkshark-devel)
-- 
2.30.1

[PATCH 2/4] kernel-shark: Allow specifying TT_FONT_FILE on cmake command line

[Michal Sojka]

On distributions like NixOS, which build packages in a sandbox,
fontconfig configuration files may not be available at build time,
which leads to the following cmake warnings even if the font itself is
available:

    Fontconfig error: Cannot load default config file
    CMake Warning at CMakeLists.txt:62 (message):

      Could not find font FreeSans!

This commit allows to specify exact font location on cmake commandline
via -DTT_FONT_FILE=... When specified, automatic font look up via
fc-list is skipped.

Signed-off-by: Michal Sojka <michal.sojka@cvut.cz>
---
 CMakeLists.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index efcccb1..c4731c1 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -50,8 +50,10 @@ find_package(OpenGL)
 find_package(GLUT)
 
 set(KS_FONT FreeSans)
-execute_process(COMMAND  bash "-c" "fc-list '${KS_FONT}' |grep ${KS_FONT}.ttf | cut -d':' -f 1 -z"
-                OUTPUT_VARIABLE TT_FONT_FILE)
+if (NOT TT_FONT_FILE)
+    execute_process(COMMAND  bash "-c" "fc-list '${KS_FONT}' |grep ${KS_FONT}.ttf | cut -d':' -f 1 -z"
+                    OUTPUT_VARIABLE TT_FONT_FILE)
+endif (NOT TT_FONT_FILE)
 
 if (TT_FONT_FILE)
 
-- 
2.30.1

[PATCH 3/4] kernel-shark: Allow installing polkit policy separately

[Michal Sojka]

For a polkit policy to be found by polkit daemon, it must be installed
to a single system-wide location, typically /usr/share/polkit-1/actions.
CMakeLists file reflects that and always installs the policy under /usr.
But when one wants to install kernel-shark to a non-standard location,
e.g., by configuring it as follows:

    cmake -D_INSTALL_PREFIX=$HOME ...

then "make install" fails, with the following error:

    CMake Error at src/cmake_install.cmake:225 (file):
      file INSTALL cannot copy file
      "/home/user/src/trace-cmd/kernel-shark/org.freedesktop.kshark-record.policy"
      to "/usr/share/polkit-1/actions/org.freedesktop.kshark-record.policy".

This commit fixes that by changing two things:
- custom location where to install polkit policy can be specified via
  cmake command line argument -D_POLKIT_INSTALL_PREFIX=... This will
  also help distributions where polkit is configured with different
  prefix than /usr.
- polkit policy is now a separate cmake component, which can be
  installed separately from the rest.

The later allows generating better messages for the user to understand
that the failed polkit policy installation is not a critical error.

Signed-off-by: Michal Sojka <michal.sojka@cvut.cz>
---
 CMakeLists.txt       |  6 ++++++
 build/install_gui.sh | 10 +++++++++-
 src/CMakeLists.txt   |  4 ++--
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c4731c1..94023a8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -36,6 +36,12 @@ elseif (NOT _LIBDIR)
 
 endif ()
 
+if (NOT _POLKIT_INSTALL_PREFIX)
+
+    set(_POLKIT_INSTALL_PREFIX "/usr")
+
+endif ()
+
 set(CMAKE_MODULE_PATH "${KS_DIR}/build")
 find_package(TraceEvent REQUIRED)
 find_package(TraceFS    REQUIRED)
diff --git a/build/install_gui.sh b/build/install_gui.sh
index 1583fb9..d262f79 100755
--- a/build/install_gui.sh
+++ b/build/install_gui.sh
@@ -1 +1,9 @@
-sudo cmake -DCOMPONENT=kernelshark -P cmake_install.cmake
+if sudo cmake -DCOMPONENT=kernelshark -P cmake_install.cmake; then
+    echo "Kernelshark installed correctly"
+else
+    exit 1
+fi
+
+if ! sudo cmake -DCOMPONENT=polkit-policy -P cmake_install.cmake; then
+    echo >&2 "Warning: polkit policy not installed"
+fi
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b81d7d9..b557eb7 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -127,8 +127,8 @@ if (Qt5Widgets_FOUND AND Qt5Network_FOUND AND TT_FONT_FILE)
                 COMPONENT                 kernelshark)
 
     install(FILES "${KS_DIR}/org.freedesktop.kshark-record.policy"
-            DESTINATION /usr/share/polkit-1/actions/
-                COMPONENT                 kernelshark)
+            DESTINATION ${_POLKIT_INSTALL_PREFIX}/share/polkit-1/actions/
+                COMPONENT                 polkit-policy)
 
     install(PROGRAMS "${KS_DIR}/bin/kshark-su-record"
             DESTINATION ${_INSTALL_PREFIX}/bin/
-- 
2.30.1

[PATCH 4/4] kernel-shark: Do not use sudo in install_gui.sh + update README

[Michal Sojka]

Using `make install` to install kernelshark can lead to errors
described in the previous commit. Therefore, we instruct users to use
the provided script install_gui.sh. We update the script not to use
sudo, because it's preferable if users give root privileges explicitly
via command line.

Signed-off-by: Michal Sojka <michal.sojka@cvut.cz>
---
 README               | 2 +-
 build/install_gui.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/README b/README
index a7e66df..f5035f9 100644
--- a/README
+++ b/README
@@ -51,7 +51,7 @@ Building:
     cd kernel-shark/build
     cmake ../
     make
-    sudo make install
+    sudo ./install_gui.sh
 
 2.1 In order to create a Doxygen documentation add -D_DOXYGEN_DOC=1
 as a CMake Command-Line option.
diff --git a/build/install_gui.sh b/build/install_gui.sh
index d262f79..c42f4da 100755
--- a/build/install_gui.sh
+++ b/build/install_gui.sh
@@ -1,9 +1,9 @@
-if sudo cmake -DCOMPONENT=kernelshark -P cmake_install.cmake; then
+if cmake -DCOMPONENT=kernelshark -P cmake_install.cmake; then
     echo "Kernelshark installed correctly"
 else
     exit 1
 fi
 
-if ! sudo cmake -DCOMPONENT=polkit-policy -P cmake_install.cmake; then
+if ! cmake -DCOMPONENT=polkit-policy -P cmake_install.cmake; then
     echo >&2 "Warning: polkit policy not installed"
 fi
-- 
2.30.1

Re: [PATCH 0/4] kernel-shark: CMake changes

[Yordan Karadzhov (VMware)]

On 17.03.21 г. 18:40, Michal Sojka wrote:
> This patch series is a follow up to my previous patch for the older
> kernel-shark repo. The first two patches will make life easier for
> distributions like NixOS, the last two patches replace my previous
> patch.
> 
> Note that I didn't add kshark-su-record to a separate component, as
> suggested by Yordan, because it is not necessary. The generated polkit
> action will refer to the correct path and I think it is preferable to
> have all kernelshark binaries in the same directory.
> 
> Michal Sojka (4):
>    kernel-shark: Allow specifying PKG_CONFIG_DIR on cmake command line
>    kernel-shark: Allow specifying TT_FONT_FILE on cmake command line
>    kernel-shark: Allow installing polkit policy separately
>    kernel-shark: Do not use sudo in install_gui.sh + update README

Hi Michal,

Thank you very much for helping us improving KernelShark!
I'm applying all 4 patches.

Best,
Yordan


> 
>   CMakeLists.txt       | 12 ++++++++++--
>   README               |  2 +-
>   build/install_gui.sh | 10 +++++++++-
>   src/CMakeLists.txt   | 10 ++++++----
>   4 files changed, 26 insertions(+), 8 deletions(-)
>