From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:48488) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jDaNn-0003Tq-S4 for guix-patches@gnu.org; Sun, 15 Mar 2020 17:01:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jDaNd-0003ta-Co for guix-patches@gnu.org; Sun, 15 Mar 2020 17:01:10 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:57948) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1jDaNa-0003sC-4H for guix-patches@gnu.org; Sun, 15 Mar 2020 17:01:04 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jDaNZ-0008R9-W0 for guix-patches@gnu.org; Sun, 15 Mar 2020 17:01:02 -0400 Subject: [bug#37868] [PATCH v6] system: Add kernel-module-packages to operating-system. Resent-Message-ID: From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20200226195929.3615-1-dannym@scratchpost.org> <20200227122519.3226-1-dannym@scratchpost.org> Date: Sun, 15 Mar 2020 22:00:04 +0100 In-Reply-To: <20200227122519.3226-1-dannym@scratchpost.org> (Danny Milosavljevic's message of "Thu, 27 Feb 2020 13:25:19 +0100") Message-ID: <877dzlibaj.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Danny Milosavljevic Cc: Mark H Weaver , 37868@debbugs.gnu.org Hi! Danny Milosavljevic skribis: > * gnu/system.scm (): Add kernel-module-packages. > (operating-system-directory-base-entries): Use it. > * doc/guix.texi (operating-system Reference): Document KERNEL-LOADABLE-MO= DULES. > * 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 i= t." > + ;; TODO: Use filter-map. > + (filter file-exists? > + (map (lambda (x) > + (string-append x file)) > + inputs))) =E2=80=9CInput=E2=80=9D in Guix is usually used to describe association lis= ts. 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=E2=80=99s shouldn=E2=80=99t be a bang, by convention. Also please ad= d 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=3D? "modules.builtin" basename)) > + (not (string=3D? "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=E2=80=99d 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=E2=80=99s creating =E2=80= =98System.maps=E2=80=99 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-fil= e 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 =E2=80=98guix system build -s =E2=80=A6 -d=E2=80=99 = and checking the relevant .drv. :-) I don=E2=80=99t think #:allow-collisions?, #:locales? and #:relative-symlin= ks? are needed, so I=E2=80=99d 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 i= t." > + ;; 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 =E2=80=9CThis is meant to be used as a profile hoo= k.=E2=80=9D 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 =E2=80=98use-modules=E2=80=99 form. > + (let* ((inputs '#$(manifest-inputs manifest)) > + (module-directories #$(input-files (manifest-inputs m= anifest) "/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) =3D=3D 1: return mod= ule-directories[0] > + (mkdir-p (string-append #$output "/lib")) > + (match versions > + ((version) > + (make-linux-module-directory #$kmod inputs version #$o= utput))) > + (exit #t))))) No need for =E2=80=98exit=E2=80=99, but perhaps and =E2=80=98error=E2=80=99= call in the unmatched case? Thanks, and apologies for the delay! Ludo=E2=80=99.