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 v2 2/2] system: Add kernel-module-packages to operating-system.
Date: Sun, 23 Feb 2020 17:36:40 +0100 [thread overview]
Message-ID: <875zfxs1k7.fsf@gnu.org> (raw)
In-Reply-To: <20200218094207.6196-3-dannym@scratchpost.org> (Danny Milosavljevic's message of "Tue, 18 Feb 2020 10:42:07 +0100")
Danny Milosavljevic <dannym@scratchpost.org> skribis:
> * gnu/system.scm (<operating-system>): Add kernel-module-packages.
> (operating-system-directory-base-entries): Use it.
> * guix/profiles.scm (linux-module-database): New procedure. Export it.
[...]
> + (kernel-module-packages operating-system-kernel-module-packages
> + (default '())) ; list of packages
Technically we don’t require them to be <package> objects, right? Any
lowerable object, like <computed-file>, would work?
Thus, I’d be tempted to remove “packages” from the field name.
‘kernel-modules’ is not a good idea because one may assume it’s a list
of .ko file names. Perhaps ‘kernel-loadable-modules’?
Could you also add an entry in guix.texi?
> + (mlet* %store-monad ((kernel -> (operating-system-kernel os))
> + (kernel-module-packages ->
> + (operating-system-kernel-module-packages os))
Please use short names for local variables; ‘modules’ is enough here.
> + (kernel*
s/kernel*/kernel/ since there’s no ambiguity.
> + (if (null? kernel-module-packages)
> + kernel
> + (profile-derivation
> + (packages->manifest
> + (cons kernel kernel-module-packages))
> + #:hooks (list linux-module-database)
> + #:locales? #f
> + #:allow-collisions? #f
> + #:relative-symlinks? #t
> + ; TODO: system, target.
> + #:system #f
> + #:target #f)))
You can omit the ‘null?’ case. Also, rather leave out #:system and
#:target so that they take their default value.
> +(define (linux-module-database manifest)
> + (mlet %store-monad
> + ((kmod (manifest-lookup-package manifest "kmod")))
Please add a docstring and make the ‘mlet’ a single line.
> + (define build
> + (with-imported-modules '((guix build utils)
> + (guix build union))
> + #~(begin
> + (use-modules (srfi srfi-1)
> + (srfi srfi-26)
> + (guix build utils)
> + (guix build union)
> + (ice-9 ftw)
> + (ice-9 match))
> + (let* ((inputs '#$(manifest-inputs manifest))
> + (input-files (lambda (path)
> + (filter file-exists?
> + (map (cut string-append <> path) inputs))))
s/path/file/ + use of ‘filter-map’
> + (module-directories (input-files "/lib/modules"))
> + (System.maps (input-files "/System.map"))
> + (Module.symverss (input-files "/Module.symvers"))
^
Typo.
Also perhaps just ‘maps-file’ and ‘symvers-file’.
> + (directory-entries (lambda (directory-name)
> + (filter (lambda (basename)
> + (not (string-prefix? "."
> + basename)))
> + (scandir directory-name))))
> + ;; Note: Should result in one entry.
> + (versions (append-map directory-entries module-directories)))
> + ;; TODO: if len(module-directories) == 1: return module-directories[0]
> + (mkdir-p (string-append #$output "/lib/modules"))
> + ;; Iterate over each kernel version directory (usually one).
> + (for-each (lambda (version)
> + (let ((destination-directory (string-append #$output "/lib/modules/" version)))
> + (when (not (file-exists? destination-directory)) ; unique
> + (union-build destination-directory
> + ;; All directories with the same version as us.
> + (filter-map (lambda (directory-name)
> + (if (member version
> + (directory-entries directory-name))
> + (string-append directory-name "/" version)
> + #f))
> + module-directories)
> + #:create-all-directories? #t)
> + ;; Delete generated files (they will be recreated shortly).
> + (for-each (lambda (basename)
> + (when (string-prefix? "modules." basename)
> + (false-if-file-not-found
> + (delete-file
> + (string-append
> + destination-directory "/"
> + basename)))))
> + (directory-entries destination-directory))
> + (unless (zero? (system* (string-append #$kmod "/bin/depmod")
> + "-e" ; Report symbols that aren't supplied
> + "-w" ; Warn on duplicates
> + "-b" #$output ; destination-directory
> + "-F" (match System.maps
> + ((x) x))
> + "-E" (match Module.symverss
> + ((x) x))
> + version))
> + (display "FAILED\n" (current-error-port))
> + (exit #f)))))
Like Mathieu wrote, I think this should be shortened and/or decomposed
in several functions, with all the effects (‘for-each’, ‘when’,
‘unless’) happening at the very end.
I wonder what’s missing form (gnu build linux-modules) to do the
“depmod” bit entirely in Scheme. It would be nice for several reasons,
one of which is that we wouldn’t need the ‘manifest-lookup-package’
hack, which in turn would allow us to keep this procedure out of (guix
profiles).
Thoughts?
Ludo’.
next prev parent reply other threads:[~2020-02-23 16:37 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 [this message]
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 ` [bug#37868] [PATCH v6] " Ludovic Courtès
2020-03-15 22:09 ` 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
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875zfxs1k7.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 public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).