| Message ID | 20210612212422.13271-1-brice@waegenei.re |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#48984] gnu: ddcutil: Patch kernel modules paths. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
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
Tobias, Thank you for the review! Tobias Geerinckx-Rice <me@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
Brice Waegeneire <brice@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>.
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;