GNU bug report logs - #48984
[PATCH] gnu: ddcutil: Patch kernel modules paths.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48984 in the body.
You can then email your comments to 48984 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Brice Waegeneire <brice <at> waegenei.re>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: ddcutil: Patch kernel modules paths.
Date: Sat, 12 Jun 2021 23:24:22 +0200

Without it "ddcutil detect" complain: “Module i2c-dev is not loaded and
ddcutil can't determine if it is built into the kernel”.

* gnu/packages/patches/ddcutil-modules-location.patch: New patch.
* gnu/local.mk (dist_patch_DATA): Register it.
* gnu/packages/hardware.scm (ddcutil)[source]: Apply it.
---
 gnu/local.mk                                  |  1 +
 gnu/packages/hardware.scm                     |  6 ++--
 .../patches/ddcutil-modules-location.patch    | 32 +++++++++++++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 gnu/packages/patches/ddcutil-modules-location.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 26b3e86e22..aa06cac37f 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -941,6 +941,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/dbus-CVE-2020-12049.patch		\
   %D%/packages/patches/dbus-c++-gcc-compat.patch		\
   %D%/packages/patches/dbus-c++-threading-mutex.patch		\
+  %D%/packages/patches/ddcutil-modules-location.patch		\
   %D%/packages/patches/dbxfs-remove-sentry-sdk.patch		\
   %D%/packages/patches/debops-constants-for-external-program-names.patch \
   %D%/packages/patches/debops-debops-defaults-fall-back-to-less.patch \
diff --git a/gnu/packages/hardware.scm b/gnu/packages/hardware.scm
index ecbcca79b1..0612e4a068 100644
--- a/gnu/packages/hardware.scm
+++ b/gnu/packages/hardware.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2018–2021 Tobias Geerinckx-Rice <me <at> tobias.gr>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
-;;; Copyright © 2020 Brice Waegeneire <brice <at> waegenei.re>
+;;; Copyright © 2020, 2021 Brice Waegeneire <brice <at> waegenei.re>
 ;;; Copyright © 2021 Evgeny Pisemsky <evgeny <at> pisemsky.com>
 ;;; Copyright © 2021 Léo Le Bouter <lle-bout <at> zaclys.net>
 ;;;
@@ -21,6 +21,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu packages hardware)
+  #:use-module (gnu packages)
   #:use-module (gnu packages admin)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages bash)
