Subject: Re: [bug#75159] [PATCH v1 3/5] gnu: Add corectrl.

From: Zheng Junjie <[email-masked]>
Date: Mon, 30 Dec 2024 14:50:00 +0800

Ashvith Shetty <[email-masked]> writes:

> * gnu/packages/pciutils.scm (corectrl): New variable.
> * gnu/packages/patches/corectrl-polkit-install-dir.patch:
> New file.
>
> Change-Id: I726c6cbfb6530abd8284f36e10a1e1f7a0fc4661
> ---
>  .../patches/corectrl-polkit-install-dir.patch | 21 +++++
>  gnu/packages/pciutils.scm                     | 83 ++++++++++++++++++-
>  2 files changed, 103 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/corectrl-polkit-install-dir.patch
>
> diff --git a/gnu/packages/patches/corectrl-polkit-install-dir.patch b/gnu/packages/patches/corectrl-polkit-install-dir.patch

you forget add this patch to gnu/local.mk(dist_patch_DATA).

You can also mention a pr upstream that allows custom
POLKIT_POLICY_INSTALL_DIR.

> new file mode 100644
> index 0000000000..586e096cf4
> --- /dev/null
> +++ b/gnu/packages/patches/corectrl-polkit-install-dir.patch
> @@ -0,0 +1,21 @@
> +diff --git a/src/helper/CMakeLists.txt b/src/helper/CMakeLists.txt
> +index c262086..523ea97 100644
> +--- a/src/helper/CMakeLists.txt
> ++++ b/src/helper/CMakeLists.txt
> +@@ -26,15 +26,7 @@ message("D-Bus files will be installed into ${DBUS_DATADIR_PREFIX_DIR}/dbus-1")
> + 
> + # Find polkit
> + pkg_check_modules(POLKIT REQUIRED polkit-gobject-1)
> +-execute_process(
> +-  COMMAND pkg-config --variable=policydir polkit-gobject-1
> +-  RESULT_VARIABLE POLKIT_POLICY_INSTALL_DIR_RESULT
> +-  OUTPUT_VARIABLE POLKIT_POLICY_INSTALL_DIR
> +-  OUTPUT_STRIP_TRAILING_WHITESPACE
> +-)
> +-if(NOT POLKIT_POLICY_INSTALL_DIR_RESULT EQUAL "0")
> +-  message(FATAL_ERROR "Failed to retrieve Polkit `policydir` variable using pkg-config")
> +-endif()
> ++option(POLKIT_POLICY_INSTALL_DIR "Polkit policy directory")
> + 
> + list(APPEND HELPER_COMPILE_DEFINITIONS
> +   SPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_TRACE
> diff --git a/gnu/packages/pciutils.scm b/gnu/packages/pciutils.scm
> index 08f1634113..1ae568a8eb 100644
> --- a/gnu/packages/pciutils.scm
> +++ b/gnu/packages/pciutils.scm
> @@ -6,6 +6,7 @@
>  ;;; Copyright © 2020, 2023, 2024 Janneke Nieuwenhuizen <[email-masked]>
>  ;;; Copyright © 2022 Brendan Tildesley <[email-masked]>
>  ;;; Copyright © 2022 Marius Bakke <[email-masked]>
> +;;; Copyright © 2024 Ashvith Shetty <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -30,11 +31,23 @@ (define-module (gnu packages pciutils)
>    #:use-module (guix utils)
>    #:use-module (guix git-download)
>    #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system qt)
>    #:use-module (gnu packages)
> +  #:use-module (gnu packages base)
> +  #:use-module (gnu packages check)
>    #:use-module (gnu packages compression)
> +  #:use-module (gnu packages crypto)
> +  #:use-module (gnu packages gcc)
> +  #:use-module (gnu packages gl)
> +  #:use-module (gnu packages logging)
> +  #:use-module (gnu packages maths)
> +  #:use-module (gnu packages polkit)
>    #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages glib)
>    #:use-module (gnu packages linux)
> -  #:use-module (gnu packages base))
> +  #:use-module (gnu packages qt)
> +  #:use-module (gnu packages vulkan)
> +  #:use-module (gnu packages xml))
>  
>  (define-public hwdata
>    (package
> @@ -173,3 +186,71 @@ (define-public pciutils
>  of operating systems.  This includes the @command{lspci} and @command{setpci}
>  commands.")
>      (license license:gpl2+)))
> +
> +;; corectrl also requires the helper service.
> +;; See corectrl-helper-service-type.
> +;;
> +;; Please read the setup instructions from:
> +;; https://gitlab.com/corectrl/corectrl/-/wikis/Installation
> +;; and
> +;; https://gitlab.com/corectrl/corectrl/-/wikis/Setup
> +;;
> +;; corectrl also requires CONFIG_NETLINK_DIAG to be set to
> +;; either 'y' or 'm' - 'm' is the default in Guix. In that case, 
> +;; the kernel module has to be loaded.
> +;;
> +;; To allow rootless corectrl, add user to the "corectrl" group.
> +
> +(define-public corectrl

i think this package should go to gnu/packages/admin.scm, because this
package is "control with ease your computer hardware using application
profiles".

> +  (package
> +    (name "corectrl")
> +    (version "1.4.3")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://gitlab.com/corectrl/corectrl")
> +             (commit (string-append "v" version))))
> +       (sha256
> +        (base32 "0qpc04xxzv4jbqqlraqriipix4ph7bm1hfiry807jjp668i9n25d"))
> +       (file-name (git-file-name name version))
> +       (patches (search-patches "corectrl-polkit-install-dir.patch"))))
> +    (build-system qt-build-system)
> +   (arguments
> +     (list
> +      #:qtbase qtbase-5
> +      #:tests? #t

please remove it, tests should not be forced.

> +      #:configure-flags #~(list "-DINSTALL_DBUS_FILES_IN_PREFIX=true"
> +                                (string-append "-DPOLKIT_POLICY_INSTALL_DIR="
> +                                                #$output
> +                                               "/share/polkit-1/actions")
> +                                (string-append "-DWITH_PCI_IDS_PATH="
> +                                               (search-input-file
> +                                                %build-inputs
> +                                                "/share/hwdata/pci.ids")))))

please not use %build-inputs. It's an outdated thing.

use
(string-append "-DWITH_PCI_IDS_PATH=" #$(this-package-input "hwdata")
"/share/hwdata/pci.ids")

> +    (native-inputs (list pkg-config gcc-13))

please add comments for why need gcc-13.

> +    (propagated-inputs (list mesa-utils vulkan-tools util-linux))

why propagated them? It's easy to conflict in profiles, maybe should use wrap-program.

> +    (inputs (list dbus
> +                  botan
> +                  catch2-3
> +                  hwdata
> +                  polkit
> +                  procps
> +                  pugixml
> +                  qtcharts-5
> +                  qtdeclarative-5
> +                  qtquickcontrols2-5
> +                  qtsvg-5
> +                  qttools-5

qttools-5 should go to native-inputs.

> +                  quazip
> +                  spdlog
> +                  trompeloeil
> +                  units
> +                  zlib))
> +    (home-page "https://gitlab.com/corectrl/corectrl")
> +    (synopsis "Profile based system control utility")
> +    (description
> +     "CoreCtrl is a Free and Open Source GNU/Linux application that allows 
> +you to control with ease your computer hardware using application profiles. 
> +It aims to be flexible, comfortable and accessible to regular users.")
> +    (license (list license:gpl3))))