[PATCH] gnu: ddcutil: Patch kernel modules paths.

  • Done
  • quality assurance status badge
Details
2 participants
  • Brice Waegeneire
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
Brice Waegeneire
Severity
normal
B
B
Brice Waegeneire wrote on 12 Jun 2021 23:24
(address . guix-patches@gnu.org)
20210612212422.13271-1-brice@waegenei.re
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

Toggle diff (83 lines)
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@tobias.gr>
;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
-;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re>
+;;; Copyright © 2020, 2021 Brice Waegeneire <brice@waegenei.re>
;;; Copyright © 2021 Evgeny Pisemsky <evgeny@pisemsky.com>
;;; Copyright © 2021 Léo Le Bouter <lle-bout@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
T
T
Tobias Geerinckx-Rice wrote on 13 Jun 2021 12:35
(name . Brice Waegeneire)(address . brice@waegenei.re)
87tum2xdal.fsf@nckx
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 ???
Toggle quote (4 lines)
> 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
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYMXfcg0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW154yUA+gIzNLf8OQaN0K4cDWZG4lKvGh08oUm8NzhPuSoS
Zp9XAP9ofj60PgwubFgHss4bcQnA0UYxEVIIUxs2BswdLh7rBg==
=mmOo
-----END PGP SIGNATURE-----

B
B
Brice Waegeneire wrote on 13 Jun 2021 15:07
Re: bug#48984: [PATCH] gnu: ddcutil: Patch kernel modules paths.
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 48984@debbugs.gnu.org)
871r95gbgq.fsf_-_@waegenei.re
Tobias,

Thank you for the review!

Tobias Geerinckx-Rice <me@tobias.gr> writes:
Toggle quote (3 lines)
> 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.

Toggle quote (3 lines)
> 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.

Toggle quote (10 lines)
> 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.

Toggle quote (2 lines)
> 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:

Toggle snippet (13 lines)
$ 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
[...]

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

Toggle quote (7 lines)
> 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?


Cheers,
- Brice
B
B
Brice Waegeneire wrote on 22 Jan 2022 00:21
(address . 48984-close@debbugs.gnu.org)
87o844smji.fsf@waegenei.re
Brice Waegeneire <brice@waegenei.re> writes:

Toggle quote (4 lines)
> * 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.
?