@@ -68,7 +69,8 @@
        (uri (string-append "https://www.ddcutil.com/tarballs/"
                            "ddcutil-" version ".tar.gz"))
        (sha256
-        (base32 "19kkwb9ijzn6ya3mvjanggh1c96fcc0lkbk7xnyi2qp6wsr4nhxp"))))
+        (base32 "19kkwb9ijzn6ya3mvjanggh1c96fcc0lkbk7xnyi2qp6wsr4nhxp"))
+       (patches (search-patches "ddcutil-modules-location.patch"))))
     (build-system gnu-build-system)
     (native-inputs
      `(("pkg-config" ,pkg-config)))
diff --git a/gnu/packages/patches/ddcutil-modules-location.patch b/gnu/packages/patches/ddcutil-modules-location.patch
new file mode 100644
index 0000000000..c36fe5f2d7
--- /dev/null
+++ b/gnu/packages/patches/ddcutil-modules-location.patch
@@ -0,0 +1,32 @@
+From https://github.com/NixOS/nixpkgs/blob/nixos-21.05/pkgs/tools/misc/ddcutil/nixos-paths.diff
+
+--- a/src/util/linux_util.c
++++ b/src/util/linux_util.c
+@@ -125,6 +125,7 @@
+                   "lib64",
+                   "lib32",
+                   "usr/lib",  // needed for arch?
++                  "run/booted-system/kernel/lib",  // Guix System
+                   NULL};
+    int result = -1;
+    int ndx = 0;
+@@ -204,14 +205,15 @@
+    if (debug)
+       printf("(%s) machine: %s", __func__, utsbuf.machine);
+
+-   char * libdirs[3];
++   char * libdirs[4];
+    libdirs[0] = "lib";
++   libdirs[1] = "run/booted-system/kernel/lib";
+    if (streq(utsbuf.machine, "amd_64")){
+-      libdirs[1] = "lib64";
+-      libdirs[2] = NULL;
++      libdirs[2] = "lib64";
++      libdirs[3] = NULL;
+    }
+    else
+-      libdirs[1] = NULL;
++      libdirs[2] = NULL;
+
+    int libsndx = 0;
+    bool found = false;
-- 
2.31.1

Message #8 received at 48984 <at> debbugs.gnu.org (full text, mbox):

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Brice Waegeneire <brice <at> waegenei.re>
Cc: guix-patches <at> gnu.org, 48984 <at> debbugs.gnu.org
Subject: Re: [bug#48984] [PATCH] gnu: ddcutil: Patch kernel modules paths.
Date: Sun, 13 Jun 2021 12:35:30 +0200

[Message part 1 (text/plain, inline)]

Brice,

Thanks for the patch!  Does it affect more than ‘ddcutil 
detect/environment’?  Anything ‘real’?

Working around one of many bugs in a broken diagnostic subtool 
isn't worth maintaining a 32-line patch.

Brice Waegeneire 写道：
> Without it "ddcutil detect" complain: “Module i2c-dev is not 
> loaded and
> ddcutil can't determine if it is built into the kernel”.

With it, it still complains:

 ~ λ ddcutil detect
 Module i2c-dev is not loaded and not built into the kernel.
 ddcutil requires module i2c-dev

It doesn't address the built-in case either, AFAICS.

The very idea of ddcutil's linux_util.c is flawed and can't be 
fixed: you don't look in /etc for the running kernel version; you 
don't use that ill-gotten string to start scanning random system 
directories for .ko files; you don't check for built-in modules by 
looking for possibly installed build leftovers.  None of that 
makes sense.  You ask kmod.  You ask the kernel!

Kind regards,

T G-R

[signature.asc (application/pgp-signature, inline)]

Message #14 received at 48984 <at> debbugs.gnu.org (full text, mbox):

From: Brice Waegeneire <brice <at> waegenei.re>
To: Tobias Geerinckx-Rice <me <at> tobias.gr>
Cc: 48984 <at> debbugs.gnu.org
Subject: Re: bug#48984: [PATCH] gnu: ddcutil: Patch kernel modules paths.
Date: Sun, 13 Jun 2021 15:07:01 +0200

Tobias,

Thank you for the review!

Tobias Geerinckx-Rice <me <at> tobias.gr> writes:
> Thanks for the patch!  Does it affect more than ‘ddcutil
> detect/environment’?  Anything ‘real’?

AFAIK no, this patch only get rid off that error message.

> Working around one of many bugs in a broken diagnostic subtool isn't worth
> maintaining a 32-line patch.

Fair enough, it's just a stop gap until 1.1.1 is released tho.

> Brice Waegeneire 写道：
>> Without it "ddcutil detect" complain: “Module i2c-dev is not loaded and
>> ddcutil can't determine if it is built into the kernel”.
>
> With it, it still complains:
>
>  ~ λ ddcutil detect
>  Module i2c-dev is not loaded and not built into the kernel.
>  ddcutil requires module i2c-dev

It's not the same error.

> It doesn't address the built-in case either, AFAICS.

I wasn't clear enough in the git message.  Here is the output wihtout
that patch and with it:

--8<---------------cut here---------------start------------->8---
$ modinfo i2c-dev | grep '^filename:'
filename:       (builtin)

$ guix environment --ad-hoc ddcutil -- ddcutil detect
Unable to read modules.builtin
Module i2c-dev is not loaded and ddcutil can't determine if it is built into the kernel
Display 1
[...]
$ ./pre-inst-env guix environment --ad-hoc ddcutil -- ddcutil detect
Display 1
[...]
--8<---------------cut here---------------end--------------->8--

With a built-in i2c-dev module, this patch remove that pesky error
message.

> The very idea of ddcutil's linux_util.c is flawed and can't be fixed: you
> don't look in /etc for the running kernel version; you don't use that
> ill-gotten string to start scanning random system directories for .ko
> files; you don't check for built-in modules by looking for possibly
> installed build leftovers.  None of that makes sense.  You ask kmod.  You
> ask the kernel!

I agreed it is flawed, but it was fixed by upstream in the 1.1.1-dev
branch¹ by using libkmod as you are suggesting.  I proposed that solution
to upstream some days ago in inssue #178²; he implementeded right away.

Since it's a development branch I tought it wasn't wise to update the
package to the latest commit.  Maybe you would you prefer such patch
instead?

¹ https://github.com/rockowitz/ddcutil/tree/1.1.1-dev
² https://github.com/rockowitz/ddcutil/issues/178#issuecomment-856297112

Cheers,
- Brice

Message #19 received at 48984-close <at> debbugs.gnu.org (full text, mbox):

From: Brice Waegeneire <brice <at> waegenei.re>
To: 48984-close <at> debbugs.gnu.org
Subject: Re: bug#48984: [PATCH] gnu: ddcutil: Patch kernel modules paths.
Date: Sat, 22 Jan 2022 00:21:05 +0100

Brice Waegeneire <brice <at> waegenei.re> writes:

> * gnu/packages/patches/ddcutil-modules-location.patch: New patch.
> * gnu/local.mk (dist_patch_DATA): Register it.
> * gnu/packages/hardware.scm (ddcutil)[source]: Apply it.

Upstream released a new version. This patch is superseded by <https://bugs.gnu.org/48984>.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.