diff mbox series

[bug#41924] profiles: Make linux-module-database skip inappropriate inputs

Message ID 169811592419302@mail.yandex.ru
State Accepted
Headers show
Series [bug#41924] profiles: Make linux-module-database skip inappropriate inputs | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job

Commit Message

Ivan Kozlov June 17, 2020, 6:44 p.m. UTC 
This allows a Linux package with CONFIG_MODULES=n, that doesn’t contain the ‘lib/modules’ directory, to be used.

* guix/profiles.scm (linux-module-database): Add if clause to ignore unrelated inputs. Allow empty result.
---
 guix/profiles.scm | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Ivan Kozlov June 17, 2020, 9:18 p.m. UTC | #1 
mkdir should of course be changed to mkdir-p.

17.06.2020, 21:45, "Ivan Kozlov" <kanichos@yandex.ru>:
> This allows a Linux package with CONFIG_MODULES=n, that doesn’t contain the ‘lib/modules’ directory, to be used.
>
> * guix/profiles.scm (linux-module-database): Add if clause to ignore unrelated inputs. Allow empty result.
> ---
>  guix/profiles.scm | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/guix/profiles.scm b/guix/profiles.scm
> index 25ff146bdf..a3868e8343 100644
> --- a/guix/profiles.scm
> +++ b/guix/profiles.scm
> @@ -1220,9 +1220,11 @@ This is meant to be used as a profile hook."
>                         inputs))
>                   (directory-entries
>                    (lambda (directory)
> - (scandir directory (lambda (basename)
> - (not
> - (string-prefix? "." basename))))))
> + (if (file-exists? directory)
> + (scandir directory (lambda (basename)
> + (not
> + (string-prefix? "." basename))))
> + '())))
>                   ;; Note: Should usually result in one entry.
>                   (versions (delete-duplicates
>                              (append-map directory-entries
> @@ -1233,6 +1235,8 @@ This is meant to be used as a profile hook."
>                    (setenv "PATH" #+(file-append kmod "/bin"))
>                    (make-linux-module-directory inputs version #$output)
>                    (setenv "PATH" old-path)))
> + ;; Do nothing when there is nothing to do
> + (() (mkdir #$output))
>                 (_ (error "Specified Linux kernel and Linux kernel modules
>  are not all of the same version")))))))
>    (gexp->derivation "linux-module-database" build
> --
> 2.26.2
Danny Milosavljevic June 18, 2020, 6:16 p.m. UTC | #2 
Hi Ivan,

thanks for the patch, and for persevering on this.

Could you also add a system test to gnu/tests/linux-modules.scm ?

That would also mean that you'd create a variant of linux-libre that has
CONFIG_MODULES=n.

>                   (directory-entries
>                    (lambda (directory)
> -                    (scandir directory (lambda (basename)
> -                                         (not
> -                                           (string-prefix? "." basename))))))
> +                    (if (file-exists? directory)
> +                        (scandir directory (lambda (basename)
> +                                             (not
> +                                              (string-prefix? "." basename))))
> +                        '())))

That would silently skip packages that don't contain kernel modules (but for
example supertux or something), too, right?
(so misconfigured guix wouldn't be detected)

Also, if you tried to use a kernel without module support and then added modules
(for the wrong kernel, one with CONFIG_MODULES=y) via extra guix packages,
this would erroneously succeed, right?
(I guess depmod would have something against it, though.  Still, I'd like a test
for that)

Usually I'd be fine with these unsupported constellations, but misconfigured
kernel vs module ABI makes me reconsider my life choices, so probably not here :->

What I would do is try to find a file that's only in the kernel when there's
module support (/lib/modules doesn't count since there could be a kernel with
module support but no modules compiled), and then try to ascertain whether
CONFIG_MODULES=y that way, so
(define config-modules? (any (append-map directory-entries with marker-file)))
or something.

Then succeed either with config-modules? and at least one file per entry, or
with (not config-modules?) and no files anywhere.

Alternatively, we could at least make it only possible for the FIRST entry
in the list of module packages (which is the Linux kernel) to have no
/lib/modules, and not care about any of the other entries in that way.

> +               ;; Do nothing when there is nothing to do
> +               (() (mkdir #$output))

Good idea.
Ludovic Courtès June 19, 2020, 7:47 a.m. UTC | #3 
Hi Ivan & Danny,

I forgot to email you back (sorry about that!): I pushed a variant of
this patch here:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=05f79da93fb4fd5216feb41510bf0a41f8eedf5b

WDYT?

Ludo’.
Ivan Kozlov June 19, 2020, 2:23 p.m. UTC | #4 
Hi,

Thanks, I’m happy enough with this.

19.06.2020, 10:47, "Ludovic Courtès" <ludo@gnu.org>:
> Hi Ivan & Danny,
>
> I forgot to email you back (sorry about that!): I pushed a variant of
> this patch here:
>
>   https://git.savannah.gnu.org/cgit/guix.git/commit/?id=05f79da93fb4fd5216feb41510bf0a41f8eedf5b
>
> WDYT?
>
> Ludo’.
Ludovic Courtès June 19, 2020, 8:27 p.m. UTC | #5 
Hi,

Ivan Kozlov <kanichos@yandex.ru> skribis:

> Thanks, I’m happy enough with this.

Cool, closing.  Thank you.

Ludo’.
diff mbox series

Patch

diff --git a/guix/profiles.scm b/guix/profiles.scm
index 25ff146bdf..a3868e8343 100644
--- a/guix/profiles.scm
+++ b/guix/profiles.scm
@@ -1220,9 +1220,11 @@  This is meant to be used as a profile hook."
                        inputs))
                  (directory-entries
                   (lambda (directory)
-                    (scandir directory (lambda (basename)
-                                         (not
-                                           (string-prefix? "." basename))))))
+                    (if (file-exists? directory)
+                        (scandir directory (lambda (basename)
+                                             (not
+                                              (string-prefix? "." basename))))
+                        '())))
                  ;; Note: Should usually result in one entry.
                  (versions (delete-duplicates
                             (append-map directory-entries
@@ -1233,6 +1235,8 @@  This is meant to be used as a profile hook."
                   (setenv "PATH" #+(file-append kmod "/bin"))
                   (make-linux-module-directory inputs version #$output)
                   (setenv "PATH" old-path)))
+               ;; Do nothing when there is nothing to do
+               (() (mkdir #$output))
                (_ (error "Specified Linux kernel and Linux kernel modules
 are not all of the same version")))))))
   (gexp->derivation "linux-module-database" build