From: "Ludovic Courtès" <ludo@gnu.org>
To: Danny Milosavljevic <dannym@scratchpost.org>
Cc: Mark H Weaver <mhw@netris.org>, 37868@debbugs.gnu.org
Subject: [bug#37868] [PATCH v6] system: Add kernel-module-packages to operating-system.
Date: Sun, 15 Mar 2020 22:00:04 +0100 [thread overview]
Message-ID: <877dzlibaj.fsf@gnu.org> (raw)
In-Reply-To: <20200227122519.3226-1-dannym@scratchpost.org> (Danny Milosavljevic's message of "Thu, 27 Feb 2020 13:25:19 +0100")
Hi!
Danny Milosavljevic <dannym@scratchpost.org> skribis:
> * gnu/system.scm (<operating-system>): Add kernel-module-packages.
> (operating-system-directory-base-entries): Use it.
> * doc/guix.texi (operating-system Reference): Document KERNEL-LOADABLE-MODULES.
> * gnu/build/linux-modules.scm (depmod!): New procedure.
> (make-linux-module-directory): New procedure. Export it.
> * guix/profiles.scm (linux-module-database): New procedure. Export it.
> * gnu/tests/linux-modules.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> * gnu/packages/linux.scm (make-linux-libre*)[arguments]<#:phases>[install]:
> Disable depmod. Remove "build" and "source" symlinks.
[...]
> +@item @code{kernel-loadable-modules} (default: '())
> +A list of objects (usually packages) to collect loadable kernel modules from.
Perhaps you can add an example.
> +(define (input-files inputs file)
> + "Given a list of directories INPUTS, return all entries with FILE in it."
> + ;; TODO: Use filter-map.
> + (filter file-exists?
> + (map (lambda (x)
> + (string-append x file))
> + inputs)))
“Input” in Guix is usually used to describe association lists. To avoid
confusion, I propose:
(define (existing-files directories base)
"Return the absolute file name of every file named BASE under the
DIRECTORIES."
(filter-map (lambda (directory)
(let ((file (string-append directory "/" base)))
(and (file-exists? file) file)))
inputs)
> +(define (depmod! kmod inputs version destination-directory output)
There’s shouldn’t be a bang, by convention. Also please add a docstring.
> + (let ((maps-files (input-files inputs "/System.map"))
> + (symvers-files (input-files inputs "/Module.symvers")))
> + (for-each (lambda (basename)
> + (when (and (string-prefix? "modules." basename)
> + (not (string=? "modules.builtin" basename))
> + (not (string=? "modules.order" basename)))
> + (delete-file (string-append destination-directory "/"
> + basename))))
> + (scandir destination-directory))
> + (invoke (string-append kmod "/bin/depmod")
Generally, for this kind of utility function, we assume that the tool is
in $PATH, which allows us to avoid carrying its file name throughout the
API. I’d suggest doing the same here.
> +(define (make-linux-module-directory kmod inputs version output)
> + "Ensures that the directory OUTPUT...VERSION can be used by the Linux
> +kernel to load modules via KMOD. The modules to put into
> +OUTPUT are taken from INPUTS."
Perhaps be more specific as to the fact that it’s creating ‘System.maps’
etc. databases?
> (let ((locale (operating-system-locale-directory os)))
> - (mlet %store-monad ((kernel -> (operating-system-kernel os))
> - (initrd -> (operating-system-initrd-file os))
> - (params (operating-system-boot-parameters-file os)))
> + (mlet* %store-monad ((kernel -> (operating-system-kernel os))
> + (modules ->
> + (operating-system-kernel-loadable-modules os))
> + (kernel
> + ;; TODO: system, target.
> + (profile-derivation
> + (packages->manifest
> + (cons kernel modules))
> + #:hooks (list linux-module-database)
> + #:locales? #f
> + #:allow-collisions? #f
> + #:relative-symlinks? #t))
I think the system and target will be correct, but perhaps you can
double-check why doing ‘guix system build -s … -d’ and checking the
relevant .drv. :-)
I don’t think #:allow-collisions?, #:locales? and #:relative-symlinks?
are needed, so I’d recommend removing them.
> +++ b/gnu/tests/linux-modules.scm
Nice!
> +;; XXX: Dupe in gnu/build/linux-modules.scm .
> +(define (input-files inputs path)
> + "Given a list of directories INPUTS, return all entries with PATH in it."
> + ;; TODO: Use filter-map.
> + #~(begin
> + (use-modules (srfi srfi-1))
> + (filter file-exists?
> + (map (lambda (x)
> + (string-append x #$path))
> + '#$inputs))))
Same comment as above. :-)
> +(define (linux-module-database manifest)
> + "Return a derivation that unions all the kernel modules in the manifest
> +and creates the dependency graph for all these kernel modules."
Perhaps explicitly write “This is meant to be used as a profile hook.”
or similar.
> + (define build
> + (with-imported-modules (source-module-closure '((guix build utils) (gnu build linux-modules)))
80 chars please. :-)
> + #~(begin
> + (use-modules (ice-9 ftw))
> + (use-modules (ice-9 match))
> + (use-modules (srfi srfi-1)) ; append-map
> + (use-modules (guix build utils)) ; mkdir-p
> + (use-modules (gnu build linux-modules))
Please make it only one ‘use-modules’ form.
> + (let* ((inputs '#$(manifest-inputs manifest))
> + (module-directories #$(input-files (manifest-inputs manifest) "/lib/modules"))
> + (directory-entries
> + (lambda (directory-name)
> + (scandir directory-name (lambda (basename)
> + (not (string-prefix? "." basename))))))
80 chars please, and also one-word identifiers are preferred for local
variables.
> + ;; Note: Should usually result in one entry.
> + (versions (delete-duplicates
> + (append-map directory-entries
> + module-directories))))
> + ;; TODO: if len(module-directories) == 1: return module-directories[0]
> + (mkdir-p (string-append #$output "/lib"))
> + (match versions
> + ((version)
> + (make-linux-module-directory #$kmod inputs version #$output)))
> + (exit #t)))))
No need for ‘exit’, but perhaps and ‘error’ call in the unmatched case?
Thanks, and apologies for the delay!
Ludo’.
next prev parent reply other threads:[~2020-03-15 21:01 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-22 15:22 [bug#37868] [PATCH] guix: Allow multiple packages to provide Linux modules in the system profile Danny Milosavljevic
2019-11-12 16:20 ` Danny Milosavljevic
2019-11-12 17:47 ` Giovanni Biscuolo
2019-11-12 18:11 ` Giovanni Biscuolo
2019-11-13 13:30 ` Ludovic Courtès
2019-11-14 16:21 ` Danny Milosavljevic
2020-02-17 17:10 ` Danny Milosavljevic
2020-02-18 8:31 ` Ludovic Courtès
2019-11-14 17:48 ` Mark H Weaver
2020-02-18 9:42 ` [bug#37868] [PATCH v2 0/2] system: Add kernel-module-packages to operating-system and use it Danny Milosavljevic
2020-02-18 9:42 ` [bug#37868] [PATCH v2 1/2] build-system/linux-module: Disable depmod Danny Milosavljevic
2020-02-23 16:22 ` Ludovic Courtès
2020-02-25 10:11 ` Danny Milosavljevic
2020-02-18 9:42 ` [bug#37868] [PATCH v2 2/2] system: Add kernel-module-packages to operating-system Danny Milosavljevic
2020-02-18 12:31 ` Mathieu Othacehe
2020-02-23 16:36 ` Ludovic Courtès
2020-02-24 16:18 ` Danny Milosavljevic
2020-02-25 10:21 ` [bug#37868] [PATCH v3] " Danny Milosavljevic
2020-02-25 10:55 ` [bug#37868] [PATCH v4] " Danny Milosavljevic
2020-02-25 11:32 ` Danny Milosavljevic
2020-02-25 13:34 ` Danny Milosavljevic
2020-02-26 19:59 ` [bug#37868] [PATCH v5] " Danny Milosavljevic
2020-02-27 11:15 ` Danny Milosavljevic
2020-02-27 12:25 ` [bug#37868] [PATCH v6] " Danny Milosavljevic
2020-02-27 13:51 ` [bug#37868] [PATCH v7] " Danny Milosavljevic
2020-02-27 15:50 ` [bug#37868] [PATCH v8] " Danny Milosavljevic
2020-03-14 18:40 ` Danny Milosavljevic
2020-03-15 10:28 ` Mathieu Othacehe
2020-03-15 10:33 ` Mathieu Othacehe
2020-03-15 18:17 ` Danny Milosavljevic
2020-03-16 9:55 ` Mathieu Othacehe
2020-03-16 20:10 ` Danny Milosavljevic
2020-03-17 9:29 ` Ludovic Courtès
2020-03-18 14:50 ` Mathieu Othacehe
2020-03-18 16:06 ` Danny Milosavljevic
2020-03-18 17:00 ` Danny Milosavljevic
2020-03-18 17:35 ` Ludovic Courtès
2020-03-20 10:19 ` Danny Milosavljevic
2020-03-20 10:32 ` Mathieu Othacehe
2020-03-20 15:13 ` Mathieu Othacehe
2020-03-20 17:52 ` Mathieu Othacehe
2020-03-21 10:06 ` Danny Milosavljevic
2020-03-22 13:36 ` Danny Milosavljevic
2020-03-22 21:11 ` Ludovic Courtès
2020-03-15 21:02 ` Ludovic Courtès
2020-03-15 21:00 ` Ludovic Courtès [this message]
2020-03-15 22:09 ` [bug#37868] [PATCH v6] " Danny Milosavljevic
2020-03-16 8:55 ` Ludovic Courtès
2020-03-16 20:04 ` Danny Milosavljevic
2020-03-16 20:31 ` Danny Milosavljevic
2020-03-17 9:20 ` Ludovic Courtès
2020-03-16 20:17 ` [bug#37868] [PATCH v9] system: Add kernel-loadable-modules " Danny Milosavljevic
2020-03-19 14:22 ` [bug#37868] [PATCH v10] " Danny Milosavljevic
2020-03-22 12:01 ` bug#37868: " Danny Milosavljevic
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877dzlibaj.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=37868@debbugs.gnu.org \
--cc=dannym@scratchpost.org \
--cc=mhw@netris.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.