From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:470:142:3::10]:57033) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jDlXX-0005am-Q6 for guix-patches@gnu.org; Mon, 16 Mar 2020 04:56:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jDlXW-0007GZ-HB for guix-patches@gnu.org; Mon, 16 Mar 2020 04:56:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:58298) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1jDlXW-0007Cv-9D for guix-patches@gnu.org; Mon, 16 Mar 2020 04:56:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jDlXW-0003qb-5w for guix-patches@gnu.org; Mon, 16 Mar 2020 04:56: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> <877dzlibaj.fsf@gnu.org> <20200315224832.5f2e336c@scratchpost.org> Date: Mon, 16 Mar 2020 09:55:35 +0100 In-Reply-To: <20200315224832.5f2e336c@scratchpost.org> (Danny Milosavljevic's message of "Sun, 15 Mar 2020 23:09:04 +0100") Message-ID: <874kuofzlk.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, Danny Milosavljevic skribis: > On Sun, 15 Mar 2020 22:00:04 +0100 > Ludovic Court=C3=A8s wrote: > >> I don=E2=80=99t think #:allow-collisions?, #:locales? and #:relative-sym= links? >> are needed, so I=E2=80=99d recommend removing them. > > Removing allow-collisions. > > Otherwise the defaults are different. > > I'm pretty sure that we don't need locales for Linux kernel modules, > for example :) #:locales? tells whether to install locales in the Guile process that builds the profile so that it can handle non-ASCII file names, for example. > That said, I can do it--but it would increase build dependencies. IMO it matters less than maintainability and conciseness in this case. :-) >> > + (let* ((inputs '#$(manifest-inputs manifest)) >> > + (module-directories #$(input-files (manifest-input= s manifest) "/lib/modules")) >> > + (directory-entries >> > + (lambda (directory-name) >> > + (scandir directory-name (lambda (basename) >> > + (not (string-prefix? = "." basename))))))=20=20 >>=20 >> also one-word identifiers are preferred for local >> variables. > > I'd like to do that but it would lose information here. > > "modules" would be too vague. "directories" would be non-unique. > (What "module-directories" means is "'/lib/modules'-directories", literal= ly) > > "entries" would be too vague too. Entries of what? > (Especially since that's a procedure). > > I'll make it say "directory" instead of "directory-name" there. Your call. My point is: if we keep with the general guideline of keeping functions small, then one-word identifiers are usually good enough because in the context of the function it should be clear and non-ambiguous. > Note: > > The "existing-files" procedure exists only in order to allow us to > build Linux kernels without any modules (neither in linux-libre nor anywh= ere > else) and have the profile hook succeed. > > Maybe it's written in an overly general way for that? What do you think? Yeah, maybe. It certainly looks weird to me to have a top-level procedure for something that=E2=80=99s in fact quite specific to the proble= m at hand (I realized when attempting to write a docstring that it=E2=80=99s a w= eird interface, and that=E2=80=99s because it=E2=80=99s in fact very specific to= what we=E2=80=99re doing here.) > (It's actually kinda bad that I ignore kernel-loadable-modules > which have no "/lib/modules" in it (better would be an error)--but I wasn= 't > sure whether manifest-inputs is guaranteed to keep the original order of > the entries--which would be: linux-libre first) Dunno, I guess it would be fine to error out when =E2=80=98kernel-loadable-modules=E2=80=99 is passed a package that doesn=E2= =80=99t have any modules. Thanks, Ludo=E2=80=99.